瀏覽代碼

Remove type from VersionConflictEngineException. (#37490)

It initially mentioned the type in the exception because the type used to be
required to uniquely identify a document. This is not necessary anymore given
that indices have at most one type.
Adrien Grand 6 年之前
父節點
當前提交
db03109fab

+ 5 - 5
client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java

@@ -137,7 +137,7 @@ public class CrudIT extends ESRestHighLevelClientTestCase {
             ElasticsearchException exception = expectThrows(ElasticsearchException.class,
                 () -> execute(deleteRequest, highLevelClient()::delete, highLevelClient()::deleteAsync));
             assertEquals(RestStatus.CONFLICT, exception.status());
-            assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[_doc][" + docId + "]: " +
+            assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[" + docId + "]: " +
                 "version conflict, required seqNo [2], primary term [2]. current document has seqNo [3] and primary term [1]]",
                 exception.getMessage());
             assertEquals("index", exception.getMetadata("es.index").get(0));
@@ -166,7 +166,7 @@ public class CrudIT extends ESRestHighLevelClientTestCase {
                 execute(deleteRequest, highLevelClient()::delete, highLevelClient()::deleteAsync);
             });
             assertEquals(RestStatus.CONFLICT, exception.status());
-            assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[_doc][" +
+            assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[" +
                 docId + "]: version conflict, current version [12] is higher or equal to the one provided [10]]", exception.getMessage());
             assertEquals("index", exception.getMetadata("es.index").get(0));
         }
@@ -301,7 +301,7 @@ public class CrudIT extends ESRestHighLevelClientTestCase {
             ElasticsearchException exception = expectThrows(ElasticsearchException.class,
                     () -> execute(getRequest, highLevelClient()::get, highLevelClient()::getAsync));
             assertEquals(RestStatus.CONFLICT, exception.status());
-            assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, " + "reason=[_doc][id]: " +
+            assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, " + "reason=[id]: " +
                     "version conflict, current version [1] is different than the one provided [2]]", exception.getMessage());
             assertEquals("index", exception.getMetadata("es.index").get(0));
         }
@@ -527,7 +527,7 @@ public class CrudIT extends ESRestHighLevelClientTestCase {
                 execute(wrongRequest, highLevelClient()::index, highLevelClient()::indexAsync);
             });
             assertEquals(RestStatus.CONFLICT, exception.status());
-            assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[_doc][id]: " +
+            assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[id]: " +
                          "version conflict, required seqNo [1], primary term [5]. current document has seqNo [2] and primary term [1]]",
                 exception.getMessage());
             assertEquals("index", exception.getMetadata("es.index").get(0));
@@ -574,7 +574,7 @@ public class CrudIT extends ESRestHighLevelClientTestCase {
             });
 
             assertEquals(RestStatus.CONFLICT, exception.status());
-            assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[_doc][with_create_op_type]: " +
+            assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[with_create_op_type]: " +
                          "version conflict, document already exists (current version [1])]", exception.getMessage());
         }
     }

+ 1 - 1
modules/reindex/src/test/java/org/elasticsearch/index/reindex/AsyncBulkByScrollActionTests.java

@@ -276,7 +276,7 @@ public class AsyncBulkByScrollActionTests extends ESTestCase {
                     versionConflicts++;
                     responses[i] = new BulkItemResponse(i, randomFrom(DocWriteRequest.OpType.values()),
                         new Failure(shardId.getIndexName(), "type", "id" + i,
-                            new VersionConflictEngineException(shardId, "type", "id", "test")));
+                            new VersionConflictEngineException(shardId, "id", "test")));
                     continue;
                 }
                 boolean createdResponse;

+ 1 - 1
modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFailureTests.java

@@ -81,7 +81,7 @@ public class ReindexFailureTests extends ReindexTestCase {
         BulkByScrollResponse response = copy.get();
         assertThat(response, matcher().batches(1).versionConflicts(1).failures(1).created(99));
         for (Failure failure: response.getBulkFailures()) {
-            assertThat(failure.getMessage(), containsString("VersionConflictEngineException[[_doc]["));
+            assertThat(failure.getMessage(), containsString("VersionConflictEngineException[["));
         }
     }
 

