feat: retry certain RESOURCE_EXHAUSTED errors observed during ReadRows and report retry attempts#1257
Conversation
7616a05 to
2de0adb
Compare
2de0adb to
abc807f
Compare
shollyman
left a comment
There was a problem hiding this comment.
LGTM, with some minor nits.
| private RetryAttemptListener readRowsRetryAttemptListener = null; | ||
|
|
||
| /** | ||
| * If a non null readRowsRetryAttemptListener is provided, client will call onRetryAtempt function |
There was a problem hiding this comment.
nit: s/onRetryAtempt/onRetryAttempt here and in the other versions.
| public Duration retryDelay = null; | ||
| } | ||
|
|
||
| private static final Metadata.Key<RetryInfo> KEY_RETRY_INFO = |
There was a problem hiding this comment.
Interesting, we'll need to see if we have compatible key resolvers for other langs. I've not seen this before, but apparently its descriptor fullname and a "-bin" suffix?
There was a problem hiding this comment.
That's exactly what it does. I'm having a hard time finding external docs about why it is supposed to be like that, but you can find other libraries interacting with gcp services using the same keys, e.g. https://github.com/googleapis/google-cloud-go/blob/master/spanner/retry.go#L33
| Errors.IsRetryableStatusResult result = Errors.isRetryableStatus(status, metadata); | ||
| if (result.isRetryable) { | ||
| // If result.retryDelay isn't null, we know exactly how long we must wait, so both regular | ||
| // and randomized delays are the same. |
There was a problem hiding this comment.
Should there still be variance for the randomized delay? result.retryDelay + jitter? Looks like the previous impl didn't jitter either so likely can be ignored if its not been a source of issues.
There was a problem hiding this comment.
I don't think it is needed in this case.
c8c2e70 to
e6f0db2
Compare
Handle certain RESOURCE_EXHAUSTED errors and report the retry attempts.
e6f0db2 to
8b99da1
Compare
🤖 I have created a release \*beep\* \*boop\* --- ## [2.1.0](https://www.github.com/googleapis/java-bigquerystorage/compare/v2.0.4...v2.1.0) (2021-08-24) ### Features * retry certain RESOURCE_EXHAUSTED errors observed during ReadRows and report retry attempts ([#1257](https://www.github.com/googleapis/java-bigquerystorage/issues/1257)) ([d56e1ca](https://www.github.com/googleapis/java-bigquerystorage/commit/d56e1caf91297d7c2e1e4a9ce1463c04e44619c0)) ### Documentation * **sample:** Remove `client` from `JsonStreamWriter` in `WriteCommittedStream` ([#1248](https://www.github.com/googleapis/java-bigquerystorage/issues/1248)) ([6d38bd5](https://www.github.com/googleapis/java-bigquerystorage/commit/6d38bd5e3ff383e55e852081bbea5807796f59dd)) ### Dependencies * update dependency com.google.cloud:google-cloud-shared-dependencies to v2.1.0 ([#1261](https://www.github.com/googleapis/java-bigquerystorage/issues/1261)) ([0edb25d](https://www.github.com/googleapis/java-bigquerystorage/commit/0edb25d4a55f5480d5717672f30b09e6433483b9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [0.127.4](https://togithub.com/googleapis/java-storage-nio/compare/v0.127.3...v0.127.4) (2023-09-26) ### Dependencies * Update dependency com.google.apis:google-api-services-storage to v1-rev20230914-2.0.0 ([googleapis#1254](https://togithub.com/googleapis/java-storage-nio/issues/1254)) ([efe45f0](https://togithub.com/googleapis/java-storage-nio/commit/efe45f029dcbf318f36f5681ad10935bfcdc2808)) * Update dependency com.google.apis:google-api-services-storage to v1-rev20230922-2.0.0 ([googleapis#1259](https://togithub.com/googleapis/java-storage-nio/issues/1259)) ([80a7dbb](https://togithub.com/googleapis/java-storage-nio/commit/80a7dbbbaf523d5771d161e9df43415cee990b6d)) * Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.16.0 ([googleapis#1257](https://togithub.com/googleapis/java-storage-nio/issues/1257)) ([7f6d165](https://togithub.com/googleapis/java-storage-nio/commit/7f6d165e04c3e3bde0416b05d06076493806c1ac)) * Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.16.1 ([googleapis#1261](https://togithub.com/googleapis/java-storage-nio/issues/1261)) ([69f15c0](https://togithub.com/googleapis/java-storage-nio/commit/69f15c004c96fef4337d9dae30258a38fa29cad3)) * Update dependency com.google.cloud:google-cloud-storage to v2.27.1 ([googleapis#1263](https://togithub.com/googleapis/java-storage-nio/issues/1263)) ([b559148](https://togithub.com/googleapis/java-storage-nio/commit/b559148c1e084446c31a10731bfe6810ac8b5245)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Bq Storage Read service will start returning a retryable RESOURCE_EXHAUSTED error in the next few weeks when a read session's parallelism is considered to be excessive, so this PR expands retry handling logic for ReadRows with 2 changes: