Skip to content
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(Spanner): add retry for RST_STREAM #5938

Merged
merged 10 commits into from Apr 5, 2023

Conversation

vishwarajanand
Copy link
Contributor

@vishwarajanand vishwarajanand commented Mar 9, 2023

This PR can be merged only after we bump up core version in Spanner. The new version will be released as part of #5948

Spanner is supposed to retry on RST_STREAM errors. We retry on all INTERNAL errors, code pointer ref so only runTransaction need to be additionally covered.

This will bring PHP spanner in parity with other language's retry on error codes, ref: googleapis/java-spanner#2111, googleapis/google-cloud-go#2460 and also potentially fix #5473.

Changes

Add default retry delay for RST_STREAM errros.

Tests

Added a new UT for validating that the retry happens for RST_STREAM errors.

ref: b/272395017

@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Mar 9, 2023
@vishwarajanand vishwarajanand marked this pull request as ready for review March 9, 2023 10:39
@vishwarajanand vishwarajanand requested review from a team as code owners March 9, 2023 10:39
Spanner/src/Database.php Outdated Show resolved Hide resolved
@vishwarajanand vishwarajanand added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Mar 13, 2023
@vishwarajanand vishwarajanand removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Mar 27, 2023
@saranshdhingra saranshdhingra self-assigned this Mar 28, 2023
Copy link
Contributor

@bshaffer bshaffer left a comment

Choose a reason for hiding this comment

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

two suggestions, but they're both minor!

.github/workflows/spanner-emulator-system-tests.yaml Outdated Show resolved Hide resolved
Spanner/src/Database.php Outdated Show resolved Hide resolved
@bshaffer bshaffer merged commit aa3bc8e into googleapis:main Apr 5, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Spanner] Server randomly returns "ServerException: INTERNAL: Received RST_STREAM with error code 2".
4 participants