summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLee Hinman <lee@writequit.org>2016-09-07 11:00:09 -0600
committerLee Hinman <lee@writequit.org>2016-09-07 13:05:18 -0600
commitb4cc3cd35dea2e3059142c6bd7e2eb13ca8944ac (patch)
tree66b9cbfecf52a6626aab23be0545d83c69fa22be
parent8502d2761f80fbb63c201cf4f824ef369eebd0f7 (diff)
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
-rw-r--r--core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java7
-rw-r--r--core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java5
-rw-r--r--core/src/main/java/org/elasticsearch/index/VersionType.java50
-rw-r--r--core/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java7
-rw-r--r--core/src/test/java/org/elasticsearch/index/VersionTypeTests.java40
-rw-r--r--core/src/test/java/org/elasticsearch/update/UpdateIT.java8
-rw-r--r--core/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java30
-rw-r--r--docs/reference/migration/migrate_5_0/docs.asciidoc5
-rw-r--r--rest-api-spec/src/main/resources/rest-api-spec/test/delete/27_force_version.yaml44
-rw-r--r--rest-api-spec/src/main/resources/rest-api-spec/test/get/90_versions.yaml27
-rw-r--r--rest-api-spec/src/main/resources/rest-api-spec/test/index/37_force_version.yaml46
-rw-r--r--rest-api-spec/src/main/resources/rest-api-spec/test/termvectors/40_versions.yaml27
12 files changed, 12 insertions, 284 deletions
diff --git a/core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java b/core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java
index 10b508d9a1..081cfd951b 100644
--- a/core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java
+++ b/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...
diff --git a/core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java b/core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java
index 0d919ff089..3f2dde6784 100644
--- a/core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java
+++ b/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) {
diff --git a/core/src/main/java/org/elasticsearch/index/VersionType.java b/core/src/main/java/org/elasticsearch/index/VersionType.java
index 3d0448d16a..062fbce10d 100644
--- a/core/src/main/java/org/elasticsearch/index/VersionType.java
+++ b/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 + "]");
}
diff --git a/core/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java b/core/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java
index 03cf86397c..3b88efca20 100644
--- a/core/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java
+++ b/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 {
diff --git a/core/src/test/java/org/elasticsearch/index/VersionTypeTests.java b/core/src/test/java/org/elasticsearch/index/VersionTypeTests.java
index 4c79ce1b49..2afe0b7fea 100644
--- a/core/src/test/java/org/elasticsearch/index/VersionTypeTests.java
+++ b/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;
diff --git a/core/src/test/java/org/elasticsearch/update/UpdateIT.java b/core/src/test/java/org/elasticsearch/update/UpdateIT.java
index a71bd466ad..7ea68dc10f 100644
--- a/core/src/test/java/org/elasticsearch/update/UpdateIT.java
+++ b/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
diff --git a/core/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java b/core/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java
index 67e7d528e5..b80c5bd8e2 100644
--- a/core/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java
+++ b/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");
diff --git a/docs/reference/migration/migrate_5_0/docs.asciidoc b/docs/reference/migration/migrate_5_0/docs.asciidoc
index 9344589296..2c2da15d0c 100644
--- a/docs/reference/migration/migrate_5_0/docs.asciidoc
+++ b/docs/reference/migration/migrate_5_0/docs.asciidoc
@@ -64,3 +64,8 @@ will also make all other changes visible immediately. This can have an impact on
performance if the same document is updated very frequently using a read modify update
pattern since it might create many small segments. This behavior can be disabled by
passing `realtime=false` to the get request.
+
+==== version type 'force' removed
+
+In 5.0.0 document modification operations may no longer specify the
+`version_type` of `force` to override any previous version checks.
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/delete/27_force_version.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/delete/27_force_version.yaml
deleted file mode 100644
index c5d1ad31d7..0000000000
--- a/rest-api-spec/src/main/resources/rest-api-spec/test/delete/27_force_version.yaml
+++ /dev/null
@@ -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}
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/get/90_versions.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/get/90_versions.yaml
index f66322bf20..e255ce510e 100644
--- a/rest-api-spec/src/main/resources/rest-api-spec/test/get/90_versions.yaml
+++ b/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" }
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/index/37_force_version.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/index/37_force_version.yaml
deleted file mode 100644
index 17d1edc303..0000000000
--- a/rest-api-spec/src/main/resources/rest-api-spec/test/index/37_force_version.yaml
+++ /dev/null
@@ -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}
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/termvectors/40_versions.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/termvectors/40_versions.yaml
index f66322bf20..e255ce510e 100644
--- a/rest-api-spec/src/main/resources/rest-api-spec/test/termvectors/40_versions.yaml
+++ b/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" }