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: latency calculation in check #1314

Merged
merged 2 commits into from
Feb 4, 2025
Merged

fix: latency calculation in check #1314

merged 2 commits into from
Feb 4, 2025

Conversation

yashmehrotra
Copy link
Member

@@ -189,15 +190,17 @@ ORDER BY time
}
uptime.Failed += datapoint.Failed
uptime.Passed += datapoint.Passed
latencies.Append(float64(datapoint.Duration))
latencies = append(latencies, float64(datapoint.Duration))
Copy link
Member

Choose a reason for hiding this comment

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

Latencies will grow for as long canary-checker ius running:

This is what claude produced for a circular buffer, that segements latencies into buckets and then drops buckets after they expire: https://gist.github.com/moshloop/45308b7f3b48bd1a1a9b7ef0a54e4056

Although I suspect this will cause a significant amount of CPU in environments with hundreds to thousands of canaries.

The reason we originally chose the rolling implementation is that it is very fast, and approximately right.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just for the graph api call, not the CRD status latencies. For this, we query check_statuses duration and then take the percentlie latency from that set

@yashmehrotra yashmehrotra requested a review from moshloop February 3, 2025 08:59
@moshloop moshloop merged commit 4a38e48 into main Feb 4, 2025
7 checks passed
@moshloop moshloop deleted the fix-latency-check branch February 4, 2025 08:36
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.

[BUG] Percentiles not works 97%
2 participants