honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove type from VersionConflictEngineException. (#37490) (#40514) 

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.
This commit is contained in:
Adrien Grand 2019-03-28 09:32:09 +01:00 committed by GitHub
parent 2326a3dccb
commit 65a35c985c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 33 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java
View File

@ -118,7 +118,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));
@ -147,7 +147,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));
}
@ -282,7 +282,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));
}
@ -508,7 +508,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));
@ -555,7 +555,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());
}
}

2
modules/reindex/src/test/java/org/elasticsearch/index/reindex/AsyncBulkByScrollActionTests.java
View File

@ -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;

2
modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFailureTests.java
View File

@ -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[["));
}
}

4
modules/reindex/src/test/resources/rest-api-spec/test/delete_by_query/10_basic.yml
View File

@ -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 }

2
modules/reindex/src/test/resources/rest-api-spec/test/reindex/10_basic.yml
View File

@ -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 }

4
modules/reindex/src/test/resources/rest-api-spec/test/update_by_query/10_basic.yml
View File

@ -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 }

4
server/src/main/java/org/elasticsearch/index/engine/Engine.java
View File

@ -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);
}
}

12
server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java
View File

@ -619,13 +619,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()) {
@ -1007,13 +1007,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(
@ -1338,13 +1338,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)) {

14
server/src/main/java/org/elasticsearch/index/engine/VersionConflictEngineException.java
View File

@ -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) {

4
server/src/test/java/org/elasticsearch/action/bulk/TransportShardBulkActionTests.java
View File

@ -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 =

8
server/src/test/java/org/elasticsearch/get/GetActionIT.java
View File

@ -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());