Jelajahi Sumber

Remove FORCE version_type

This was an error-prone version type that allowed overriding previous
version semantics. It could cause primaries and replicas to be out of
sync however, so it has been removed.

Resolves #19769
Lee Hinman 9 tahun lalu
induk
melakukan
49695af2ac

+ 1 - 6
core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java

@@ -139,12 +139,7 @@ public class UpdateHelper extends AbstractComponent {
             return new Result(indexRequest, DocWriteResponse.Result.CREATED, null, null);
         }
 
-        long updateVersion = getResult.getVersion();
-
-        if (request.versionType() != VersionType.INTERNAL) {
-            assert request.versionType() == VersionType.FORCE;
-            updateVersion = request.version(); // remember, match_any is excluded by the conflict test
-        }
+        final long updateVersion = getResult.getVersion();
 
         if (getResult.internalSourceRef() == null) {
             // no source, we can't do nothing, through a failure...

+ 3 - 2
core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java

@@ -106,8 +106,9 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
             validationException = addValidationError("id is missing", validationException);
         }
 
-        if (!(versionType == VersionType.INTERNAL || versionType == VersionType.FORCE)) {
-            validationException = addValidationError("version type [" + versionType + "] is not supported by the update API", validationException);
+        if (versionType != VersionType.INTERNAL) {
+            validationException = addValidationError("version type [" + versionType + "] is not supported by the update API",
+                    validationException);
         } else {
 
             if (version != Versions.MATCH_ANY && retryOnConflict > 0) {

+ 0 - 50
core/src/main/java/org/elasticsearch/index/VersionType.java

@@ -198,52 +198,6 @@ public enum VersionType implements Writeable {
             return version >= 0L || version == Versions.MATCH_ANY;
         }
 
-    },
-    /**
-     * Warning: this version type should be used with care. Concurrent indexing may result in loss of data on replicas
-     */
-    FORCE((byte) 3) {
-        @Override
-        public boolean isVersionConflictForWrites(long currentVersion, long expectedVersion, boolean deleted) {
-            if (currentVersion == Versions.NOT_FOUND) {
-                return false;
-            }
-            if (expectedVersion == Versions.MATCH_ANY) {
-                throw new IllegalStateException("you must specify a version when use VersionType.FORCE");
-            }
-            return false;
-        }
-
-        @Override
-        public String explainConflictForWrites(long currentVersion, long expectedVersion, boolean deleted) {
-            throw new AssertionError("VersionType.FORCE should never result in a write conflict");
-        }
-
-        @Override
-        public boolean isVersionConflictForReads(long currentVersion, long expectedVersion) {
-            return false;
-        }
-
-        @Override
-        public String explainConflictForReads(long currentVersion, long expectedVersion) {
-            throw new AssertionError("VersionType.FORCE should never result in a read conflict");
-        }
-
-        @Override
-        public long updateVersion(long currentVersion, long expectedVersion) {
-            return expectedVersion;
-        }
-
-        @Override
-        public boolean validateVersionForWrites(long version) {
-            return version >= 0L;
-        }
-
-        @Override
-        public boolean validateVersionForReads(long version) {
-            return version >= 0L || version == Versions.MATCH_ANY;
-        }
-
     };
 
     private final byte value;
@@ -337,8 +291,6 @@ public enum VersionType implements Writeable {
             return EXTERNAL;
         } else if ("external_gte".equals(versionType)) {
             return EXTERNAL_GTE;
-        } else if ("force".equals(versionType)) {
-            return FORCE;
         }
         throw new IllegalArgumentException("No version type match [" + versionType + "]");
     }
@@ -357,8 +309,6 @@ public enum VersionType implements Writeable {
             return EXTERNAL;
         } else if (value == 2) {
             return EXTERNAL_GTE;
-        } else if (value == 3) {
-            return FORCE;
         }
         throw new IllegalArgumentException("No version type match [" + value + "]");
     }

+ 2 - 5
core/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java

