-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DM-37973: Display Czar monitoring parameters on the Qserv Web Dashboard #809
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
iagaponenko
force-pushed
the
tickets/DM-37973
branch
2 times, most recently
from
September 13, 2023 01:19
8f689b6
to
c72e287
Compare
jgates108
approved these changes
Sep 13, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just minor comments.
iagaponenko
force-pushed
the
tickets/DM-37973
branch
from
September 14, 2023 07:18
c72e287
to
96a4755
Compare
iagaponenko
force-pushed
the
tickets/DM-37973
branch
from
September 14, 2023 07:28
96a4755
to
d8caf55
Compare
iagaponenko
changed the title
Tickets/dm 37973
DM-37973: Display Czar monitoring parameters on the Qserv Web Dashboard
Sep 14, 2023
fritzm
approved these changes
Sep 20, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmake and js LGTM, thanks!
The new implementation allows not to provide database names in the connection strings. The connection string parser was extended with an additional parameter allowing to privide the default database name if the one is found missing in the input connection string.
The change was made to the Configuraton servive of the Replication/Ingest system.
Refactored dependencies accordingly. The move allows re-using the class by other Qserv modules w/o adding a direct depedency onto the module 'replica' which was the original location of the class. Fixed a bug in the implementation of the timer to allow subsequent restarts. Unblocking implementation of the callback calling mechanism. Extended a model of the callbacks to allow ordering automatic restarts of the timer via the return value (boolean type) of the callbacks. Minor refactpring of the implementation.
Extend the Histogram class to report the histogram label (id). Added a histogram to capture the performance of the result file reads at Czar. Added counters for the number of on-going result file reads (file-based result delivery protocol only) and merges. Fixed database merge histogram load. Added integral counters for metrics reset at the start up time of Czar. Added the timestamp (milliseconds) attribute to the JSON object that returns a state of the monitoring parameters. The timestamps are going to be used for the performance monitoring of Qserv.
The history is recorded in the transient monitoring store (class qserv::CzarStats) from where it could be sampled at run time via the Qserv mysql-proxy service.
The requests are made by calling a special stored procedure that is recognized by Qserv. The procedure allows a single parameter (a command) that is interpreted by Czar and processed accordingly.
The fix was introduced after discovering that the shared connection usage approach in the original design of this and related classes is highly unreliable. A connection shared by the queries seems to be sensitive to the service startup ordering and occasional disconnects. As a result of that the shared connection could just stop working or it could not be alive from the very start of Czar. It's also possible that the shared connection object is not properly synchronized to allow using it in the multi-threaded environment (which would be a bug). The last theory will be further investigated later.
The service was replaced with two separate services. The first one reports info on the ongoing queries. The second service is used for searching and displaying info on the past queries.
The improvement aimed at reducing code duplication as well as the total amounbt of code in the application.
Separated the query into into two pages - the active queries and the past queries. Also rearranges sub-tabs under the 'Status' tab for more intuitive navigation.
Added a link (an additional column on the active queries table) to the newely added Query progression plot. Fixed minor layout bugs on the active and past query tables. Loading the Highcharts.js library on demand.
iagaponenko
force-pushed
the
tickets/DM-37973
branch
from
September 21, 2023 13:39
d8caf55
to
dc3b3b3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.