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

We should not retry ABORTED transactions #1230

Closed
mziccard opened this issue Sep 8, 2016 · 7 comments
Closed

We should not retry ABORTED transactions #1230

mziccard opened this issue Sep 8, 2016 · 7 comments
Assignees
Labels
api: datastore Issues related to the Datastore API.

Comments

@mziccard
Copy link
Contributor

mziccard commented Sep 8, 2016

In datastore we have ABORTED among retryable exceptions. This causes commit requests failed due to contention to be retried. Retrying an ABORTED transaction always fails with INVALID ARGUMENT error.

I suggest we never retry ABORTED. \cc @aozarov @ajkannan

@mziccard mziccard added the api: datastore Issues related to the Datastore API. label Sep 8, 2016
@ajkannan
Copy link

ajkannan commented Sep 9, 2016

Just to make sure I understand, when you said "Retrying an aborted transaction", you meant any Datastore rpc rather than committing a Datastore Transaction specifically, correct? Since the Datastore docs seem to suggest ABORTED is a retryable exception, I wouldn't necessarily expect the non-retryable error code INVALID_ARGUMENT to be returned when retrying (especially if the original error is due to contention). Is there any more information included in the INVALID ARGUMENT error message?

@aozarov
Copy link
Contributor

aozarov commented Sep 9, 2016

I agree with @ajkannan. AFAIK contention should not invalidate a transaction.
/cc @eddavisson is that no longer (or never was) the case?

@eddavisson
Copy link

Contention has to invalidate a transaction, otherwise we can't guarantee serializability. In this scenario:

t1 = datastore.beginTransaction()
entity = t1.lookup(key)
datastore.commit(entity.set('foo', 'd')) // not in transaction
t1.commit(entity.set('foo', 't'))

The commit can't be retried because the value returned by the lookup call was invalidated by the commit from outside the transaction. You have to start over.

You could, however, retry a NON_TRANSACTIONAL commit (i.e. a blind write) if it fails with ABORTED.

This sounds like a documentation bug to me?

@mziccard
Copy link
Contributor Author

mziccard commented Sep 9, 2016

This sounds like a documentation bug to me?

Yes, docs should make clear that ABORTED commit requests should only be retried when they are NON_TRANSACTIONAL. Otherwise, the whole transaction should be retried.

@aozarov
Copy link
Contributor

aozarov commented Sep 9, 2016

Ah, right. so I guess ABORTED is like getting java.util.ConcurrentModificationException in the AE datastore Java API. @eddavisson why/in-what-case would you propagate such an error for non transnational commits? Indeed, doc is not clear and I think should make the distinction between transnational to non transnational when it comes to its retry suggestion.

@eddavisson
Copy link

The NON_TRANSACTIONAL commit still does a read/write "transaction" internally. If an entity group changes between the read and write, then the commit fails with ABORTED. Entity groups that are seeing a lot of contention can still hit this.

One final note about the retryability of TRANSACTIONAL commits: you can retry the entire operation (starting with the beginTransaction() call). That's what, for example, the @ndb.transactional decorator does (https://cloud.google.com/appengine/docs/python/ndb/transactions).

I'll work on getting our documentation fixed.

@WalterHub
Copy link
Contributor

Thank you for the doc feedback. A fix has been published to https://cloud.google.com/datastore/docs/concepts/errors.

github-actions bot pushed a commit to suztomo/google-cloud-java that referenced this issue Jul 1, 2022
🤖 I have created a release *beep* *boop*
---


## [3.4.0](googleapis/java-asset@v3.3.1...v3.4.0) (2022-06-30)


### Features

* Enable REST transport for most of Java and Go clients ([googleapis#1242](googleapis/java-asset#1242)) ([86eb248](googleapis/java-asset@86eb248))


### Bug Fixes

* update gapic-generator-java with mock service generation fixes ([googleapis#1249](googleapis/java-asset#1249)) ([7d124ad](googleapis/java-asset@7d124ad))


### Dependencies

* update dependency com.google.api.grpc:proto-google-cloud-orgpolicy-v1 to v2.2.1 ([googleapis#1245](googleapis/java-asset#1245)) ([9b0d4e9](googleapis/java-asset@9b0d4e9))
* update dependency com.google.api.grpc:proto-google-cloud-pubsub-v1 to v1.101.0 ([googleapis#1228](googleapis/java-asset#1228)) ([c5f1712](googleapis/java-asset@c5f1712))
* update dependency com.google.api.grpc:proto-google-cloud-pubsub-v1 to v1.101.1 ([googleapis#1233](googleapis/java-asset#1233)) ([42031ef](googleapis/java-asset@42031ef))
* update dependency com.google.api.grpc:proto-google-identity-accesscontextmanager-v1 to v1.3.1 ([googleapis#1246](googleapis/java-asset#1246)) ([d868803](googleapis/java-asset@d868803))
* update dependency com.google.cloud:google-cloud-bigquery to v2.13.1 ([googleapis#1227](googleapis/java-asset#1227)) ([113c52f](googleapis/java-asset@113c52f))
* update dependency com.google.cloud:google-cloud-bigquery to v2.13.3 ([googleapis#1234](googleapis/java-asset#1234)) ([7006c44](googleapis/java-asset@7006c44))
* update dependency com.google.cloud:google-cloud-bigquery to v2.13.4 ([googleapis#1239](googleapis/java-asset#1239)) ([9c59632](googleapis/java-asset@9c59632))
* update dependency com.google.cloud:google-cloud-bigquery to v2.13.6 ([googleapis#1244](googleapis/java-asset#1244)) ([a305745](googleapis/java-asset@a305745))
* update dependency com.google.cloud:google-cloud-core to v2.8.0 ([googleapis#1238](googleapis/java-asset#1238)) ([8224b02](googleapis/java-asset@8224b02))
* update dependency com.google.cloud:google-cloud-resourcemanager to v1.4.0 ([googleapis#1230](googleapis/java-asset#1230)) ([5655582](googleapis/java-asset@5655582))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v2.13.0 ([googleapis#1241](googleapis/java-asset#1241)) ([2ebe221](googleapis/java-asset@2ebe221))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
github-actions bot pushed a commit that referenced this issue Sep 15, 2022
🤖 I have created a release *beep* *boop*
---


### Updating meta-information for bleeding-edge SNAPSHOT release.

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the Datastore API.
Projects
None yet
Development

No branches or pull requests

5 participants