Allow 0 as a valid external version

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

Author: bleskes

docs/reference/docs/index_.asciidoc

@@ -109,7 +109,7 @@ By default, internal versioning is used that starts at 1 and increments
 with each update, deletes included. Optionally, the version number can be
 supplemented with an external value (for example, if maintained in a
 database). To enable this functionality, `version_type` should be set to
-`external`. The value provided must be a numeric, long value greater than 0,
+`external`. The value provided must be a numeric, long value greater or equal to 0,
 and less than around 9.2e+18. When using the external version type, instead
 of checking for a matching version number, the system checks to see if
 the version number passed to the index request is greater than the
@@ -138,12 +138,13 @@ of the stored document.
 
 `external` or `external_gt`:: only index the document if the given version is strictly higher
 than the version of the stored document *or* if there is no existing document. The given
-version will be used as the new version and will be stored with the new document.
+version will be used as the new version and will be stored with the new document. The supplied
+version must be a non-negative long number.
 
 `external_gte`:: only index the document if the given version is *equal* or higher
 than the version of the stored document. If there is no existing document
 the operation will succeed as well. The given version will be used as the new version
-and will be stored with the new document.
+and will be stored with the new document. The supplied version must be a non-negative long number.
 
 `force`:: the document will be indexed regardless of the version of the stored document or if there
 is no existing document. The given version will be used as the new version and will be stored

rest-api-spec/test/index/35_external_version.yaml

@@ -1,6 +1,17 @@
 ---
 "External version":
 
+ - do:
+      index:
+          index:          test_1
+          type:           test
+          id:             1
+          body:           { foo: bar }
+          version_type:   external
+          version:        0
+
+ - match:   { _version: 0 }
+
  - do:
       index:
           index:          test_1
@@ -10,7 +21,7 @@
           version_type:   external
           version:        5
 
- - match:   { _version: 5}
+ - match:   { _version: 5 }
 
  - do:
       catch:             conflict
@@ -22,6 +33,16 @@
           version_type:   external
           version:        5
 
+ - do:
+      catch:             conflict
+      index:
+          index:          test_1
+          type:           test
+          id:             1
+          body:           { foo: bar }
+          version_type:   external
+          version:        0
+
  - do:
       index:
           index:          test_1

rest-api-spec/test/index/36_external_gte_version.yaml

@@ -1,6 +1,17 @@
 ---
 "External GTE version":
 
+ - do:
+      index:
+          index:          test_1
+          type:           test
+          id:             1
+          body:           { foo: bar }
+          version_type:   external_gte
+          version:        0
+
+ - match:   { _version: 0}
+
  - do:
       index:
           index:          test_1
@@ -20,7 +31,7 @@
           id:             1
           body:           { foo: bar }
           version_type:   external_gte
-          version:        4
+          version:        0
 
  - do:
       index:

src/main/java/org/elasticsearch/action/delete/DeleteRequest.java

@@ -24,6 +24,7 @@
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.lucene.uid.Versions;
 import org.elasticsearch.index.VersionType;
 
 import java.io.IOException;
@@ -205,7 +206,7 @@ public void readFrom(StreamInput in) throws IOException {
         id = in.readString();
         routing = in.readOptionalString();
         refresh = in.readBoolean();
-        version = in.readLong();
+        version = Versions.readVersion(in);
         versionType = VersionType.fromValue(in.readByte());
     }
 
@@ -216,7 +217,7 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeString(id);
         out.writeOptionalString(routing());
         out.writeBoolean(refresh);
-        out.writeLong(version);
+        Versions.writeVersion(version, out);
         out.writeByte(versionType.getValue());
     }
 

src/main/java/org/elasticsearch/action/delete/TransportDeleteAction.java

