Repository navigation
[Java] Set job location when cleaning up BigQueryIO export reads - #40794
Conversation
There was a problem hiding this comment.
📋 Review Summary
This PR fixes a bug in BigQueryIO export reads where the post-read cleanup step looked up the extract job without specifying a location, causing either leaked temporary Avro files (on 404) or infinite retry loops (on 400 INVALID_ARGUMENT in regional endpoints). However, for query-based export reads, the cleanup step does not follow the same location resolution logic used when creating the query/temp dataset and extract job, leaving regional queries that infer location from withQueryTempDataset or dry-run referenced tables broken.
🔍 General Feedback
- The table read path in
getExtractJobLocationaccurately mirrorsBigQuerySourceBase#splitby looking up the source table's dataset location. - For query reads (
BigQueryQuerySourceDef),BigQueryQueryHelper.executeQueryresolves the location from three possible sources (configuredqueryTempDataset, dry-run referenced tables, orqueryLocation) and creates the temporary dataset and destination table in that location.BigQuerySourceBase#splitthen launches the extract job in that temp dataset's location. Relying solely ongetQueryLocation()ingetExtractJobLocationmisses the first two cases.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
The export-read cleanup looked up the extract job without a location, which fails for regional datasets: cleanup is skipped (404) or fails repeatedly, stalling streaming jobs (400, e.g. on TPC). For table reads, use the source table's dataset location. For query reads, resolve the location the same way executeQuery does (explicit location, else the first referenced table's dataset), via a shared helper that doesn't rerun the query. Fall back to a user-provided temp dataset's location. Add regression tests.
|
/gemini review |
There was a problem hiding this comment.
📋 Review Summary
This PR properly resolves and sets the BigQuery job location when looking up the extract job during post-read cleanup of BigQueryIO EXPORT reads. It handles both table reads (using the source table's dataset location) and query reads (via a shared BigQueryQueryHelper.resolveQueryLocation helper and tempDatasetId fallback), and includes comprehensive unit tests for both standard and template-compatible execution paths.
🔍 General Feedback
- The extracted
BigQueryQueryHelper.resolveQueryLocationcleanly deduplicates location resolution betweenexecuteQueryandBigQueryQuerySourceDef.getQueryLocationwithout re-executing queries or leaking resources. - Regression tests in
BigQueryIOReadTesteffectively cover both table and query export reads with and without template compatibility.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
|
Assigning reviewers: R: @chamikaramj for label java. 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). |
Part of #40569
Summary
BigQueryIO.read()with the defaultEXPORTmethod runs a cleanup step after the read that looks up the extract job to find and delete the exported Avro files. That lookup is built without a job location. BigQuery requires the location to look up jobs in regional locations, so for tables in regional datasets the lookup fails. Depending on the endpoint, cleanup is either silently skipped (leaking export files) or fails repeatedly, which stalls streaming pipelines indefinitely.This PR sets the job location on that lookup, the same way the export job itself does.
What happens today
The export job is created with a location (
BigQuerySourceBase#executeExtract):But the cleanup in
TypedRead#expandrebuilds the reference without one:The resulting
jobs.gethas nolocationparameter. For a job in a regional location:getJobtreats that as "not found" and returnsnull, so cleanup is silently skipped and the exported files under<tempLocation>/BigQueryExtractTemp/are leaked. This is easy to miss, because datasets in theUS/EUmulti-regions work without a location.INVALID_ARGUMENTinstead.getJobdoesn't treat that as "not found", retries, and throws. In a streaming pipeline the failed bundle is retried forever, so the job never makes progress.Observed on TPC (streaming pipeline reading a table in
u-us-prp1as a side input):The fix
When building the cleanup's job reference, set the location the export job ran in:
BigQueryHelpers.getDatasetLocation), the same valueBigQuerySourceBaseuses for the export.withQueryLocationvalue.The location is only set when known. Query reads without an explicit location behave as before. Per the
withQueryLocationJavadoc, a location must be specified for queries not executed inUS/EU, and those multi-regions work without one.