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

Indexing a document fails when setting version=0 & version_type=external #5662

Closed
bleskes opened this issue Apr 2, 2014 · 5 comments
Closed
Assignees
Labels
blocker :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >regression v1.2.0 v2.0.0-beta1

Comments

@bleskes
Copy link
Contributor

bleskes commented Apr 2, 2014

This is a regression introduced in 4e0e406 , which was release with 1.1

@bleskes bleskes self-assigned this Apr 2, 2014
@pickypg
Copy link
Member

pickypg commented Apr 4, 2014

This is a regression added here, but it was the already documented behavior in 0.90:

To enable this functionality, version_type should be set to external. The value provided must be a numeric, long value greater than 0, and less than around 9.2e+18.

Personally, I think that it is an improvement, particularly because it matches the existing documentation and it makes sense to start at > 0.

@amitsoni13
Copy link

Hi - Just to add, I think it would be incorrect to assume that the version numbers would always be positive. In our system, the first version number is zero and hence 1.1 failed for me as soon as we deployed and we had to revert.

@bleskes
Copy link
Contributor Author

bleskes commented Apr 16, 2014

@amitsoni13 I agree.

@pickypg Although it was documented we didn't enforce it on the 0.90 and 1.0 so people could in practice index document with version 0 so they now have it in their index. I will be working to make 0 a valid external version.

@pickypg
Copy link
Member

pickypg commented Apr 16, 2014

@bleskes Sounds good to me. 0 makes just as much sense to developers.

I'm curious though, are you going to make 0 the default starting value even without anything specified, or will that stay as 1?

@bleskes
Copy link
Contributor Author

bleskes commented Apr 16, 2014

@pickypg I believe you refer to the internal version type where ES is in charge of managing versions - this will not change at it will start at 1. This discussion only relates to the external version type.

@bleskes bleskes removed the bug label Apr 18, 2014
@s1monw s1monw added the blocker label Apr 24, 2014
bleskes added a commit to bleskes/elasticsearch that referenced this issue May 13, 2014
Until now all version types have officially required the version to be a positive long number. Despite of this has being documented, ES versions <=1.0 did not enforce it when using the `external` version type. As a result people have succesfully indexed documents with 0 as a version. In 1.1. we introduced validation checks on incoming version values and causing indexing request to fail if the version was set to 0. While this is strictly speaking OK, we effectively have a situation where data already indexed does not match the version invariant.

To be lenient and adhere to spirit of our data backward compatibility policy, we have decided to allow 0 as a valid external version type. This is somewhat complicated as 0 is also the internal value of `MATCH_ANY`, which indicates requests should succeed regardles off the current doc version. To keep things simple, this commit changes the internal value of `MATCH_ANY` to `-3` for all version types.

Since we're doing this in a minor release (and because versions are stored in the transaction log), the default `internal` version type still accepts 0 as a `MATCH_ANY` value. This is not a problem for other version types as `MATCH_ANY` doesn't make sense in that context.

Closes elastic#5662
bleskes added a commit that referenced this issue May 16, 2014
Until now all version types have officially required the version to be a positive long number. Despite of this has being documented, ES versions <=1.0 did not enforce it when using the `external` version type. As a result people have succesfully indexed documents with 0 as a version. In 1.1. we introduced validation checks on incoming version values and causing indexing request to fail if the version was set to 0. While this is strictly speaking OK, we effectively have a situation where data already indexed does not match the version invariant.

To be lenient and adhere to spirit of our data backward compatibility policy, we have decided to allow 0 as a valid external version type. This is somewhat complicated as 0 is also the internal value of `MATCH_ANY`, which indicates requests should succeed regardles off the current doc version. To keep things simple, this commit changes the internal value of `MATCH_ANY` to `-3` for all version types.

Since we're doing this in a minor release (and because versions are stored in the transaction log), the default `internal` version type still accepts 0 as a `MATCH_ANY` value. This is not a problem for other version types as `MATCH_ANY` doesn't make sense in that context.

Closes #5662
@clintongormley clintongormley added the :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. label Jun 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >regression v1.2.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants