Repository navigation
Fix flaky PipelineBasedStreamingInsertTest.test_failure_in_some_rows_does_not_duplicate - #41004
Conversation
…does_not_duplicate The mocked insert_rows_json call failed rows by their position in the batch (index 0 and 2), assuming they are always 'value1' and 'value5'. However, _StreamToBigQuery assigns each row a random shard key in [0, 500] and applies ReshufflePerKey, so the order in which rows reach the client is not guaranteed. When 'value1' and 'value5' land on the same shard (~0.2% of runs), the rows arrive as [value1, value5, value3], and 'value3' is failed instead of 'value5', which breaks the RETRY_NEVER variant. Identify the rows to fail by value instead of by index.
|
Encountered in #40960 (comment) |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review Summary
This PR fixes a flaky unit test (PipelineBasedStreamingInsertTest.test_failure_in_some_rows_does_not_duplicate) by matching simulated row failures by row value rather than batch index. Because _StreamToBigQuery assigns random shard keys and applies ReshufflePerKey, batch ordering is non-deterministic when shard keys collide.
🔍 General Feedback
- The root cause analysis and fix are clean, self-contained, and well-commented.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
|
/gemini review |
There was a problem hiding this comment.
📋 Review Summary
This PR resolves a test flake in PipelineBasedStreamingInsertTest.test_failure_in_some_rows_does_not_duplicate caused by non-deterministic row ordering after _StreamToBigQuery's random-key reshuffle. By matching rows to fail by their columnA value rather than their batch index, the mock callback consistently fails 'value1' and 'value5' regardless of arrival order.
🔍 General Feedback
- The root cause analysis and fix are clean, well-documented, and directly address the ~0.2% shard collision flake without altering the test's intent.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #41004 +/- ##
==========================================
+ Coverage 79.83% 80.09% +0.25%
==========================================
Files 494 494
Lines 83575 83615 +40
==========================================
+ Hits 66722 66970 +248
+ Misses 16853 16645 -208
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Assigning reviewers: R: @claudevdm for label python. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
The mocked insert_rows_json call failed rows by their position in the batch (index 0 and 2), assuming they are always 'value1' and 'value5'. However, _StreamToBigQuery assigns each row a random shard key in [0, 500] and applies ReshufflePerKey, so the order in which rows reach the client is not guaranteed. When 'value1' and 'value5' land on the same shard (~0.2% of runs), the rows arrive as [value1, value5, value3], and 'value3' is failed instead of 'value5', which breaks the RETRY_NEVER variant.
Identify the rows to fail by value instead of by index.