fix: handled race condition in stateless query integration test #4045
Conversation
The testTableResultJobIdAndQueryId test was failing intermittently on slower networks. The test strictly asserted that Job ID must be null for stateless queries. However, the library correctly falls back to creating a Job ID if the stateless query times out. This change updates the assertion logic to accept either a valid Query ID (stateless success) or a valid Job ID (fallback success). Fixes googleapis#4008
product-auto-label
bot
added
size: s
Summary of ChangesHello @SivamuruganP, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an intermittent integration test failure in the BigQuery client library. The test, which verifies stateless query behavior, was failing due to a race condition where network latency could cause the BigQuery service to fall back to creating a Job ID for a query intended to be stateless. The change updates the test's assertions to correctly validate both the intended stateless outcome (presence of a Query ID) and the acceptable fallback behavior (presence of a Job ID when a timeout occurs), ensuring the test accurately reflects the library's contract. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a race condition in the ITBigQueryTest:testTableResultJobIdAndQueryId integration test. The original test incorrectly assumed that a stateless query would never result in a jobId. The fix correctly handles the fallback scenario where a jobId is created due to network latency. The logic is sound. I've suggested a small refactoring to make the assertion more concise and readable.
google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/it/ITBigQueryTest.java
Outdated
Show resolved
Hide resolved
Applied feedback from code review to use exclusive OR operator for validating JobID/QueryID mutual exclusivity.
product-auto-label
bot
added
size: xs
Applied XOR assertion logic to testStatelessQueries. Test was failing on slow networks because they did not account for JOB_CREATION_OPTIONAL falling back to job creation. Fixes googleapis#4002
product-auto-label
bot
added
size: s
lqiu96
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
|
Thanks you for your contribution! We'll take a look at this over the next few days. |
|
/gcbrun |
| TableResult tableResult = executeSimpleQuery(bigQuery); | ||
| assertNotNull(tableResult.getQueryId()); | ||
| assertNull(tableResult.getJobId()); | ||
| // Use XOR: We accept EITHER a QueryId (fast path) OR a JobId (slow fallback), but not both. |
There was a problem hiding this comment.
nit: Can you add a bit more to the comment to explain the why. Same for the change below
e.g. For stateless queries, bigquery may determine to return a job instead of a query based on the query complexity or other factors (job timeout config).
There was a problem hiding this comment.
Addressed. Added the explanation about the complexity/timeout fallback mechanism.
lqiu96
left a comment
lqiu96
left a comment
There was a problem hiding this comment.
Changes LGTM.
stateless query times out (race condition)
I believe the default jobTimeoutMs is configured to be 10000 (10s). IIUC, for these tests I don't it's due timeouts specifically. Seems to be some non-deterministic factors inside the BQ engine that has it return a job instead of a query.
3 participants
Fixes #4008, Fixes #4002 ☕️
The integration tests
ITBigQueryTest:testTableResultJobIdAndQueryIdandITBigQueryTest:testStatelessQuerieswere failing intermittently on slower networks.Both tests strictly asserted that Job ID must be null for stateless queries. However, the library correctly falls back to creating a Job ID if the stateless query times out (race condition).
This change updates the assertion logic in both tests to accept either a valid Query ID (stateless success) or a valid Job ID (fallback success) using an XOR check.
Checklist