-
Notifications
You must be signed in to change notification settings - Fork 116
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
chore: changing health status thresholds #1905
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Samhith Kakarla <[email protected]>
Signed-off-by: Samhith Kakarla <[email protected]>
Signed-off-by: Samhith Kakarla <[email protected]>
Signed-off-by: Samhith Kakarla <[email protected]>
unit tests were erroring because of change in thresholds so i changed the cutoff values in the tests to be dependent on DefaultBufferUsageLimit and match the new thresholds. Also added TestHealthThresholds to show that thresholds are actually changing as expected when BufferUsageLimit changes |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1905 +/- ##
==========================================
- Coverage 59.82% 58.24% -1.59%
==========================================
Files 286 291 +5
Lines 23689 24390 +701
==========================================
+ Hits 14172 14205 +33
- Misses 8588 9243 +655
- Partials 929 942 +13 ☔ View full report in Codecov by Sentry. |
// criticalBufferThreshold is the threshold above which the health of a vertex is critical | ||
criticalBufferThreshold = 95 | ||
criticalBufferThreshold = v1alpha1.DefaultBufferUsageLimit * float64(90) // 95 |
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.
The threshold for different vertex should be different - based on the vertex limits
configuration, is this right? @kohlisid
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.
Yes now that we have it relative, it can vary vertex to vertex.
The code will need changes to compare each timeline using the limits for the corresponding vertex limits.
Signed-off-by: Samhith Kakarla <[email protected]>
…a/numaflow into health-thresholds
Hey @samhith-kakarla , Yashash and I just made changes to our e2e tests and committed to the main. If you continue working on your branch, you might see some un-expected e2e testing failures. To fix them, please rebase. Thanks! |
Explain what this PR does.