-
Notifications
You must be signed in to change notification settings - Fork 179
fix: retry rst stream #3000
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: retry rst stream #3000
Conversation
Can you rebase on to master? The integration tests should be less flaky |
@@ -263,6 +265,19 @@ protected boolean isRequestRetryable() { | |||
return rpc.isRetryable(getRetryRequest()); | |||
} | |||
|
|||
private boolean isRstStream(Status status) { | |||
if (!(this instanceof RetryingReadRowsOperation) || status.getCode() != Code.INTERNAL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can improve the isntanceof here
Maybe create a new method isRetryable(Status)
which is:
protected boolean isRetryable(Status s) {
return retryOptions.isRetryable(code);
}
and override it in RetryReadRowsoperation:
@OverRide
protected boolean isRetryable(Status s) {
return isRstStream(Status status) || super.isRetryable(code);
}
...lient-core/src/main/java/com/google/cloud/bigtable/grpc/async/AbstractRetryingOperation.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
* fix: retry rst stream * check if operation is readrows * Refactor * refactor
🤖 I have created a release \*beep\* \*boop\* --- ## [1.20.0-sp.3](https://www.github.com/googleapis/java-bigtable-hbase/compare/1.20.0-sp.2...v1.20.0-sp.3) (2021-09-10) ### Bug Fixes * retry rst stream (port of [#3000](https://www.github.com/googleapis/java-bigtable-hbase/issues/3000)) ([#3211](https://www.github.com/googleapis/java-bigtable-hbase/issues/3211)) ([9f63882](https://www.github.com/googleapis/java-bigtable-hbase/commit/9f6388228d94d78ef1676a86dc39ad849629ea88)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This was fixed in veneer client in https://github.com/googleapis/java-bigtable/pull/586/files but not fixed in bigtable-client-core.
I also noticed different error messages:
"Received Rst stream" mentioned in the original issue
"RST_STREAM closed stream" from b/189326784
"Received RST_STREAM" mentioned in b/179093825
So checking all 3 cases.
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:
Fixes #2177 ☕️