-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
load stats: fix integration test flake #12265
Conversation
Waiting on a load stats response can race with resetting the counters when initializing a watch. Moving the counter increment prevents the race. Fixes #11784 Signed-off-by: Matt Klein <[email protected]>
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 change LGTM (do no harm), but I don't fully grok why this fixes the race. Can you elaborate a little more for posterity?
There is a race condition here: envoy/test/integration/load_stats_integration_test.cc Lines 603 to 605 in da58cfc
Sorry I should have made this more clear. This took me a while to figure out. |
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.
Makes sense, thanks!
Waiting on a load stats response can race with resetting the counters when initializing a watch. Moving the counter increment prevents the race. Fixes envoyproxy#11784 Signed-off-by: Matt Klein <[email protected]> Signed-off-by: Yuchen Dai <[email protected]>
Waiting on a load stats response can race with resetting the counters when initializing a watch. Moving the counter increment prevents the race. Fixes envoyproxy#11784 Signed-off-by: Matt Klein <[email protected]> Signed-off-by: Yuchen Dai <[email protected]> Co-authored-by: Matt Klein <[email protected]>
Waiting on a load stats response can race with resetting the counters when initializing a watch. Moving the counter increment prevents the race. Fixes envoyproxy#11784 Signed-off-by: Matt Klein <[email protected]> Signed-off-by: Kevin Baichoo <[email protected]>
Waiting on a load stats response can race with resetting the counters when initializing a watch. Moving the counter increment prevents the race. Fixes envoyproxy#11784 Signed-off-by: Matt Klein <[email protected]> Signed-off-by: chaoqinli <[email protected]>
Waiting on a load stats response can race with resetting
the counters when initializing a watch. Moving the counter
increment prevents the race.
Fixes #11784
Risk Level: Low
Testing: Existing tests, 1000 runs of the integration test
Docs Changes: N/A
Release Notes: N/A