Datastore: expose retry parameters

[oliverchang]

Would it be possible to expose retry parameters for datastore?

We are seeing frequent "InternalServerError: 500 GOAWAY received" errors during queries, and it looks like the default retry does not handle internal errors: https://github.com/GoogleCloudPlatform/google-cloud-python/blob/master/datastore/google/cloud/datastore_v1/gapic/datastore_client_config.py#L5

InternalServerError: 500 GOAWAY received
at raise_from (c:\project\src\third_party\six.py:737)
at error_remapped_callable (c:\project\src\third_party\google\api_core\grpc_helpers.py:61)
at func_with_timeout (c:\project\src\third_party\google\api_core\timeout.py:206)
at retry_target (c:\project\src\third_party\google\api_core\retry.py:177)
at retry_wrapped_func (c:\project\src\third_party\google\api_core\retry.py:260)
at __call__ (c:\project\src\third_party\google\api_core\gapic_v1\method.py:139)
at run_query (c:\project\src\third_party\google\cloud\datastore_v1\gapic\datastore_client.py:294)
at _next_page (c:\project\src\third_party\google\cloud\datastore\query.py:517)
at _page_iter (c:\project\src\third_party\google\api_core\page_iterator.py:230)
at _items_iter (c:\project\src\third_party\google\api_core\page_iterator.py:199)

It's not possible for us to add our own retry here either to retry fetching a particular page, as _items_iter and _page_iter are generators and we can't retry on failures during iteration.

Would it be possible to expose these settings somehow, or add INTERNAL to the list of retryable errors? We'd like to avoid doing something like #3128 (comment)

[inferno-chromium]

This is not a feature, but a bug that is triggering quite often on our fuzzing infrastructure. @tseaver can you prioritize this. Or just add InternalServerError to the retry_codes before next release please ?

[glarrain]

I feel I've just hit a wall regarding setting retry/timeout strategy/settings for the Python clients for GCP services. The more I dig up it feels this fundamental missing feature (and for some cases it could be argued it is more a bug than a requested feature) has no priority for the project maintainers.

See #3109 and #2988. Both were closed on the argument that some work was in progress that would eventually fix the issues (they didn't). That was in April and August 2017!

[tseaver]

The list of "idempotent" errors is generated from the datastore_gapic.yaml file (see its public mirror).

@pcostell Could you open a bug to update the upstream repo for googleapis to include 500 errors as idempotent?

[tseaver]

FWIW, I'd be happy to review / merge a PR which added retry arguments to the various top-level methods which invoke the GAPIC layer. For an example, see spanner's Session.execute_sql method. The hardest part about the PR is adding test coverage / assertions that the new argument is forwarded to the GAPIC layer.

[tseaver]

Overall design of retries for manual clients is tracked in #9298.