Skip to content
This repository was archived by the owner on Mar 6, 2026. It is now read-only.

fix: retry 'job exceeded rate limits' for DDL queries - #1794

Merged
kiraksi merged 12 commits into
googleapis:mainfrom
kiraksi:job-rate-limit
Jan 24, 2024
Merged

fix: retry 'job exceeded rate limits' for DDL queries#1794
kiraksi merged 12 commits into
googleapis:mainfrom
kiraksi:job-rate-limit

Conversation

@kiraksi

Copy link
Copy Markdown
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes#1790 🦕

@product-auto-labelproduct-auto-labelBot added size: s Pull request size is small. api: bigquery Issues related to the googleapis/python-bigquery API. labels Jan 19, 2024
@kiraksi
kiraksi marked this pull request as ready for review January 19, 2024 23:46
@kiraksi
kiraksi requested review from a team, PhongChuong, chalmerlowe and tswast and removed request for PhongChuongJanuary 19, 2024 23:46

@tswasttswast left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I have some feedback regarding testing. I think it may make more sense to follow the pattern I use here, instead:

deftest_query_and_wait_retries_job():
freezegun.freeze_time(auto_tick_seconds=100)
client=mock.create_autospec(Client)
client._call_api.__name__="_call_api"
client._call_api.__qualname__="Client._call_api"
client._call_api.__annotations__= {}
client._call_api.__type_params__= ()
client._call_api.side_effect= (
google.api_core.exceptions.BadGateway("retry me"),
google.api_core.exceptions.InternalServerError("job_retry me"),
google.api_core.exceptions.BadGateway("retry me"),
{
"jobReference": {
"projectId": "response-project",
"jobId": "abc",
"location": "response-location",
},
"jobComplete": True,
"schema": {
"fields": [
{"name": "full_name", "type": "STRING", "mode": "REQUIRED"},
{"name": "age", "type": "INT64", "mode": "NULLABLE"},
],
},
"rows": [
{"f": [{"v": "Whillma Phlyntstone"}, {"v": "27"}]},
{"f": [{"v": "Bhetty Rhubble"}, {"v": "28"}]},
{"f": [{"v": "Phred Phlyntstone"}, {"v": "32"}]},
{"f": [{"v": "Bharney Rhubble"}, {"v": "33"}]},
],
},
)
rows=_job_helpers.query_and_wait(
client,
query="SELECT 1",
location="request-location",
project="request-project",
job_config=None,
page_size=None,
max_results=None,
retry=retries.Retry(
lambdaexc: isinstance(exc, google.api_core.exceptions.BadGateway),
multiplier=1.0,
).with_deadline(
200.0
), # Since auto_tick_seconds is 100, we should get at least 1 retry.
job_retry=retries.Retry(
lambdaexc: isinstance(exc, google.api_core.exceptions.InternalServerError),
multiplier=1.0,
).with_deadline(600.0),
)
assertlen(list(rows)) ==4
# For this code path, where the query has finished immediately, we should
# only be calling the jobs.query API and no other request path.
request_path="/projects/request-project/queries"
forcallinclient._call_api.call_args_list:
_, kwargs=call
assertkwargs["method"] =="POST"
assertkwargs["path"] ==request_path

It may make sense to also add a test for just the predicate itself:

deftest_DEFAULT_JOB_RETRY_predicate():

Comment threadtests/unit/test_job_retry.py Outdated
Comment threadtests/unit/test_job_retry.py Outdated
Comment threadtests/unit/test_job_retry.py Outdated
Comment threadtests/unit/test_job_retry.py Outdated
Comment threadtests/unit/test_job_retry.py Outdated
Comment threadtests/unit/test_job_retry.py Outdated
@product-auto-labelproduct-auto-labelBot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Jan 22, 2024
@kiraksi
kiraksi requested a review from tswastJanuary 22, 2024 22:56
Comment threadtests/unit/test_job_retry.py Outdated
Comment threadtests/unit/test_job_retry.py Outdated
Comment threadtests/unit/test_job_retry.py Outdated
Comment threadtests/unit/test_job_retry.py Outdated
Comment threadtests/unit/test_job_retry.py Outdated

_, kwargs = calls[2]
assert kwargs["method"] == "POST"
assert kwargs["path"].startswith(response_path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting one. Not sure why we'd be doing a POST to this path. That'd indicate that we are doing a jobs insert call, which really shouldn't be happening.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a todo statement for the fix in #1797

Comment threadtests/unit/test_job_retry.py Outdated
Comment threadtests/unit/test_job_retry.py Outdated
@kiraksi
kiraksi requested a review from tswastJanuary 24, 2024 21:28

@tswasttswast left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@nlenepveu

Copy link
Copy Markdown

Thanks @kiraksi!

@kiraksi
kiraksi deleted the job-rate-limit branch January 24, 2024 23:22
Comment threadtests/unit/test_job_retry.py

from google.cloud.bigquery.client import Client
from google.cloud.bigquery import _job_helpers
from google.cloud.bigquery.retry import DEFAULT_JOB_RETRY

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: bigqueryIssues related to the googleapis/python-bigquery API.size: mPull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: client doesn't retry "Job exceeded rate limits" for DDL query jobs that exceed quota for table update operations

3 participants

@kiraksi@nlenepveu@tswast