@@ -112,7 +112,7 @@ protected boolean resolveRequest(final ClusterState state, final DeleteRequest r
                     if (request.versionType() != VersionType.INTERNAL) {
                         // TODO: implement this feature
                         throw new ElasticsearchIllegalArgumentException("routing value is required for deleting documents of type [" + request.type()
-                                + "] while using version_type [" + request.versionType());
+                                + "] while using version_type [" + request.versionType() + "]");
                     }
                     indexDeleteAction.execute(new IndexDeleteRequest(request), new ActionListener<IndexDeleteResponse>() {
                         @Override

src/main/java/org/elasticsearch/action/delete/index/IndexDeleteRequest.java

@@ -23,6 +23,7 @@
 import org.elasticsearch.action.support.replication.IndexReplicationOperationRequest;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.lucene.uid.Versions;
 
 import java.io.IOException;
 
@@ -72,7 +73,7 @@ public void readFrom(StreamInput in) throws IOException {
         type = in.readString();
         id = in.readString();
         refresh = in.readBoolean();
-        version = in.readLong();
+        version = Versions.readVersion(in);
     }
 
     @Override
@@ -81,6 +82,6 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeString(type);
         out.writeString(id);
         out.writeBoolean(refresh);
-        out.writeLong(version);
+        Versions.writeVersion(version, out);
     }
 }

src/main/java/org/elasticsearch/action/delete/index/ShardDeleteRequest.java

@@ -23,6 +23,7 @@
 import org.elasticsearch.action.support.replication.ShardReplicationOperationRequest;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.lucene.uid.Versions;
 
 import java.io.IOException;
 
@@ -98,7 +99,7 @@ public void readFrom(StreamInput in) throws IOException {
         type = in.readString();
         id = in.readString();
         refresh = in.readBoolean();
-        version = in.readLong();
+        version = Versions.readVersion(in);
     }
 
     @Override
@@ -108,6 +109,6 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeString(type);
         out.writeString(id);
         out.writeBoolean(refresh);
-        out.writeLong(version);
+        Versions.writeVersion(version, out);
     }
 }

src/main/java/org/elasticsearch/action/get/GetRequest.java

@@ -267,7 +267,7 @@ public void readFrom(StreamInput in) throws IOException {
         }
 
         this.versionType = VersionType.fromValue(in.readByte());
-        this.version = in.readVLong();
+        this.version = Versions.readVersionWithVLongForBW(in);
 
         fetchSourceContext = FetchSourceContext.optionalReadFromStream(in);
     }
@@ -298,7 +298,7 @@ public void writeTo(StreamOutput out) throws IOException {
         }
 
         out.writeByte(versionType.getValue());
-        out.writeVLong(version);
+        Versions.writeVersionWithVLongForBW(version, out);
 
         FetchSourceContext.optionalWriteToStream(fetchSourceContext, out);
     }

src/main/java/org/elasticsearch/action/get/MultiGetRequest.java

@@ -169,7 +169,7 @@ public void readFrom(StreamInput in) throws IOException {
                     fields[i] = in.readString();
                 }
             }
-            version = in.readVLong();
+            version = Versions.readVersionWithVLongForBW(in);
             versionType = VersionType.fromValue(in.readByte());
 
             fetchSourceContext = FetchSourceContext.optionalReadFromStream(in);
@@ -190,7 +190,7 @@ public void writeTo(StreamOutput out) throws IOException {
                 }
             }
 
-            out.writeVLong(version);
+            Versions.writeVersionWithVLongForBW(version, out);
             out.writeByte(versionType.getValue());
 
             FetchSourceContext.optionalWriteToStream(fetchSourceContext, out);

src/main/java/org/elasticsearch/action/get/MultiGetShardRequest.java

@@ -25,6 +25,7 @@
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.lucene.uid.Versions;
 import org.elasticsearch.index.VersionType;
 import org.elasticsearch.search.fetch.source.FetchSourceContext;
 
@@ -138,7 +139,7 @@ public void readFrom(StreamInput in) throws IOException {
             } else {
                 fields.add(null);
             }
-            versions.add(in.readVLong());
+            versions.add(Versions.readVersionWithVLongForBW(in));
             versionTypes.add(VersionType.fromValue(in.readByte()));
 
             fetchSourceContexts.add(FetchSourceContext.optionalReadFromStream(in));
@@ -175,7 +176,7 @@ public void writeTo(StreamOutput out) throws IOException {
                     out.writeString(field);
                 }
             }
-            out.writeVLong(versions.get(i));
+            Versions.writeVersionWithVLongForBW(versions.get(i), out);
             out.writeByte(versionTypes.get(i).getValue());
             FetchSourceContext fetchSourceContext = fetchSourceContexts.get(i);
             FetchSourceContext.optionalWriteToStream(fetchSourceContext, out);

