Skip to content

Treat a 404 from BigQuery table and dataset deletes as success - #39934

Open
mxtymoshyk wants to merge 1 commit into
apache:masterfrom
mxtymoshyk:bq-delete-404-is-success
Open

Treat a 404 from BigQuery table and dataset deletes as success#39934
mxtymoshyk wants to merge 1 commit into
apache:masterfrom
mxtymoshyk:bq-delete-404-is-success

Conversation

@mxtymoshyk

Copy link
Copy Markdown

BigQueryIO creates temporary tables and datasets and deletes them when it is done. A delete can succeed at BigQuery and still have its work item fail to commit afterwards. The runner then replays that work item, and the replayed delete gets a 404 because the first attempt already removed the resource.

DatasetServiceImpl.deleteTable and deleteDataset passed ALWAYS_RETRY, so that 404 was retried MAX_RPC_RETRIES (9) times with exponential backoff and then rethrown as an IOException. In a streaming job the work item is retried forever and the job cannot drain.

Both methods now pass DONT_RETRY_NOT_FOUND and swallow an item-not-found error, since "the resource is gone" is the outcome the caller asked for. This mirrors how getTable in the same class already handles a 404. Every other status code keeps its existing retry count and failure behaviour.

fixes #24997

Notes for reviewers

  • The 404 is only swallowed for the two delete calls. dryRunQuery and patchTableDescription still use ALWAYS_RETRY, and no getX/create/patch path changed.
  • ApiErrorExtractor.itemNotFound walks getCause(), so it still recognises the 404 after executeWithRetries wraps it in a new IOException -- the same reason the existing getTable catch block works.
  • A swallowed 404 is logged at INFO with the table/dataset id, so an operator can still see that the delete found nothing.
  • The two new tests prepare exactly one mock response, so a retry trips the Verify in the shared mock request. They assert both "did not throw" and "did not retry".
  • Verified locally on :sdks:java:io:google-cloud-platform: BigQueryServicesImplTest passes 50/50 with the change. Reverting only the BigQueryServicesImpl change makes both new tests fail with a VerifyException from the retried request, confirming the tests cover the fix.
  • No prior PR referenced this issue.

BigQueryIO deletes the temporary tables and datasets it creates. A delete
can succeed at BigQuery and still have its work item fail to commit
afterwards; the runner then replays the work item and the replayed delete
gets a 404 because the first attempt already removed the resource.

deleteTable and deleteDataset passed ALWAYS_RETRY, so that 404 was retried
MAX_RPC_RETRIES times and then thrown. In a streaming job the work item
retries forever, which stalls a drain.

Both now use DONT_RETRY_NOT_FOUND and swallow an item-not-found error,
matching how getTable already handles a 404. Every other status keeps its
existing retry and failure behaviour.
@github-actions

Copy link
Copy Markdown
Contributor

Assigning reviewers:

R: @kennknowles for label java.

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: BigqueryIO: 404 on delete should be a success

1 participant