+ 2 - 2
modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yml

@@ -129,7 +129,7 @@
   - match: {failures.0.status: 409}
   - match: {failures.0.cause.type:   version_conflict_engine_exception}
   # Use a regex so we don't mind if the current version isn't always 1. Sometimes it comes out 2.
-  - match: {failures.0.cause.reason: "/\\[_doc\\]\\[1\\]:.version.conflict,.current.version.\\[\\d+\\].is.different.than.the.one.provided.\\[\\d+\\]/"}
+  - match: {failures.0.cause.reason: "/\\[1\\]:.version.conflict,.current.version.\\[\\d+\\].is.different.than.the.one.provided.\\[\\d+\\]/"}
   - match: {failures.0.cause.shard:  /\d+/}
   - match: {failures.0.cause.index:  test}
   - gte: { took: 0 }
@@ -185,7 +185,7 @@
   - match: {failures.0.id:     "1"}
   - match: {failures.0.status: 409}
   - match: {failures.0.cause.type:   version_conflict_engine_exception}
-  - match: {failures.0.cause.reason: "/\\[_doc\\]\\[1\\]:.version.conflict,.required.seqNo.\\[\\d+\\]/"}
+  - match: {failures.0.cause.reason: "/\\[1\\]:.version.conflict,.required.seqNo.\\[\\d+\\]/"}
   - match: {failures.0.cause.shard:  /\d+/}
   - match: {failures.0.cause.index:  test}
   - gte: { took: 0 }

+ 1 - 1
modules/reindex/src/test/resources/rest-api-spec/test/reindex/10_basic.yml

@@ -160,7 +160,7 @@
   - match: {failures.0.status: 409}
   - match: {failures.0.cause.type:   version_conflict_engine_exception}
   # Use a regex so we don't mind if the version isn't always 1. Sometimes it comes out 2.
-  - match: {failures.0.cause.reason: "/\\[_doc\\]\\[1\\]:.version.conflict,.document.already.exists.\\(current.version.\\[\\d+\\]\\)/"}
+  - match: {failures.0.cause.reason: "/\\[1\\]:.version.conflict,.document.already.exists.\\(current.version.\\[\\d+\\]\\)/"}
   - match: {failures.0.cause.shard:  /\d+/}
   - match: {failures.0.cause.index:  dest}
   - gte: { took: 0 }

+ 2 - 2
modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/10_basic.yml

@@ -109,7 +109,7 @@
   - match: {failures.0.status: 409}
   - match: {failures.0.cause.type:   version_conflict_engine_exception}
   # Use a regex so we don't mind if the current version isn't always 1. Sometimes it comes out 2.
-  - match: {failures.0.cause.reason: "/\\[_doc\\]\\[1\\]:.version.conflict,.current.version.\\[\\d+\\].is.different.than.the.one.provided.\\[\\d+\\]/"}
+  - match: {failures.0.cause.reason: "/\\[1\\]:.version.conflict,.current.version.\\[\\d+\\].is.different.than.the.one.provided.\\[\\d+\\]/"}
   - match: {failures.0.cause.shard:  /\d+/}
   - match: {failures.0.cause.index:  test}
   - gte: { took: 0 }
@@ -151,7 +151,7 @@
   - match: {failures.0.id:     "1"}
   - match: {failures.0.status: 409}
   - match: {failures.0.cause.type:   version_conflict_engine_exception}
-  - match: {failures.0.cause.reason: "/\\[_doc\\]\\[1\\]:.version.conflict,.required.seqNo.\\[\\d+\\]/"}
+  - match: {failures.0.cause.reason: "/\\[1\\]:.version.conflict,.required.seqNo.\\[\\d+\\]/"}
   - match: {failures.0.cause.shard:  /\d+/}
   - match: {failures.0.cause.index:  test}
   - gte: { took: 0 }

