-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
ccl/sqlproxyccl/acl: fix TestParsingErrorHandling flake #99050
Conversation
TestParsingErrorHandling asserts that the error count metric is updated correctly when reading a file fails or succeeds, and the files are checked at a regular interval. For the tests that interval is set to 100ms, and we waited 200ms to ensure the metric would be updated, but that seems to not be reliable. This change increases the wait to 500ms which should ensure the file is re-read before we check the value of the error metics.
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.
Saw this flake here too so stamping this fix, thank you! - https://teamcity.cockroachdb.com/viewLog.html?buildId=9153860&buildTypeId=Cockroach_BazelEssentialCi
TFTR! bors r+ |
🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
Build failed (retrying...): |
Build failed: |
bors r=adityamaru,jaylim-crl |
Build succeeded: |
Regarding the release note, the details here seem to only apply to internal audiences, unless we want to slim this down and include a user-facing mention of the impact, e.g. performance/reliability. What do you think @pjtatlow? Will leave it out of release notes for now but can add back with edits if needed. |
TestParsingErrorHandling asserts that the error count metric is updated
correctly when reading a file fails or succeeds, and the files are checked
at a regular interval. For the tests that interval is set to 100ms, and we waited
200ms to ensure the metric would be updated, but that seems to not be reliable.
This change increases the wait to 500ms which should ensure the file is
re-read before we check the value of the error metics.
Fixes #98839