@@ -235,14 +235,11 @@ public class BulkWithUpdatesIT extends ESIntegTestCase {
                 .add(client().prepareUpdate("test", "type", "e1")
                         .setDoc("field", "2").setVersion(10)) // INTERNAL
                 .add(client().prepareUpdate("test", "type", "e1")
-                        .setDoc("field", "3").setVersion(20).setVersionType(VersionType.FORCE))
-                .add(client().prepareUpdate("test", "type", "e1")
-                        .setDoc("field", "4").setVersion(20).setVersionType(VersionType.INTERNAL))
+                        .setDoc("field", "3").setVersion(13).setVersionType(VersionType.INTERNAL))
                 .get();
 
         assertThat(bulkResponse.getItems()[0].getFailureMessage(), containsString("version conflict"));
-        assertThat(bulkResponse.getItems()[1].getResponse().getVersion(), equalTo(20L));
-        assertThat(bulkResponse.getItems()[2].getResponse().getVersion(), equalTo(21L));
+        assertThat(bulkResponse.getItems()[1].getFailureMessage(), containsString("version conflict"));
     }
 
     public void testBulkUpdateMalformedScripts() throws Exception {

+ 0 - 40
core/src/test/java/org/elasticsearch/index/VersionTypeTests.java

@@ -77,13 +77,6 @@ public class VersionTypeTests extends ESTestCase {
         assertTrue(VersionType.EXTERNAL_GTE.validateVersionForReads(randomIntBetween(1, Integer.MAX_VALUE)));
         assertFalse(VersionType.EXTERNAL_GTE.validateVersionForReads(randomIntBetween(Integer.MIN_VALUE, -1)));
 
-        assertTrue(VersionType.FORCE.validateVersionForWrites(randomIntBetween(1, Integer.MAX_VALUE)));
-        assertFalse(VersionType.FORCE.validateVersionForWrites(Versions.MATCH_ANY));
-        assertFalse(VersionType.FORCE.validateVersionForWrites(randomIntBetween(Integer.MIN_VALUE, 0)));
-        assertTrue(VersionType.FORCE.validateVersionForReads(Versions.MATCH_ANY));
-        assertTrue(VersionType.FORCE.validateVersionForReads(randomIntBetween(1, Integer.MAX_VALUE)));
-        assertFalse(VersionType.FORCE.validateVersionForReads(randomIntBetween(Integer.MIN_VALUE, -1)));
-
         assertTrue(VersionType.INTERNAL.validateVersionForWrites(randomIntBetween(1, Integer.MAX_VALUE)));
         assertTrue(VersionType.INTERNAL.validateVersionForWrites(Versions.MATCH_ANY));
         assertFalse(VersionType.INTERNAL.validateVersionForWrites(randomIntBetween(Integer.MIN_VALUE, 0)));
@@ -153,36 +146,6 @@ public class VersionTypeTests extends ESTestCase {
 
     }
 
-    public void testForceVersionConflict() throws Exception {
-        assertFalse(VersionType.FORCE.isVersionConflictForWrites(Versions.NOT_FOUND, 10, randomBoolean()));
-
-        // MATCH_ANY must throw an exception in the case of force version, as the version must be set! it used as the new value
-        try {
-            VersionType.FORCE.isVersionConflictForWrites(10, Versions.MATCH_ANY, randomBoolean());
-            fail();
-        } catch (IllegalStateException e) {
-            //yes!!
-        }
-
-        // if we didn't find a version (but the index does support it), we always accept
-        assertFalse(VersionType.FORCE.isVersionConflictForWrites(Versions.NOT_FOUND, Versions.NOT_FOUND, randomBoolean()));
-        assertFalse(VersionType.FORCE.isVersionConflictForWrites(Versions.NOT_FOUND, 10, randomBoolean()));
-
-        assertFalse(VersionType.FORCE.isVersionConflictForReads(Versions.NOT_FOUND, Versions.NOT_FOUND));
-        assertFalse(VersionType.FORCE.isVersionConflictForReads(Versions.NOT_FOUND, 10));
-        assertFalse(VersionType.FORCE.isVersionConflictForReads(Versions.NOT_FOUND, Versions.MATCH_ANY));
-
-
-        // and the standard behavior
-        assertFalse(VersionType.FORCE.isVersionConflictForWrites(10, 10, randomBoolean()));
-        assertFalse(VersionType.FORCE.isVersionConflictForWrites(9, 10, randomBoolean()));
-        assertFalse(VersionType.FORCE.isVersionConflictForWrites(10, 9, randomBoolean()));
-        assertFalse(VersionType.FORCE.isVersionConflictForReads(10, 10));
-        assertFalse(VersionType.FORCE.isVersionConflictForReads(9, 10));
-        assertFalse(VersionType.FORCE.isVersionConflictForReads(10, 9));
-        assertFalse(VersionType.FORCE.isVersionConflictForReads(10, Versions.MATCH_ANY));
-    }
-
     public void testUpdateVersion() {
         assertThat(VersionType.INTERNAL.updateVersion(Versions.NOT_FOUND, 10), equalTo(1L));
         assertThat(VersionType.INTERNAL.updateVersion(1, 1), equalTo(2L));
@@ -196,9 +159,6 @@ public class VersionTypeTests extends ESTestCase {
         assertThat(VersionType.EXTERNAL_GTE.updateVersion(1, 10), equalTo(10L));
         assertThat(VersionType.EXTERNAL_GTE.updateVersion(10, 10), equalTo(10L));
 
-        assertThat(VersionType.FORCE.updateVersion(Versions.NOT_FOUND, 10), equalTo(10L));
-        assertThat(VersionType.FORCE.updateVersion(11, 10), equalTo(10L));
-
 // Old indexing code
 //        if (index.versionType() == VersionType.INTERNAL) { // internal version type
 //            updatedVersion = (currentVersion == Versions.NOT_SET || currentVersion == Versions.NOT_FOUND) ? 1 : currentVersion + 1;

+ 1 - 7
core/src/test/java/org/elasticsearch/update/UpdateIT.java

@@ -527,15 +527,9 @@ public class UpdateIT extends ESIntegTestCase {
                         .setVersionType(VersionType.EXTERNAL).execute(),
                 ActionRequestValidationException.class);
 
-
-        // With force version
-        client().prepareUpdate(indexOrAlias(), "type", "2")
-                .setScript(new Script("", ScriptService.ScriptType.INLINE,  "put_values", Collections.singletonMap("text", "v10")))
-                .setVersion(10).setVersionType(VersionType.FORCE).get();
-
         GetResponse get = get("test", "type", "2");
         assertThat(get.getVersion(), equalTo(10L));
-        assertThat((String) get.getSource().get("text"), equalTo("v10"));
+        assertThat((String) get.getSource().get("text"), equalTo("value"));
 
         // upserts - the combination with versions is a bit weird. Test are here to ensure we do not change our behavior unintentionally
 

+ 0 - 30
core/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java

@@ -72,36 +72,6 @@ public class SimpleVersioningIT extends ESIntegTestCase {
         assertThat(indexResponse.getVersion(), equalTo(18L));
     }
 
-    public void testForce() throws Exception {
-        createIndex("test");
-        ensureGreen("test"); // we are testing force here which doesn't work if we are recovering at the same time - zzzzz...
-        IndexResponse indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_1").setVersion(12).setVersionType(VersionType.FORCE).get();
-        assertThat(indexResponse.getVersion(), equalTo(12L));
-
-        indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_2").setVersion(12).setVersionType(VersionType.FORCE).get();
-        assertThat(indexResponse.getVersion(), equalTo(12L));
-
-        indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_2").setVersion(14).setVersionType(VersionType.FORCE).get();
-        assertThat(indexResponse.getVersion(), equalTo(14L));
-
-        indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_1").setVersion(13).setVersionType(VersionType.FORCE).get();
-        assertThat(indexResponse.getVersion(), equalTo(13L));
-
-        client().admin().indices().prepareRefresh().execute().actionGet();
-        if (randomBoolean()) {
-            refresh();
-        }
-        for (int i = 0; i < 10; i++) {
-            assertThat(client().prepareGet("test", "type", "1").get().getVersion(), equalTo(13L));
-        }
-
-        // deleting with a lower version works.
-        long v = randomIntBetween(12, 14);
-        DeleteResponse deleteResponse = client().prepareDelete("test", "type", "1").setVersion(v).setVersionType(VersionType.FORCE).get();
-        assertEquals(DocWriteResponse.Result.DELETED, deleteResponse.getResult());
-        assertThat(deleteResponse.getVersion(), equalTo(v));
-    }
-
     public void testExternalGTE() throws Exception {
         createIndex("test");
 

+ 2 - 0
docs/reference/migration/migrate_6_0.asciidoc

@@ -33,3 +33,5 @@ include::migrate_6_0/mapping.asciidoc[]
 include::migrate_6_0/rest.asciidoc[]
 
 include::migrate_6_0/search.asciidoc[]
+
+include::migrate_6_0/docs.asciidoc[]

+ 7 - 0
docs/reference/migration/migrate_6_0/docs.asciidoc

@@ -0,0 +1,7 @@
+[[breaking_60_document_api_changes]]
+=== Document API changes
+
+==== version type 'force' removed
+
+Document modification operations may no longer specify the `version_type` of
+`force` to override any previous version checks.

+ 0 - 44
rest-api-spec/src/main/resources/rest-api-spec/test/delete/27_force_version.yaml

@@ -1,44 +0,0 @@
----
-"Force version":
-
- - do:
-      index:
-          index:          test_1
-          type:           test
-          id:             1
-          body:           { foo: bar }
-          version_type:   force
-          version:        5
-
- - match:   { _version: 5}
-
- - do:
-      delete:
-          index:          test_1
-          type:           test
-          id:             1
-          version_type:   force
-          version:        4
-
- - match:   { _version: 4}
-
- - do:
-      index:
-          index:          test_1
-          type:           test
-          id:             1
-          body:           { foo: bar }
-          version_type:   force
-          version:        6
-
- - match:   { _version: 6}
-
- - do:
-      delete:
-          index:          test_1
-          type:           test
-          id:             1
-          version_type:   force
-          version:        6
-
- - match:   { _version: 6}

+ 0 - 27
rest-api-spec/src/main/resources/rest-api-spec/test/get/90_versions.yaml

@@ -86,30 +86,3 @@
           id:     1
           version: 1
           version_type: external_gte
-
- - do:
-      get:
-          index:  test_1
-          type:   test
-          id:     1
-          version: 2
-          version_type: force
- - match:   { _id: "1" }
-
- - do:
-      get:
-          index:  test_1
-          type:   test
-          id:     1
-          version: 10
-          version_type: force
- - match:   { _id: "1" }
-
- - do:
-      get:
-          index:  test_1
-          type:   test
-          id:     1
-          version: 1
-          version_type: force
- - match:   { _id: "1" }

+ 0 - 46
rest-api-spec/src/main/resources/rest-api-spec/test/index/37_force_version.yaml

@@ -1,46 +0,0 @@
----
-"Force version":
-
- - do:
-      index:
-          index:          test_1
-          type:           test
-          id:             1
-          body:           { foo: bar }
-          version_type:   force
-          version:        5
-
- - match:   { _version: 5}
-
- - do:
-      index:
-          index:          test_1
-          type:           test
-          id:             1
-          body:           { foo: bar }
-          version_type:   force
-          version:        4
-
- - match:   { _version: 4}
-
- - do:
-      index:
-          index:          test_1
-          type:           test
-          id:             1
-          body:           { foo: bar2 }
-          version_type:   force
-          version:        5
-
- - match:   { _version: 5}
-
- - do:
-      index:
-          index:          test_1
-          type:           test
-          id:             1
-          body:           { foo: bar3 }
-          version_type:   force
-          version:        5
-
- - match:   { _version: 5}

+ 0 - 27
rest-api-spec/src/main/resources/rest-api-spec/test/termvectors/40_versions.yaml

@@ -86,30 +86,3 @@
           id:     1
           version: 1
           version_type: external_gte
-
- - do:
-      get:
-          index:  test_1
-          type:   test
-          id:     1
-          version: 2
-          version_type: force
- - match:   { _id: "1" }
-
- - do:
-      get:
-          index:  test_1
-          type:   test
-          id:     1
-          version: 10
-          version_type: force
- - match:   { _id: "1" }
-
- - do:
-      get:
-          index:  test_1
-          type:   test
-          id:     1
-          version: 1
-          version_type: force
- - match:   { _id: "1" }