[SPARK-47081][CONNECT][FOLLOW] Unflake Progress Execution #46060
Conversation
This file contains 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
hvanhovell
approved these changes
Apr 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Why are the changes needed?
Running some of the tests repetitively have shown that there is an edge case in which the progress reporting yields undesired behavior. This is in particular the case when the response sender loop has already sent the last expected message in the stream to indicate that the stream is finished, but the response sender would send at least one additional progress message.
This patch removes this race condition and guarantees that the
ResultCompletemessage is the last message on the stream.Does this PR introduce any user-facing change?
No
How was this patch tested?
Added an additional UT, but existing tests cover the race condition.
Was this patch authored or co-authored using generative AI tooling?
No