src/main/java/org/elasticsearch/action/index/IndexRequest.java

@@ -632,7 +632,7 @@ public void readFrom(StreamInput in) throws IOException {
 
         opType = OpType.fromId(in.readByte());
         refresh = in.readBoolean();
-        version = in.readLong();
+        version = Versions.readVersion(in);
         versionType = VersionType.fromValue(in.readByte());
         if (in.getVersion().onOrAfter(Version.V_1_2_0)) {
             autoGeneratedId = in.readBoolean();
@@ -651,7 +651,7 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeBytesReference(source);
         out.writeByte(opType.id());
         out.writeBoolean(refresh);
-        out.writeLong(version);
+        Versions.writeVersion(version, out);
         out.writeByte(versionType.getValue());
         if (out.getVersion().onOrAfter(Version.V_1_2_0)) {
             out.writeBoolean(autoGeneratedId);

src/main/java/org/elasticsearch/action/update/UpdateRequest.java

@@ -609,7 +609,7 @@ public void readFrom(StreamInput in) throws IOException {
             upsertRequest.readFrom(in);
         }
         docAsUpsert = in.readBoolean();
-        version = in.readLong();
+        version = Versions.readVersion(in);
         versionType = VersionType.fromValue(in.readByte());
     }
 
@@ -655,7 +655,7 @@ public void writeTo(StreamOutput out) throws IOException {
             upsertRequest.writeTo(out);
         }
         out.writeBoolean(docAsUpsert);
-        out.writeLong(version);
+        Versions.writeVersion(version, out);
         out.writeByte(versionType.getValue());
     }
 

src/main/java/org/elasticsearch/common/lucene/uid/Versions.java

@@ -22,7 +22,10 @@
 import org.apache.lucene.index.*;
 import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.Numbers;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.index.mapper.internal.UidFieldMapper;
 import org.elasticsearch.index.mapper.internal.VersionFieldMapper;
 
@@ -32,11 +35,55 @@
 /** Utility class to resolve the Lucene doc ID and version for a given uid. */
 public class Versions {
 
-    public static final long MATCH_ANY = 0L; // Version was not specified by the user
+    public static final long MATCH_ANY = -3L; // Version was not specified by the user
+    // the value for MATCH_ANY before ES 1.2.0 - will be removed
+    public static final long MATCH_ANY_PRE_1_2_0 = 0L;
     public static final long NOT_FOUND = -1L;
     public static final long NOT_SET = -2L;
 
-    private Versions() {}
+    public static void writeVersion(long version, StreamOutput out) throws IOException {
+        if (out.getVersion().before(Version.V_1_2_0) && version == MATCH_ANY) {
+            // we have to send out a value the node will understand
+            version = MATCH_ANY_PRE_1_2_0;
+        }
+        out.writeLong(version);
+    }
+
+    public static long readVersion(StreamInput in) throws IOException {
+        long version = in.readLong();
+        if (in.getVersion().before(Version.V_1_2_0) && version == MATCH_ANY_PRE_1_2_0) {
+            version = MATCH_ANY;
+        }
+        return version;
+    }
+
+    public static void writeVersionWithVLongForBW(long version, StreamOutput out) throws IOException {
+        if (out.getVersion().onOrAfter(Version.V_1_2_0)) {
+            out.writeLong(version);
+            return;
+        }
+
+        if (version == MATCH_ANY) {
+            // we have to send out a value the node will understand
+            version = MATCH_ANY_PRE_1_2_0;
+        }
+        out.writeVLong(version);
+    }
+
+    public static long readVersionWithVLongForBW(StreamInput in) throws IOException {
+        if (in.getVersion().onOrAfter(Version.V_1_2_0)) {
+            return in.readLong();
+        } else {
+            long version = in.readVLong();
+            if (version == MATCH_ANY_PRE_1_2_0) {
+                return MATCH_ANY;
+            }
+            return version;
+        }
+    }
+
+    private Versions() {
+    }
 
     /** Wraps an {@link AtomicReaderContext}, a doc ID <b>relative to the context doc base</b> and a version. */
     public static class DocIdAndVersion {