+ 2 - 2
server/src/main/java/org/elasticsearch/index/engine/Engine.java

@@ -633,14 +633,14 @@ public abstract class Engine implements Closeable {
         if (docIdAndVersion != null) {
             if (get.versionType().isVersionConflictForReads(docIdAndVersion.version, get.version())) {
                 Releasables.close(searcher);
-                throw new VersionConflictEngineException(shardId, get.type(), get.id(),
+                throw new VersionConflictEngineException(shardId, get.id(),
                         get.versionType().explainConflictForReads(docIdAndVersion.version, get.version()));
             }
             if (get.getIfSeqNo() != SequenceNumbers.UNASSIGNED_SEQ_NO && (
                 get.getIfSeqNo() != docIdAndVersion.seqNo || get.getIfPrimaryTerm() != docIdAndVersion.primaryTerm
             )) {
                 Releasables.close(searcher);
-                throw new VersionConflictEngineException(shardId, get.type(), get.id(),
+                throw new VersionConflictEngineException(shardId, get.id(),
                     get.getIfSeqNo(), get.getIfPrimaryTerm(), docIdAndVersion.seqNo, docIdAndVersion.primaryTerm);
             }
         }

+ 6 - 6
server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java

@@ -616,13 +616,13 @@ public class InternalEngine extends Engine {
                         return GetResult.NOT_EXISTS;
                     }
                     if (get.versionType().isVersionConflictForReads(versionValue.version, get.version())) {
-                        throw new VersionConflictEngineException(shardId, get.type(), get.id(),
+                        throw new VersionConflictEngineException(shardId, get.id(),
                             get.versionType().explainConflictForReads(versionValue.version, get.version()));
                     }
                     if (get.getIfSeqNo() != SequenceNumbers.UNASSIGNED_SEQ_NO && (
                         get.getIfSeqNo() != versionValue.seqNo || get.getIfPrimaryTerm() != versionValue.term
                         )) {
-                        throw new VersionConflictEngineException(shardId, get.type(), get.id(),
+                        throw new VersionConflictEngineException(shardId, get.id(),
                             get.getIfSeqNo(), get.getIfPrimaryTerm(), versionValue.seqNo, versionValue.term);
                     }
                     if (get.isReadFromTranslog()) {
@@ -1004,13 +1004,13 @@ public class InternalEngine extends Engine {
                 currentNotFoundOrDeleted = versionValue.isDelete();
             }
             if (index.getIfSeqNo() != SequenceNumbers.UNASSIGNED_SEQ_NO && versionValue == null) {
-                final VersionConflictEngineException e = new VersionConflictEngineException(shardId, index.type(), index.id(),
+                final VersionConflictEngineException e = new VersionConflictEngineException(shardId, index.id(),
                     index.getIfSeqNo(), index.getIfPrimaryTerm(), SequenceNumbers.UNASSIGNED_SEQ_NO, 0);
                 plan = IndexingStrategy.skipDueToVersionConflict(e, currentNotFoundOrDeleted, currentVersion, getPrimaryTerm());
             } else if (index.getIfSeqNo() != SequenceNumbers.UNASSIGNED_SEQ_NO && (
                 versionValue.seqNo != index.getIfSeqNo() || versionValue.term != index.getIfPrimaryTerm()
             )) {
-                final VersionConflictEngineException e = new VersionConflictEngineException(shardId, index.type(), index.id(),
+                final VersionConflictEngineException e = new VersionConflictEngineException(shardId, index.id(),
                     index.getIfSeqNo(), index.getIfPrimaryTerm(), versionValue.seqNo, versionValue.term);
                 plan = IndexingStrategy.skipDueToVersionConflict(e, currentNotFoundOrDeleted, currentVersion, getPrimaryTerm());
             } else if (index.versionType().isVersionConflictForWrites(
@@ -1335,13 +1335,13 @@ public class InternalEngine extends Engine {
         }
         final DeletionStrategy plan;
         if (delete.getIfSeqNo() != SequenceNumbers.UNASSIGNED_SEQ_NO && versionValue == null) {
-            final VersionConflictEngineException e = new VersionConflictEngineException(shardId, delete.type(), delete.id(),
+            final VersionConflictEngineException e = new VersionConflictEngineException(shardId, delete.id(),
                 delete.getIfSeqNo(), delete.getIfPrimaryTerm(), SequenceNumbers.UNASSIGNED_SEQ_NO, 0);
             plan = DeletionStrategy.skipDueToVersionConflict(e, currentVersion, getPrimaryTerm(), currentlyDeleted);
         } else if (delete.getIfSeqNo() != SequenceNumbers.UNASSIGNED_SEQ_NO && (
             versionValue.seqNo != delete.getIfSeqNo() || versionValue.term != delete.getIfPrimaryTerm()
         )) {
-            final VersionConflictEngineException e = new VersionConflictEngineException(shardId, delete.type(), delete.id(),
+            final VersionConflictEngineException e = new VersionConflictEngineException(shardId, delete.id(),
                 delete.getIfSeqNo(), delete.getIfPrimaryTerm(), versionValue.seqNo, versionValue.term);
             plan = DeletionStrategy.skipDueToVersionConflict(e, currentVersion, getPrimaryTerm(), currentlyDeleted);
         } else if (delete.versionType().isVersionConflictForWrites(currentVersion, delete.version(), currentlyDeleted)) {

+ 7 - 7
server/src/main/java/org/elasticsearch/index/engine/VersionConflictEngineException.java

@@ -28,25 +28,25 @@ import java.io.IOException;
 public class VersionConflictEngineException extends EngineException {
 
     public VersionConflictEngineException(ShardId shardId, Engine.Operation op, long currentVersion, boolean deleted) {
-        this(shardId, op.type(), op.id(), op.versionType().explainConflictForWrites(currentVersion, op.version(), deleted));
+        this(shardId, op.id(), op.versionType().explainConflictForWrites(currentVersion, op.version(), deleted));
     }
 
-    public VersionConflictEngineException(ShardId shardId, String type, String id,
+    public VersionConflictEngineException(ShardId shardId, String id,
                                           long compareAndWriteSeqNo, long compareAndWriteTerm,
                                           long currentSeqNo, long currentTerm) {
-        this(shardId, type, id, "required seqNo [" + compareAndWriteSeqNo + "], primary term [" + compareAndWriteTerm +"]." +
+        this(shardId, id, "required seqNo [" + compareAndWriteSeqNo + "], primary term [" + compareAndWriteTerm +"]." +
             (currentSeqNo == SequenceNumbers.UNASSIGNED_SEQ_NO ?
                 " but no document was found" :
                 " current document has seqNo [" + currentSeqNo + "] and primary term ["+ currentTerm + "]"
             ));
     }
 
-    public VersionConflictEngineException(ShardId shardId, String type, String id, String explanation) {
-        this(shardId, null, type, id, explanation);
+    public VersionConflictEngineException(ShardId shardId, String id, String explanation) {
+        this(shardId, null, id, explanation);
     }
 
-    public VersionConflictEngineException(ShardId shardId, Throwable cause, String type, String id, String explanation) {
-        this(shardId, "[{}][{}]: version conflict, {}", cause, type, id, explanation);
+    public VersionConflictEngineException(ShardId shardId, Throwable cause, String id, String explanation) {
+        this(shardId, "[{}]: version conflict, {}", cause, id, explanation);
     }
 
     public VersionConflictEngineException(ShardId shardId, String msg, Throwable cause, Object... params) {

+ 2 - 2
server/src/test/java/org/elasticsearch/action/bulk/TransportShardBulkActionTests.java

@@ -550,7 +550,7 @@ public class TransportShardBulkActionTests extends IndexShardTestCase {
 
         IndexRequest updateResponse = new IndexRequest("index", "_doc", "id").source(Requests.INDEX_CONTENT_TYPE, "field", "value");
 
-        Exception err = new VersionConflictEngineException(shardId, "_doc", "id",
+        Exception err = new VersionConflictEngineException(shardId, "id",
             "I'm conflicted <(;_;)>");
         Engine.IndexResult indexResult = new Engine.IndexResult(err, 0, 0, 0);
         IndexShard shard = mock(IndexShard.class);
@@ -784,7 +784,7 @@ public class TransportShardBulkActionTests extends IndexShardTestCase {
 
         IndexRequest updateResponse = new IndexRequest("index", "_doc", "id").source(Requests.INDEX_CONTENT_TYPE, "field", "value");
 
-        Exception err = new VersionConflictEngineException(shardId, "_doc", "id",
+        Exception err = new VersionConflictEngineException(shardId, "id",
             "I'm conflicted <(;_;)>");
         Engine.IndexResult conflictedResult = new Engine.IndexResult(err, 0, 0);
         Engine.IndexResult mappingUpdate =

+ 4 - 4
server/src/test/java/org/elasticsearch/get/GetActionIT.java

@@ -441,7 +441,7 @@ public class GetActionIT extends ESIntegTestCase {
         assertThat(response.getResponses()[1].getResponse().getSourceAsMap().get("field").toString(), equalTo("value1"));
         assertThat(response.getResponses()[2].getFailure(), notNullValue());
         assertThat(response.getResponses()[2].getFailure().getId(), equalTo("1"));
-        assertThat(response.getResponses()[2].getFailure().getMessage(), startsWith("[type1][1]: version conflict"));
+        assertThat(response.getResponses()[2].getFailure().getMessage(), startsWith("[1]: version conflict"));
         assertThat(response.getResponses()[2].getFailure().getFailure(), instanceOf(VersionConflictEngineException.class));
 
         //Version from Lucene index
@@ -464,7 +464,7 @@ public class GetActionIT extends ESIntegTestCase {
         assertThat(response.getResponses()[1].getResponse().getSourceAsMap().get("field").toString(), equalTo("value1"));
         assertThat(response.getResponses()[2].getFailure(), notNullValue());
         assertThat(response.getResponses()[2].getFailure().getId(), equalTo("1"));
-        assertThat(response.getResponses()[2].getFailure().getMessage(), startsWith("[type1][1]: version conflict"));
+        assertThat(response.getResponses()[2].getFailure().getMessage(), startsWith("[1]: version conflict"));
         assertThat(response.getResponses()[2].getFailure().getFailure(), instanceOf(VersionConflictEngineException.class));
 
 
@@ -489,7 +489,7 @@ public class GetActionIT extends ESIntegTestCase {
         assertThat(response.getResponses()[1].getFailure(), notNullValue());
         assertThat(response.getResponses()[1].getFailure().getId(), equalTo("2"));
         assertThat(response.getResponses()[1].getIndex(), equalTo("test"));
-        assertThat(response.getResponses()[1].getFailure().getMessage(), startsWith("[type1][2]: version conflict"));
+        assertThat(response.getResponses()[1].getFailure().getMessage(), startsWith("[2]: version conflict"));
         assertThat(response.getResponses()[2].getId(), equalTo("2"));
         assertThat(response.getResponses()[2].getIndex(), equalTo("test"));
         assertThat(response.getResponses()[2].getFailure(), nullValue());
@@ -515,7 +515,7 @@ public class GetActionIT extends ESIntegTestCase {
         assertThat(response.getResponses()[1].getFailure(), notNullValue());
         assertThat(response.getResponses()[1].getFailure().getId(), equalTo("2"));
         assertThat(response.getResponses()[1].getIndex(), equalTo("test"));
-        assertThat(response.getResponses()[1].getFailure().getMessage(), startsWith("[type1][2]: version conflict"));
+        assertThat(response.getResponses()[1].getFailure().getMessage(), startsWith("[2]: version conflict"));
         assertThat(response.getResponses()[2].getId(), equalTo("2"));
         assertThat(response.getResponses()[2].getIndex(), equalTo("test"));
         assertThat(response.getResponses()[2].getFailure(), nullValue());