feat(bigquery): handle duplicate job error with jobs.query - #6801
alvarowolfx wants to merge 2 commits into
Conversation
product-auto-label
Bot
added
the
api: bigquery
|
The debatable aspect here is that 409 errors on jobs.query don't have a structured way to provide job ID, so we have to do some error message parsing. Thoughts @dbolduc ? |
![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 enhances BigQuery query execution by handling duplicate job errors (HTTP 409). When a duplicate job is detected, the system now parses the job reference from the error message and attempts to attach to the existing job. The review feedback highlights that the current parsing logic for duplicate job references is fragile when encountering domain-scoped project IDs without a location, and suggests a more robust parsing implementation along with corresponding unit test updates.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6801 +/- ##
==========================================
- Coverage 97.03% 97.02% -0.01%
==========================================
Files 326 326
Lines 107511 107659 +148
==========================================
+ Hits 104322 104459 +137
- Misses 3189 3200 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Error parsing is definitely a smell and brittle, but sometimes it must be done. We can open an issue / feature request with the service team to make it a structured error and reference that in the code. |
dbolduc
left a comment
dbolduc
left a comment
There was a problem hiding this comment.
Looks good. I like that we are making the effort to retry these.
| let existing_job = match get { | ||
| Some(get) => Box::pin(get.send()).await.ok(), | ||
| None => None, | ||
| }; |
There was a problem hiding this comment.
nit: this looks like an .and_then()
| // This error names the job and, unlike a `jobs.get` | ||
| // failure, never makes the retry loop reissue the query. |
There was a problem hiding this comment.
nit: there are a few ways to end up here. Should it be like?
// We were unable to parse the job in the error message, or successfully send a
// `GetJob` RPC. Return the original error message.
Or: // Oh well, we tried. 😛
2 participants
A transport/io error can happen after query was submitted via jobs.query and finished execution, and RPC level retries might try to re-run it with same request ID. But request ID only dedups in-flight queries, this causes a 409 error. Instead of re running the query, we can connect to the existing one to avoid extra billing.
Towards #6218 #6717