-
Notifications
You must be signed in to change notification settings - Fork 198
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
Add Prometheus metrics endpoint #511
Merged
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
… refactoring for testability
🦾 🥳 |
Closed
Added #514 for making this more configurable |
andygrove
approved these changes
Nov 16, 2022
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.
Fantastic work @thinkharderdev
I tested this and it works well.
# HELP job_cancelled_total Counter of cancelled jobs
# TYPE job_cancelled_total counter
job_cancelled_total 0
# HELP job_completed_total Counter of completed jobs
# TYPE job_completed_total counter
job_completed_total 3
# HELP job_exec_time_seconds Histogram of successful job execution time in seconds
# TYPE job_exec_time_seconds histogram
job_exec_time_seconds_bucket{le="0.5"} 0
job_exec_time_seconds_bucket{le="1"} 0
job_exec_time_seconds_bucket{le="5"} 0
job_exec_time_seconds_bucket{le="30"} 3
job_exec_time_seconds_bucket{le="60"} 3
job_exec_time_seconds_bucket{le="+Inf"} 3
job_exec_time_seconds_sum 28.743
job_exec_time_seconds_count 3
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.
Which issue does this PR close?
Closes #504
Closes #507
Depends on #504
Posting this for review now, the functionality is done but I also want to address #507 here. If I can't get to that before this is approved it's fine to merge and I can add the user guide in a separate PR.Rationale for this change
Track the size of the pending task queue in the scheduler and expose through both prometheus metrics and the external scaler service.
The number of pending tasks is the number of tasks that can be scheduled but for which there is no executor slot to schedule it on (i.e. it does not count tasks for unresolved stages, etc).
What changes are included in this PR?
Add a
pending_tasks
state to theQueryStageScheduler
and track the value in response to scheduler events.I also added some more bells and whistles to the
SchedulerTest
utility introduced in the previous PR. We can now submit a job and control when "virtual" executors send their task updates so we have finer-grained control to test different scenarios.Added a rudimentary user guide.
Are there any user-facing changes?
The external scaler service should now return the actual number of pending tasks instead of a hard-coded value. Also, the pending task queue size should be available in the prometheus metrics.
No