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

fix: Add ABORTED to retryable status codes.#391

Merged
chrisrossi merged 1 commit into
googleapis:masterfrom
chrisrossi:fix-383
Apr 20, 2020
Merged

fix: Add ABORTED to retryable status codes.#391
chrisrossi merged 1 commit into
googleapis:masterfrom
chrisrossi:fix-383

Conversation

@chrisrossi
Copy link
Copy Markdown
Contributor

Fixes #383.

@chrisrossi chrisrossi requested a review from cguardia April 16, 2020 18:43
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 16, 2020
@chrisrossi
Copy link
Copy Markdown
Contributor Author

A little bit of background discussion, since even though it's a simple change, the implications are somewhat complicated:

Andrew Gorcester, Tue 5:22 PM

@chris Rossi Earlier I said ABORTED could be retried; I thought better of it and dug a little deeper and it's a bit more complicated than that
From Clem: "the semantics of ABORTED is: "retry at a higher level"
for transactions, this means retrying the complete transaction, not only the Commit RPC"
That suggests that some RPCs could potentially be retried if they're self-contiained but many cannot be, they would need user interaction or code that retries the entire process from the start

Chris Rossi, Yesterday 11:35 AM

Ok. I think NDB has retry for transactions and for individual RPCs. I'll take a closer look and see if I can't tease out a way to apply this to the former and not the latter.

Chris Rossi, Yesterday 4:33 PM

So, I'm poking around, and if we're in a transaction, and we get a retryable error, we already retry the entire transaction. So if we add ABORTED to our list retryable codes, we'll be following the semantics of ABORTED when we're in a transaction. If we're not in a transaction, then we shouldn't see it, would be my guess, so having it in the list shouldn't make any difference one way or the other for non-transactions. Does that seem reasonable?

Andrew Gorcester, Yesterday 4:34 PM

hmm... I think so, yeah.

Copy link
Copy Markdown
Contributor

@cguardia cguardia left a comment

Choose a reason for hiding this comment

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

Looks good.

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

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"too much contention" server errors should be retried

3 participants