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

buffered_metrics: reduce a bit lock contention #289

Merged
merged 5 commits into from
Nov 15, 2023

Conversation

iksaif
Copy link
Contributor

@iksaif iksaif commented Nov 7, 2023

062e18c introduced a performance regression by trying to fix the computation of the actual sampling rate.

This commit takes some shortcuts to reduce lock contention while trying to keep the sampling rate correct.

062e18c introduced
a performance regression by trying to fix the computation
of the actual sampling rate.

This commit takes some shortcuts to reduce lock contention while trying to keep the sampling rate
correct.
vickenty
vickenty previously approved these changes Nov 10, 2023
statsd/metrics.go Outdated Show resolved Hide resolved
Comment on lines +198 to +199
if s.specifiedRate != 1.0 {
rate = s.specifiedRate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't match comment on specifiedRate. Should we be checking totalSamples in the condition above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update the comment, originally I planned to try to do something smarter, but I think it would require more thinking .. so reverting to the previous behavior is a better first step

statsd/buffered_metric_context.go Outdated Show resolved Hide resolved
statsd/options.go Outdated Show resolved Hide resolved
Co-authored-by: Vickenty Fesunov <[email protected]>
@vickenty vickenty merged commit 54ec306 into DataDog:master Nov 15, 2023
22 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.

3 participants