[xDS e2e test] fix flake in HcmConfigUpdatedWithoutRdsChange test#39810
Closed
markdroth wants to merge 1 commit intogrpc:masterfrom
Closed
[xDS e2e test] fix flake in HcmConfigUpdatedWithoutRdsChange test#39810markdroth wants to merge 1 commit intogrpc:masterfrom
markdroth wants to merge 1 commit intogrpc:masterfrom
Conversation
markdroth
added
the
release notes: no
eugeneo
approved these changes
Jun 9, 2025
grpc-checks
bot
added
bloat/none
per-call-memory/neutral
per-channel-memory/neutral
labels
anniefrchz
pushed a commit
to anniefrchz/grpc
that referenced
this pull request
Jun 25, 2025
…pc#39810) The test was assuming that as soon as the ADS server saw the client's ACK, the client had already switched to the new config. In practice, that's not a good assumption, because the client will send the ACK before it actually swaps the new config into place. So instead, we simply send RPCs until we detect that the client has seen the change. I believe this flake was triggered by grpc#39736, but that PR wasn't at fault; it just changed the timing enough to expose the race condition in this test. Closes grpc#39810 COPYBARA_INTEGRATE_REVIEW=grpc#39810 from markdroth:xds_cluster_e2e_test_flake_fix 5d7f84d PiperOrigin-RevId: 769643991
paulosjca
pushed a commit
to paulosjca/grpc
that referenced
this pull request
Aug 23, 2025
…pc#39810) The test was assuming that as soon as the ADS server saw the client's ACK, the client had already switched to the new config. In practice, that's not a good assumption, because the client will send the ACK before it actually swaps the new config into place. So instead, we simply send RPCs until we detect that the client has seen the change. I believe this flake was triggered by grpc#39736, but that PR wasn't at fault; it just changed the timing enough to expose the race condition in this test. Closes grpc#39810 COPYBARA_INTEGRATE_REVIEW=grpc#39810 from markdroth:xds_cluster_e2e_test_flake_fix 5d7f84d PiperOrigin-RevId: 769643991
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The test was assuming that as soon as the ADS server saw the client's ACK, the client had already switched to the new config. In practice, that's not a good assumption, because the client will send the ACK before it actually swaps the new config into place. So instead, we simply send RPCs until we detect that the client has seen the change.
I believe this flake was triggered by #39736, but that PR wasn't at fault; it just changed the timing enough to expose the race condition in this test.