Skip to content
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

Emit metrics for record_progress endpoint #709

Merged
merged 1 commit into from
Nov 5, 2023

Conversation

Mark-Simulacrum
Copy link
Member

Previously we were only tracking the worker time, not the endpoint. We see that there is a direct correlation with the throughput of a job and the worker time. This seems wrong to me, because as long as the worker is keeping up with the input rate, the throughput shouldn't be affected.

Note that we believe that the worker should not affect the HTTP endpoint at all - we connect these with a bounded queue and pushing into the queue is done with try_send, which shouldn't block (https://docs.rs/crossbeam-channel/latest/crossbeam_channel/struct.Sender.html#method.try_send) and returns an error if the queue is full. We already emit a metric if the queue is full, and that's not happening here.

The hope is that the extra metric here gives us some clue for what the problem is.

Metric graphs:

image image

Previously we were only tracking the worker time, not the endpoint.
@Mark-Simulacrum
Copy link
Member Author

@bors r+

@bors
Copy link
Contributor

bors commented Nov 5, 2023

📌 Commit bac1249 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Nov 5, 2023

⌛ Testing commit bac1249 with merge 4d35849...

@bors
Copy link
Contributor

bors commented Nov 5, 2023

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 4d35849 to master...

@bors bors merged commit 4d35849 into rust-lang:master Nov 5, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants