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

Calling the Update API using EXTERNAL(_GTE) version type should throw a validation error #5661

Closed
bleskes opened this issue Apr 2, 2014 · 9 comments

Comments

@bleskes
Copy link
Contributor

bleskes commented Apr 2, 2014

The update API is utility to do a get-change-and-index cycle while guarantying the document didn't change between the get and the index phase of the operation. We do so using Elasticsearch's versioning system which allows you to fail an indexing request if the document doesn't have the expected version (i.e., changed).

At the moment, using the EXTERNAL or EXTERNAL_GTE version types break this guaranties. These should be disabled. In the future we can extend the index API to allow supplying both an expected exiting version and a version to be index at which point we could have a proper support in the update API.

@bleskes bleskes self-assigned this Apr 2, 2014
bleskes added a commit to bleskes/elasticsearch that referenced this issue Apr 24, 2014
…n read operation & disallow them in the Update API

Separate version check logic for reads and writes for all version types, which allows different behavior in these cases.
Change `VersionType.EXTERNAL` & `VersionType.EXTERNAL_GTE` to behave the same as `VersionType.INTERNAL` for read operations.
The previous behavior was fit for writes but is useless in reads.

This commit also makes the usage of `EXTERNAL` & `EXTERNAL_GTE` in the update api raise a validation error as it make cause data to
be lost.

Closes elastic#5663 , closes elastic#5661
@s1monw s1monw added the blocker label Apr 24, 2014
bleskes added a commit that referenced this issue Apr 25, 2014
…n read operation & disallow them in the Update API

Separate version check logic for reads and writes for all version types, which allows different behavior in these cases.
Change `VersionType.EXTERNAL` & `VersionType.EXTERNAL_GTE` to behave the same as `VersionType.INTERNAL` for read operations.
The previous behavior was fit for writes but is useless in reads.

This commit also makes the usage of `EXTERNAL` & `EXTERNAL_GTE` in the update api raise a validation error as it make cause data to
be lost.

Closes #5663 , Closes #5661, Closes #5929
@mjgp2
Copy link

mjgp2 commented Nov 10, 2014

So you have removed the ability to update only if the current version matches a specific (external) version?

@bleskes
Copy link
Contributor Author

bleskes commented Nov 10, 2014

@mjgp2 well, you can abuse it that way. The thing is that if external version semantics (update if version is lower then X) can not guarantee we don't override changes made by a concurrent indexing/update commands.

@mjgp2
Copy link

mjgp2 commented Nov 10, 2014

@bleskes could you point me at where using a generated id vs using an externally generated id differs here in the code? I'm trying to understand exactly what the limitation is to work out how easy it is to fix.

@mjgp2
Copy link

mjgp2 commented Nov 10, 2014

Oh ok so it should be fairly simple to reintroduce using external ids then, as in there's no impassable barrier here by the looks of it? :)

@bleskes
Copy link
Contributor Author

bleskes commented Nov 11, 2014

Well, as I said , the problem is an algorithmic one - the mixture of external version semantics during the get process (exact match) and indexing process (local indexed version must be lower then the once supplied) that don’t play well together to guaranty no data is lost during concurrent indexing. In general the external version is designed to be used with an external source of truth where the update will be done and sent to ES as an indexing requests.

On Tue, Nov 11, 2014 at 12:12 AM, Matthew Painter
notifications@github.com wrote:

Oh ok so it should be fairly simple to reintroduce using external ids then, as in there's no impassable barrier here by the looks of it? :)

Reply to this email directly or view it on GitHub:
#5661 (comment)

@cvasii
Copy link

cvasii commented Nov 24, 2014

But if I use version type as "force", then it should be the same thing as using version type external in pre 1.2 releases?

Actually, is not. No VersionConflictException will be thrown in this case. Ignore my question

@TronPaul
Copy link

Please update the documentation surrounding external versioning to indicate that you shouldn't use the Update API with external versioning.

http://www.elasticsearch.org/guide/en/elasticsearch/guide/current/optimistic-concurrency-control.html
http://www.elasticsearch.org/blog/elasticsearch-versioning-support/

Was switching to external versioning following the above links and was confused by the errors I was getting until running into this issue.

@clintongormley
Copy link

@TronPaul the update docs already specify that external versioning is not supported: http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/docs-update.html#_parameters_3

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