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

fix: make the timings recorded in wait_duration and histogram consistent #101

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

aqrln
Copy link
Collaborator

@aqrln aqrln commented Oct 15, 2024

Follow-up to #99, there was a comment from me that wasn't addressed: #99 (comment).

We were computing the time spent waiting for a connection twice, once to add it to internals.wait_duration and once to record it in a histogram, so the latter is technically always larger. If the process is preempted by the kernel between these points, the timings can diverge significantly, especially in resource-constrained environments when running many threads/processes.

This PR changes the DurationHistogramGuard::elapsed method into a method that consumes self and records the elapsed time and returns in lockstep, only invoking Instant::elapsed once. The destructor that updates the histogram still exists to make the API misuse-resistant and because that's the contract of a guard, but it's not a code path that happens to be taken anywhere in practice right now.

@SevInf SevInf merged commit 4c05a47 into importcjj:main Oct 15, 2024
1 check passed
@aqrln aqrln deleted the duration-histogram-guard-consistency branch October 15, 2024 15:55
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