From 44b87e5a1cbb063456d7634a75b977be1facbaab Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Sun, 27 Jan 2019 22:24:00 -0500 Subject: [PATCH 01/37] Remove internal versioning as CAS --- .../client/WatcherRequestConverters.java | 1 - .../client/watcher/PutWatchRequest.java | 11 ---- .../client/WatcherRequestConvertersTests.java | 10 ++-- .../reindex/AsyncDeleteByQueryAction.java | 22 ++------ .../reindex/TransportDeleteByQueryAction.java | 2 +- .../reindex/TransportUpdateByQueryAction.java | 10 +--- .../elasticsearch/action/DocWriteRequest.java | 17 ++++-- .../action/update/UpdateHelper.java | 2 +- .../action/update/UpdateRequest.java | 54 +++++++------------ .../index/get/ShardGetService.java | 7 +-- .../index/shard/ShardGetServiceTests.java | 26 ++++----- 11 files changed, 57 insertions(+), 105 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/WatcherRequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/WatcherRequestConverters.java index 34fb826d62382..9718607d8b80e 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/WatcherRequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/WatcherRequestConverters.java @@ -70,7 +70,6 @@ static Request putWatch(PutWatchRequest putWatchRequest) { Request request = new Request(HttpPut.METHOD_NAME, endpoint); RequestConverters.Params params = new RequestConverters.Params(request) - .withVersion(putWatchRequest.getVersion()) .withIfSeqNo(putWatchRequest.ifSeqNo()) .withIfPrimaryTerm(putWatchRequest.ifPrimaryTerm()); if (putWatchRequest.isActive() == false) { diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/watcher/PutWatchRequest.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/watcher/PutWatchRequest.java index 8b83970723dd2..1d13a77e06cf9 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/watcher/PutWatchRequest.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/watcher/PutWatchRequest.java @@ -21,7 +21,6 @@ import org.elasticsearch.client.Validatable; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.lucene.uid.Versions; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.seqno.SequenceNumbers; @@ -43,11 +42,9 @@ public final class PutWatchRequest implements Validatable { private final BytesReference source; private final XContentType xContentType; private boolean active = true; - private long version = Versions.MATCH_ANY; private long ifSeqNo = SequenceNumbers.UNASSIGNED_SEQ_NO; private long ifPrimaryTerm = UNASSIGNED_PRIMARY_TERM; - public PutWatchRequest(String id, BytesReference source, XContentType xContentType) { Objects.requireNonNull(id, "watch id is missing"); if (isValidId(id) == false) { @@ -95,14 +92,6 @@ public XContentType xContentType() { return xContentType; } - public long getVersion() { - return version; - } - - public void setVersion(long version) { - this.version = version; - } - /** * only performs this put request if the watch's last modification was assigned the given * sequence number. Must be used in combination with {@link #setIfPrimaryTerm(long)} diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/WatcherRequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/WatcherRequestConvertersTests.java index a31206bee88cc..b4dc612e341ea 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/WatcherRequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/WatcherRequestConvertersTests.java @@ -29,8 +29,8 @@ import org.elasticsearch.client.watcher.DeactivateWatchRequest; import org.elasticsearch.client.watcher.DeleteWatchRequest; import org.elasticsearch.client.watcher.ExecuteWatchRequest; -import org.elasticsearch.client.watcher.PutWatchRequest; import org.elasticsearch.client.watcher.GetWatchRequest; +import org.elasticsearch.client.watcher.PutWatchRequest; import org.elasticsearch.client.watcher.StartWatchServiceRequest; import org.elasticsearch.client.watcher.StopWatchServiceRequest; import org.elasticsearch.client.watcher.WatcherStatsRequest; @@ -88,9 +88,11 @@ public void testPutWatch() throws Exception { } if (randomBoolean()) { - long version = randomLongBetween(10, 100); - putWatchRequest.setVersion(version); - expectedParams.put("version", String.valueOf(version)); + long seqNo = randomNonNegativeLong(); + long ifPrimaryTerm = randomLongBetween(1, 200); + putWatchRequest.setIfSeqNo(seqNo); + expectedParams.put("if_seq_no", String.valueOf(seqNo)); + expectedParams.put("if_primary_term", String.valueOf(ifPrimaryTerm)); } Request request = WatcherRequestConverters.putWatch(putWatchRequest); diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AsyncDeleteByQueryAction.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AsyncDeleteByQueryAction.java index b317ea06d9f35..f7d8d037fcddb 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AsyncDeleteByQueryAction.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/AsyncDeleteByQueryAction.java @@ -20,11 +20,9 @@ package org.elasticsearch.index.reindex; import org.apache.logging.log4j.Logger; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.delete.DeleteRequest; import org.elasticsearch.client.ParentTaskAssigningClient; -import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.script.ScriptService; import org.elasticsearch.threadpool.ThreadPool; @@ -33,18 +31,10 @@ */ public class AsyncDeleteByQueryAction extends AbstractAsyncBulkByScrollAction { - private final boolean useSeqNoForCAS; - public AsyncDeleteByQueryAction(BulkByScrollTask task, Logger logger, ParentTaskAssigningClient client, ThreadPool threadPool, TransportDeleteByQueryAction action, DeleteByQueryRequest request, - ScriptService scriptService, ClusterState clusterState, ActionListener listener) { - super(task, - // not all nodes support sequence number powered optimistic concurrency control, we fall back to version - clusterState.nodes().getMinNodeVersion().onOrAfter(Version.V_6_7_0) == false, - // all nodes support sequence number powered optimistic concurrency control and we can use it - clusterState.nodes().getMinNodeVersion().onOrAfter(Version.V_6_7_0), - logger, client, threadPool, action, request, listener); - useSeqNoForCAS = clusterState.nodes().getMinNodeVersion().onOrAfter(Version.V_6_7_0); + ScriptService scriptService, ActionListener listener) { + super(task, false, true, logger, client, threadPool, action, request, listener); } @Override @@ -60,12 +50,8 @@ protected RequestWrapper buildRequest(ScrollableHitSource.Hit doc delete.index(doc.getIndex()); delete.type(doc.getType()); delete.id(doc.getId()); - if (useSeqNoForCAS) { - delete.setIfSeqNo(doc.getSeqNo()); - delete.setIfPrimaryTerm(doc.getPrimaryTerm()); - } else { - delete.version(doc.getVersion()); - } + delete.setIfSeqNo(doc.getSeqNo()); + delete.setIfPrimaryTerm(doc.getPrimaryTerm()); return wrap(delete); } diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/TransportDeleteByQueryAction.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/TransportDeleteByQueryAction.java index 08538b335535d..d7959f0058974 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/TransportDeleteByQueryAction.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/TransportDeleteByQueryAction.java @@ -61,7 +61,7 @@ public void doExecute(Task task, DeleteByQueryRequest request, ActionListener buildRequest(ScrollableHitSource.Hit doc) index.type(doc.getType()); index.id(doc.getId()); index.source(doc.getSource(), doc.getXContentType()); - if (useSeqNoForCAS) { - index.setIfSeqNo(doc.getSeqNo()); - index.setIfPrimaryTerm(doc.getPrimaryTerm()); - } else { - index.versionType(VersionType.INTERNAL); - index.version(doc.getVersion()); - } + index.setIfSeqNo(doc.getSeqNo()); + index.setIfPrimaryTerm(doc.getPrimaryTerm()); index.setPipeline(mainRequest.getPipeline()); return wrap(index); } diff --git a/server/src/main/java/org/elasticsearch/action/DocWriteRequest.java b/server/src/main/java/org/elasticsearch/action/DocWriteRequest.java index d8a9a3503a617..373dfaa5c7416 100644 --- a/server/src/main/java/org/elasticsearch/action/DocWriteRequest.java +++ b/server/src/main/java/org/elasticsearch/action/DocWriteRequest.java @@ -257,16 +257,23 @@ static void writeDocumentRequest(StreamOutput out, DocWriteRequest request) static ActionRequestValidationException validateSeqNoBasedCASParams( DocWriteRequest request, ActionRequestValidationException validationException) { - if (request.versionType().validateVersionForWrites(request.version()) == false) { - validationException = addValidationError("illegal version value [" + request.version() + "] for version type [" - + request.versionType().name() + "]", validationException); + final long version = request.version(); + final VersionType versionType = request.versionType(); + if (versionType.validateVersionForWrites(version) == false) { + validationException = addValidationError("illegal version value [" + version + "] for version type [" + + versionType.name() + "]", validationException); } - if (request.versionType() == VersionType.FORCE) { + if (versionType == VersionType.FORCE) { validationException = addValidationError("version type [force] may no longer be used", validationException); } + if (versionType == VersionType.INTERNAL && version != Versions.MATCH_ANY && version != Versions.MATCH_DELETED) { + validationException = addValidationError("internal versioning can not be used for optimistic concurrency control. " + + "Please use `if_seq_no` and `if_primary_term` instead", validationException); + } + if (request.ifSeqNo() != UNASSIGNED_SEQ_NO && ( - request.versionType() != VersionType.INTERNAL || request.version() != Versions.MATCH_ANY + versionType != VersionType.INTERNAL || version != Versions.MATCH_ANY )) { validationException = addValidationError("compare and write operations can not use versioning", validationException); } diff --git a/server/src/main/java/org/elasticsearch/action/update/UpdateHelper.java b/server/src/main/java/org/elasticsearch/action/update/UpdateHelper.java index 8cd6146768fff..54cd38aa0b960 100644 --- a/server/src/main/java/org/elasticsearch/action/update/UpdateHelper.java +++ b/server/src/main/java/org/elasticsearch/action/update/UpdateHelper.java @@ -70,7 +70,7 @@ public UpdateHelper(ScriptService scriptService) { */ public Result prepare(UpdateRequest request, IndexShard indexShard, LongSupplier nowInMillis) { final GetResult getResult = indexShard.getService().getForUpdate( - request.type(), request.id(), request.version(), request.versionType(), request.ifSeqNo(), request.ifPrimaryTerm()); + request.type(), request.id(), request.ifSeqNo(), request.ifPrimaryTerm()); return prepare(indexShard.shardId(), request, getResult, nowInMillis); } diff --git a/server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java b/server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java index 2a1865aa80818..7f1f9f2233280 100644 --- a/server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java @@ -108,8 +108,6 @@ public class UpdateRequest extends InstanceShardOperationRequest private FetchSourceContext fetchSourceContext; - private long version = Versions.MATCH_ANY; - private VersionType versionType = VersionType.INTERNAL; private int retryOnConflict = 0; private long ifSeqNo = UNASSIGNED_SEQ_NO; private long ifPrimaryTerm = UNASSIGNED_PRIMARY_TERM; @@ -150,9 +148,6 @@ public UpdateRequest(String index, String type, String id) { @Override public ActionRequestValidationException validate() { ActionRequestValidationException validationException = super.validate(); - if (version != Versions.MATCH_ANY && upsertRequest != null) { - validationException = addValidationError("can't provide both upsert request and a version", validationException); - } if(upsertRequest != null && upsertRequest.version() != Versions.MATCH_ANY) { validationException = addValidationError("can't provide version in upsert request", validationException); } @@ -163,21 +158,6 @@ public ActionRequestValidationException validate() { validationException = addValidationError("id is missing", 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) { - validationException = addValidationError("can't provide both retry_on_conflict and a specific version", - validationException); - } - - if (!versionType.validateVersionForWrites(version)) { - validationException = addValidationError("illegal version value [" + version + "] for version type [" + - versionType.name() + "]", validationException); - } - } validationException = DocWriteRequest.validateSeqNoBasedCASParams(this, validationException); @@ -530,24 +510,22 @@ public int retryOnConflict() { @Override public UpdateRequest version(long version) { - this.version = version; - return this; + throw new UnsupportedOperationException("update requests do not support versioning"); } @Override public long version() { - return this.version; + return Versions.MATCH_ANY; } @Override public UpdateRequest versionType(VersionType versionType) { - this.versionType = versionType; - return this; + throw new UnsupportedOperationException("update requests do not support versioning"); } @Override public VersionType versionType() { - return this.versionType; + return VersionType.INTERNAL; } /** @@ -877,12 +855,16 @@ public void readFrom(StreamInput in) throws IOException { upsertRequest.readFrom(in); } docAsUpsert = in.readBoolean(); - version = in.readLong(); - versionType = VersionType.fromValue(in.readByte()); - if (in.getVersion().onOrAfter(Version.V_7_0_0)) { - ifSeqNo = in.readZLong(); - ifPrimaryTerm = in.readVLong(); + if (in.getVersion().before(Version.V_7_0_0)) { + long version = in.readLong(); + VersionType versionType = VersionType.readFromStream(in); + if (version != Versions.MATCH_ANY || versionType != VersionType.INTERNAL) { + throw new UnsupportedOperationException( + "versioned update requests have been removed in 7.0. Use if_seq_no and if_primary_term"); + } } + ifSeqNo = in.readZLong(); + ifPrimaryTerm = in.readVLong(); detectNoop = in.readBoolean(); scriptedUpsert = in.readBoolean(); } @@ -932,12 +914,12 @@ public void writeTo(StreamOutput out) throws IOException { upsertRequest.writeTo(out); } out.writeBoolean(docAsUpsert); - out.writeLong(version); - out.writeByte(versionType.getValue()); - if (out.getVersion().onOrAfter(Version.V_7_0_0)) { - out.writeZLong(ifSeqNo); - out.writeVLong(ifPrimaryTerm); + if (out.getVersion().before(Version.V_7_0_0)) { + out.writeLong(Versions.MATCH_ANY); + out.writeByte(VersionType.INTERNAL.getValue()); } + out.writeZLong(ifSeqNo); + out.writeVLong(ifPrimaryTerm); out.writeBoolean(detectNoop); out.writeBoolean(scriptedUpsert); } diff --git a/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java b/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java index 9fb1cb804946f..3c85fe40c5ba7 100644 --- a/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java +++ b/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.document.DocumentField; +import org.elasticsearch.common.lucene.uid.Versions; import org.elasticsearch.common.lucene.uid.VersionsAndSeqNoResolver.DocIdAndVersion; import org.elasticsearch.common.metrics.CounterMetric; import org.elasticsearch.common.metrics.MeanMetric; @@ -102,9 +103,9 @@ private GetResult get(String type, String id, String[] gFields, boolean realtime } } - public GetResult getForUpdate(String type, String id, long version, VersionType versionType, long ifSeqNo, long ifPrimaryTerm) { - return get(type, id, new String[]{RoutingFieldMapper.NAME}, true, version, versionType, ifSeqNo, ifPrimaryTerm, - FetchSourceContext.FETCH_SOURCE, true); + public GetResult getForUpdate(String type, String id, long ifSeqNo, long ifPrimaryTerm) { + return get(type, id, new String[]{RoutingFieldMapper.NAME}, true, + Versions.MATCH_ANY, VersionType.INTERNAL, ifSeqNo, ifPrimaryTerm, FetchSourceContext.FETCH_SOURCE, true); } /** diff --git a/server/src/test/java/org/elasticsearch/index/shard/ShardGetServiceTests.java b/server/src/test/java/org/elasticsearch/index/shard/ShardGetServiceTests.java index 496221ca9fc4e..5492b8bf7c672 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/ShardGetServiceTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/ShardGetServiceTests.java @@ -22,7 +22,6 @@ import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.index.VersionType; import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.engine.VersionConflictEngineException; import org.elasticsearch.index.get.GetResult; @@ -31,7 +30,6 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; -import static org.elasticsearch.common.lucene.uid.Versions.MATCH_ANY; import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_PRIMARY_TERM; import static org.elasticsearch.index.seqno.SequenceNumbers.UNASSIGNED_SEQ_NO; @@ -51,8 +49,7 @@ public void testGetForUpdate() throws IOException { recoverShardFromStore(primary); Engine.IndexResult test = indexDoc(primary, "test", "0", "{\"foo\" : \"bar\"}"); assertTrue(primary.getEngine().refreshNeeded()); - GetResult testGet = primary.getService().getForUpdate( - "test", "0", test.getVersion(), VersionType.INTERNAL, UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM); + GetResult testGet = primary.getService().getForUpdate("test", "0", UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM); assertFalse(testGet.getFields().containsKey(RoutingFieldMapper.NAME)); assertEquals(new String(testGet.source(), StandardCharsets.UTF_8), "{\"foo\" : \"bar\"}"); try (Engine.Searcher searcher = primary.getEngine().acquireSearcher("test", Engine.SearcherScope.INTERNAL)) { @@ -61,8 +58,7 @@ public void testGetForUpdate() throws IOException { Engine.IndexResult test1 = indexDoc(primary, "test", "1", "{\"foo\" : \"baz\"}", XContentType.JSON, "foobar"); assertTrue(primary.getEngine().refreshNeeded()); - GetResult testGet1 = primary.getService().getForUpdate( - "test", "1", test1.getVersion(), VersionType.INTERNAL, UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM); + GetResult testGet1 = primary.getService().getForUpdate("test", "1", UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM); assertEquals(new String(testGet1.source(), StandardCharsets.UTF_8), "{\"foo\" : \"baz\"}"); assertTrue(testGet1.getFields().containsKey(RoutingFieldMapper.NAME)); assertEquals("foobar", testGet1.getFields().get(RoutingFieldMapper.NAME).getValue()); @@ -77,20 +73,19 @@ public void testGetForUpdate() throws IOException { // now again from the reader Engine.IndexResult test2 = indexDoc(primary, "test", "1", "{\"foo\" : \"baz\"}", XContentType.JSON, "foobar"); assertTrue(primary.getEngine().refreshNeeded()); - testGet1 = primary.getService().getForUpdate("test", "1", test2.getVersion(), VersionType.INTERNAL, - UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM); + testGet1 = primary.getService().getForUpdate("test", "1", UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM); assertEquals(new String(testGet1.source(), StandardCharsets.UTF_8), "{\"foo\" : \"baz\"}"); assertTrue(testGet1.getFields().containsKey(RoutingFieldMapper.NAME)); assertEquals("foobar", testGet1.getFields().get(RoutingFieldMapper.NAME).getValue()); final long primaryTerm = primary.getOperationPrimaryTerm(); - testGet1 = primary.getService().getForUpdate("test", "1", MATCH_ANY, VersionType.INTERNAL, test2.getSeqNo(), primaryTerm); + testGet1 = primary.getService().getForUpdate("test", "1", test2.getSeqNo(), primaryTerm); assertEquals(new String(testGet1.source(), StandardCharsets.UTF_8), "{\"foo\" : \"baz\"}"); expectThrows(VersionConflictEngineException.class, () -> - primary.getService().getForUpdate("test", "1", MATCH_ANY, VersionType.INTERNAL, test2.getSeqNo() + 1, primaryTerm)); + primary.getService().getForUpdate("test", "1", test2.getSeqNo() + 1, primaryTerm)); expectThrows(VersionConflictEngineException.class, () -> - primary.getService().getForUpdate("test", "1", MATCH_ANY, VersionType.INTERNAL, test2.getSeqNo(), primaryTerm + 1)); + primary.getService().getForUpdate("test", "1", test2.getSeqNo(), primaryTerm + 1)); closeShards(primary); } @@ -108,16 +103,13 @@ public void testTypelessGetForUpdate() throws IOException { Engine.IndexResult indexResult = indexDoc(shard, "some_type", "0", "{\"foo\" : \"bar\"}"); assertTrue(indexResult.isCreated()); - GetResult getResult = shard.getService().getForUpdate( - "some_type", "0", MATCH_ANY, VersionType.INTERNAL, UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM); + GetResult getResult = shard.getService().getForUpdate("some_type", "0", UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM); assertTrue(getResult.isExists()); - getResult = shard.getService().getForUpdate( - "some_other_type", "0", MATCH_ANY, VersionType.INTERNAL, UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM); + getResult = shard.getService().getForUpdate("some_other_type", "0", UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM); assertFalse(getResult.isExists()); - getResult = shard.getService().getForUpdate( - "_doc", "0", MATCH_ANY, VersionType.INTERNAL, UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM); + getResult = shard.getService().getForUpdate("_doc", "0", UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM); assertTrue(getResult.isExists()); closeShards(shard); From 7ef53e9b20666d9d10b2a8557ebbf4703094d177 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Fri, 1 Feb 2019 10:28:15 +0100 Subject: [PATCH 02/37] fix upsert + seq numbers --- .../elasticsearch/action/bulk/BulkRequest.java | 14 +++++--------- .../action/update/UpdateRequest.java | 17 +++++++++++------ .../action/bulk/BulkRequestTests.java | 11 ++++++----- .../action/update/UpdateRequestTests.java | 7 ++++--- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java index b5c786ab2df6d..42f569c0a9bda 100644 --- a/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java +++ b/server/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java @@ -46,9 +46,9 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.VersionType; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.rest.action.document.RestBulkAction; -import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import java.io.IOException; @@ -501,8 +501,11 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null .create(true).setPipeline(pipeline).setIfSeqNo(ifSeqNo).setIfPrimaryTerm(ifPrimaryTerm) .source(sliceTrimmingCarriageReturn(data, from, nextMarker, xContentType), xContentType), payload); } else if ("update".equals(action)) { + if (version != Versions.MATCH_ANY || versionType != VersionType.INTERNAL) { + throw new IllegalArgumentException("Update requests do not support versioning. " + + "Please use `if_seq_no` and `if_primary_term` instead"); + } UpdateRequest updateRequest = new UpdateRequest(index, type, id).routing(routing).retryOnConflict(retryOnConflict) - .version(version).versionType(versionType) .setIfSeqNo(ifSeqNo).setIfPrimaryTerm(ifPrimaryTerm) .routing(routing); // EMPTY is safe here because we never call namedObject @@ -516,15 +519,8 @@ public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Null } IndexRequest upsertRequest = updateRequest.upsertRequest(); if (upsertRequest != null) { - upsertRequest.version(version); - upsertRequest.versionType(versionType); upsertRequest.setPipeline(defaultPipeline); } - IndexRequest doc = updateRequest.doc(); - if (doc != null) { - doc.version(version); - doc.versionType(versionType); - } internalAdd(updateRequest, payload); } diff --git a/server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java b/server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java index 7f1f9f2233280..3693975ddab08 100644 --- a/server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java @@ -158,15 +158,20 @@ public ActionRequestValidationException validate() { validationException = addValidationError("id is missing", validationException); } - validationException = DocWriteRequest.validateSeqNoBasedCASParams(this, validationException); - if (ifSeqNo != UNASSIGNED_SEQ_NO && retryOnConflict > 0) { - validationException = addValidationError("compare and write operations can not be retried", validationException); - } + if (ifSeqNo != UNASSIGNED_SEQ_NO) { + if (retryOnConflict > 0) { + validationException = addValidationError("compare and write operations can not be retried", validationException); + } - if (ifSeqNo != UNASSIGNED_SEQ_NO && docAsUpsert) { - validationException = addValidationError("compare and write operations can not be used with upsert", validationException); + if (docAsUpsert) { + validationException = addValidationError("compare and write operations can not be used with upsert", validationException); + } + if (upsertRequest != null) { + validationException = + addValidationError("upsert requests don't support `if_seq_no` and `if_primary_term`", validationException); + } } if (script == null && doc == null) { diff --git a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java index 75701e0685290..6d3e4c04c13d7 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java @@ -311,7 +311,7 @@ public void testSmileIsSupported() throws IOException { assertWarnings(RestBulkAction.TYPES_DEPRECATION_MESSAGE); } - public void testToValidateUpsertRequestAndVersionInBulkRequest() throws IOException { + public void testToValidateUpsertRequestAndCASInBulkRequest() throws IOException { XContentType xContentType = XContentType.SMILE; BytesReference data; try (BytesStreamOutput out = new BytesStreamOutput()) { @@ -321,7 +321,8 @@ public void testToValidateUpsertRequestAndVersionInBulkRequest() throws IOExcept builder.field("_index", "index"); builder.field("_type", "type"); builder.field("_id", "id"); - builder.field("version", 1L); + builder.field("if_seq_no", 1L); + builder.field("if_primary_term", 100L); builder.endObject(); builder.endObject(); } @@ -330,7 +331,8 @@ public void testToValidateUpsertRequestAndVersionInBulkRequest() throws IOExcept builder.startObject(); builder.startObject("doc").endObject(); Map values = new HashMap<>(); - values.put("version", 2L); + values.put("if_seq_no", 1L); + values.put("if_primary_term", 100L); values.put("_index", "index"); values.put("_type", "type"); builder.field("upsert", values); @@ -341,8 +343,7 @@ public void testToValidateUpsertRequestAndVersionInBulkRequest() throws IOExcept } BulkRequest bulkRequest = new BulkRequest(); bulkRequest.add(data, null, null, xContentType); - assertThat(bulkRequest.validate().validationErrors(), contains("can't provide both upsert request and a version", - "can't provide version in upsert request")); + assertThat(bulkRequest.validate().validationErrors(), contains("upsert requests don't support `if_seq_no` and `if_primary_term`")); //This test's JSON contains outdated references to types assertWarnings(RestBulkAction.TYPES_DEPRECATION_MESSAGE); } diff --git a/server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java b/server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java index 5a734352eafb2..4085af802ebc7 100644 --- a/server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java @@ -500,12 +500,13 @@ public void testToAndFromXContent() throws IOException { assertToXContentEquivalent(originalBytes, finalBytes, xContentType); } - public void testToValidateUpsertRequestAndVersion() { + public void testToValidateUpsertRequestAndCAS() { UpdateRequest updateRequest = new UpdateRequest("index", "type", "id"); - updateRequest.version(1L); + updateRequest.setIfSeqNo(1L); + updateRequest.setIfPrimaryTerm(1L); updateRequest.doc("{}", XContentType.JSON); updateRequest.upsert(new IndexRequest("index","type", "id")); - assertThat(updateRequest.validate().validationErrors(), contains("can't provide both upsert request and a version")); + assertThat(updateRequest.validate().validationErrors(), contains("upsert requests don't support `if_seq_no` and `if_primary_term`")); } public void testToValidateUpsertRequestWithVersion() { From 16cc5a695ea870e3dcfbb01284f5c674d7763fbf Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Sat, 2 Feb 2019 09:43:30 +0100 Subject: [PATCH 03/37] remove version params from the update rest api --- .../src/main/resources/rest-api-spec/api/update.json | 9 --------- .../rest/action/document/RestUpdateAction.java | 2 -- 2 files changed, 11 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/update.json b/rest-api-spec/src/main/resources/rest-api-spec/api/update.json index 92f1013a317c3..106b29b252ad3 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/update.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/update.json @@ -70,15 +70,6 @@ "if_primary_term" : { "type" : "number", "description" : "only perform the update operation if the last operation that has changed the document has the specified primary term" - }, - "version": { - "type": "number", - "description": "Explicit version number for concurrency control" - }, - "version_type": { - "type": "enum", - "options": ["internal", "force"], - "description": "Specific version type" } } }, diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java index 463a18ea6b802..804fa61fc53b2 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestUpdateAction.java @@ -83,8 +83,6 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC } updateRequest.retryOnConflict(request.paramAsInt("retry_on_conflict", updateRequest.retryOnConflict())); - updateRequest.version(RestActions.parseVersion(request)); - updateRequest.versionType(VersionType.fromString(request.param("version_type"), updateRequest.versionType())); updateRequest.setIfSeqNo(request.paramAsLong("if_seq_no", updateRequest.ifSeqNo())); updateRequest.setIfPrimaryTerm(request.paramAsLong("if_primary_term", updateRequest.ifPrimaryTerm())); From 9c186580cc28df603a327045da97c702f681a9a2 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Sat, 2 Feb 2019 09:48:52 +0100 Subject: [PATCH 04/37] lint --- .../org/elasticsearch/action/update/UpdateRequestTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java b/server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java index 4085af802ebc7..642d14e2258cb 100644 --- a/server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java @@ -506,7 +506,8 @@ public void testToValidateUpsertRequestAndCAS() { updateRequest.setIfPrimaryTerm(1L); updateRequest.doc("{}", XContentType.JSON); updateRequest.upsert(new IndexRequest("index","type", "id")); - assertThat(updateRequest.validate().validationErrors(), contains("upsert requests don't support `if_seq_no` and `if_primary_term`")); + assertThat(updateRequest.validate().validationErrors(), + contains("upsert requests don't support `if_seq_no` and `if_primary_term`")); } public void testToValidateUpsertRequestWithVersion() { From 11834f85fe7fd2f791a506ed57a1cb60a3fc065f Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Sun, 3 Feb 2019 12:22:41 +0100 Subject: [PATCH 05/37] move ml to seq no cas --- .../ml/action/TransportOpenJobAction.java | 2 +- .../action/TransportUpdateFilterAction.java | 30 ++++++++++---- .../xpack/ml/job/JobManager.java | 6 +-- .../ml/job/persistence/JobConfigProvider.java | 41 +++++++++++++------ .../ml/integration/JobConfigProviderIT.java | 16 ++++---- 5 files changed, 63 insertions(+), 32 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java index d365733eac0b4..f873d8699b9b4 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java @@ -514,7 +514,7 @@ public void onTimeout(TimeValue timeout) { private void clearJobFinishedTime(String jobId, ActionListener listener) { JobUpdate update = new JobUpdate.Builder(jobId).setClearFinishTime(true).build(); - jobConfigProvider.updateJob(jobId, update, null, ActionListener.wrap( + jobConfigProvider.updateJob(jobId, update, null, clusterService.state().nodes().getMinNodeVersion(), ActionListener.wrap( job -> listener.onResponse(new AcknowledgedResponse(true)), e -> { logger.error("[" + jobId + "] Failed to clear finished_time", e); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java index f464b86d29822..d8d5fe216b2a4 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.ml.action; import org.elasticsearch.ResourceNotFoundException; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.get.GetAction; import org.elasticsearch.action.get.GetRequest; @@ -17,6 +18,7 @@ import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; @@ -52,14 +54,16 @@ public class TransportUpdateFilterAction extends HandledTransportAction) UpdateFilterAction.Request::new); this.client = client; this.jobManager = jobManager; + this.clusterService = clusterService; } @Override @@ -95,13 +99,20 @@ private void updateFilter(FilterWithVersion filterWithVersion, UpdateFilterActio } MlFilter updatedFilter = MlFilter.builder(filter.getId()).setDescription(description).setItems(items).build(); - indexUpdatedFilter(updatedFilter, filterWithVersion.version, request, listener); + indexUpdatedFilter( + updatedFilter, filterWithVersion.version, filterWithVersion.seqNo, filterWithVersion.primaryTerm, request, listener); } - private void indexUpdatedFilter(MlFilter filter, long version, UpdateFilterAction.Request request, + private void indexUpdatedFilter(MlFilter filter, final long version, final long seqNo, final long primaryTerm, + UpdateFilterAction.Request request, ActionListener listener) { IndexRequest indexRequest = new IndexRequest(MlMetaIndex.INDEX_NAME, MlMetaIndex.TYPE, filter.documentId()); - indexRequest.version(version); + if (clusterService.state().nodes().getMinNodeVersion().onOrAfter(Version.V_6_7_0)) { + indexRequest.setIfSeqNo(seqNo); + indexRequest.setIfPrimaryTerm(primaryTerm); + } else { + indexRequest.version(version); + } indexRequest.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); try (XContentBuilder builder = XContentFactory.jsonBuilder()) { @@ -146,7 +157,7 @@ public void onResponse(GetResponse getDocResponse) { XContentParser parser = XContentFactory.xContent(XContentType.JSON) .createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, stream)) { MlFilter filter = MlFilter.LENIENT_PARSER.apply(parser, null).build(); - listener.onResponse(new FilterWithVersion(filter, getDocResponse.getVersion())); + listener.onResponse(new FilterWithVersion(filter, getDocResponse)); } } else { this.onFailure(new ResourceNotFoundException(Messages.getMessage(Messages.FILTER_NOT_FOUND, filterId))); @@ -167,10 +178,15 @@ private static class FilterWithVersion { private final MlFilter filter; private final long version; + private final long seqNo; + private final long primaryTerm; - private FilterWithVersion(MlFilter filter, long version) { + private FilterWithVersion(MlFilter filter, GetResponse getDocResponse) { this.filter = filter; - this.version = version; + this.version = getDocResponse.getVersion(); + this.seqNo = getDocResponse.getSeqNo(); + this.primaryTerm = getDocResponse.getPrimaryTerm(); + } } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java index 53559aee4701b..6696bfe1ad96a 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java @@ -333,7 +333,7 @@ public void updateJob(UpdateJobAction.Request request, ActionListener { jobConfigProvider.updateJobWithValidation(request.getJobId(), request.getJobUpdate(), maxModelMemoryLimit, - this::validate, ActionListener.wrap( + this::validate, clusterService.state().nodes().getMinNodeVersion(), ActionListener.wrap( updatedJob -> postJobUpdate(request, updatedJob, actionListener), actionListener::onFailure )); @@ -603,8 +603,8 @@ public void revertSnapshot(RevertModelSnapshotAction.Request request, ActionList .setModelSnapshotId(modelSnapshot.getSnapshotId()) .build(); - jobConfigProvider.updateJob(request.getJobId(), update, maxModelMemoryLimit, ActionListener.wrap( - job -> { + jobConfigProvider.updateJob(request.getJobId(), update, maxModelMemoryLimit, clusterService.state().nodes().getMinNodeVersion(), + ActionListener.wrap(job -> { auditor.info(request.getJobId(), Messages.getMessage(Messages.JOB_AUDIT_REVERTED, modelSnapshot.getDescription())); updateHandler.accept(Boolean.TRUE); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobConfigProvider.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobConfigProvider.java index 9019dc2032ccf..e5ee8855969a3 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobConfigProvider.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobConfigProvider.java @@ -10,6 +10,7 @@ import org.apache.lucene.search.join.ScoreMode; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.DocWriteResponse; @@ -21,6 +22,7 @@ import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.index.IndexAction; import org.elasticsearch.action.index.IndexRequest; +import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchResponse; @@ -225,9 +227,12 @@ public void onFailure(Exception e) { * @param maxModelMemoryLimit The maximum model memory allowed. This can be {@code null} * if the job's {@link org.elasticsearch.xpack.core.ml.job.config.AnalysisLimits} * are not changed. + * @param minClusterNodeVersion the minimum version of nodes in the cluster * @param updatedJobListener Updated job listener */ - public void updateJob(String jobId, JobUpdate update, ByteSizeValue maxModelMemoryLimit, ActionListener updatedJobListener) { + public void updateJob(String jobId, JobUpdate update, ByteSizeValue maxModelMemoryLimit, + Version minClusterNodeVersion, + ActionListener updatedJobListener) { GetRequest getRequest = new GetRequest(AnomalyDetectorsIndex.configIndexName(), ElasticsearchMappings.DOC_TYPE, Job.documentId(jobId)); @@ -239,7 +244,9 @@ public void onResponse(GetResponse getResponse) { return; } - long version = getResponse.getVersion(); + final long version = getResponse.getVersion(); + final long seqNo = getResponse.getSeqNo(); + final long primaryTerm = getResponse.getPrimaryTerm(); BytesReference source = getResponse.getSourceAsBytesRef(); Job.Builder jobBuilder; try { @@ -259,7 +266,7 @@ public void onResponse(GetResponse getResponse) { return; } - indexUpdatedJob(updatedJob, version, updatedJobListener); + indexUpdatedJob(updatedJob, version, seqNo, primaryTerm, minClusterNodeVersion, updatedJobListener); } @Override @@ -280,17 +287,18 @@ public interface UpdateValidator { } /** - * Similar to {@link #updateJob(String, JobUpdate, ByteSizeValue, ActionListener)} but + * Similar to {@link #updateJob(String, JobUpdate, ByteSizeValue, Version, ActionListener)} but * with an extra validation step which is called before the updated is applied. * * @param jobId The Id of the job to update * @param update The job update * @param maxModelMemoryLimit The maximum model memory allowed * @param validator The job update validator + * @param minClusterNodeVersion the minimum version of a node ifn the cluster * @param updatedJobListener Updated job listener */ public void updateJobWithValidation(String jobId, JobUpdate update, ByteSizeValue maxModelMemoryLimit, - UpdateValidator validator, ActionListener updatedJobListener) { + UpdateValidator validator, Version minClusterNodeVersion, ActionListener updatedJobListener) { GetRequest getRequest = new GetRequest(AnomalyDetectorsIndex.configIndexName(), ElasticsearchMappings.DOC_TYPE, Job.documentId(jobId)); @@ -302,7 +310,9 @@ public void onResponse(GetResponse getResponse) { return; } - long version = getResponse.getVersion(); + final long version = getResponse.getVersion(); + final long seqNo = getResponse.getSeqNo(); + final long primaryTerm = getResponse.getPrimaryTerm(); BytesReference source = getResponse.getSourceAsBytesRef(); Job originalJob; try { @@ -324,7 +334,7 @@ public void onResponse(GetResponse getResponse) { return; } - indexUpdatedJob(updatedJob, version, updatedJobListener); + indexUpdatedJob(updatedJob, version, seqNo, primaryTerm, minClusterNodeVersion, updatedJobListener); }, updatedJobListener::onFailure )); @@ -337,17 +347,22 @@ public void onFailure(Exception e) { }); } - private void indexUpdatedJob(Job updatedJob, long version, ActionListener updatedJobListener) { + private void indexUpdatedJob(Job updatedJob, long version, long seqNo, long primaryTerm, Version minClusterNodeVersion, + ActionListener updatedJobListener) { try (XContentBuilder builder = XContentFactory.jsonBuilder()) { XContentBuilder updatedSource = updatedJob.toXContent(builder, ToXContent.EMPTY_PARAMS); - IndexRequest indexRequest = client.prepareIndex(AnomalyDetectorsIndex.configIndexName(), + IndexRequestBuilder indexRequest = client.prepareIndex(AnomalyDetectorsIndex.configIndexName(), ElasticsearchMappings.DOC_TYPE, Job.documentId(updatedJob.getId())) .setSource(updatedSource) - .setVersion(version) - .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) - .request(); + .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); + if (minClusterNodeVersion.onOrAfter(Version.V_6_7_0)) { + indexRequest.setIfSeqNo(seqNo); + indexRequest.setIfPrimaryTerm(primaryTerm); + } else { + indexRequest.setVersion(version); + } - executeAsyncWithOrigin(client, ML_ORIGIN, IndexAction.INSTANCE, indexRequest, ActionListener.wrap( + executeAsyncWithOrigin(client, ML_ORIGIN, IndexAction.INSTANCE, indexRequest.request(), ActionListener.wrap( indexResponse -> { assert indexResponse.getResult() == DocWriteResponse.Result.UPDATED; updatedJobListener.onResponse(updatedJob); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobConfigProviderIT.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobConfigProviderIT.java index 0266247714dfe..3e20bdd73de07 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobConfigProviderIT.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobConfigProviderIT.java @@ -8,6 +8,7 @@ import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.ResourceNotFoundException; +import org.elasticsearch.Version; import org.elasticsearch.action.DocWriteResponse; import org.elasticsearch.action.delete.DeleteResponse; import org.elasticsearch.action.index.IndexResponse; @@ -147,8 +148,8 @@ public void testCrud() throws InterruptedException { JobUpdate jobUpdate = new JobUpdate.Builder(jobId).setDescription("This job has been updated").build(); AtomicReference updateJobResponseHolder = new AtomicReference<>(); - blockingCall(actionListener -> jobConfigProvider.updateJob(jobId, jobUpdate, new ByteSizeValue(32), actionListener), - updateJobResponseHolder, exceptionHolder); + blockingCall(actionListener -> jobConfigProvider.updateJob + (jobId, jobUpdate, new ByteSizeValue(32), Version.CURRENT, actionListener), updateJobResponseHolder, exceptionHolder); assertNull(exceptionHolder.get()); assertEquals("This job has been updated", updateJobResponseHolder.get().getDescription()); @@ -205,8 +206,8 @@ public void testUpdateWithAValidationError() throws Exception { .build(); AtomicReference updateJobResponseHolder = new AtomicReference<>(); - blockingCall(actionListener -> jobConfigProvider.updateJob(jobId, invalidUpdate, new ByteSizeValue(32), actionListener), - updateJobResponseHolder, exceptionHolder); + blockingCall(actionListener -> jobConfigProvider.updateJob(jobId, invalidUpdate, new ByteSizeValue(32), Version.CURRENT, + actionListener), updateJobResponseHolder, exceptionHolder); assertNull(updateJobResponseHolder.get()); assertNotNull(exceptionHolder.get()); assertThat(exceptionHolder.get(), instanceOf(ElasticsearchStatusException.class)); @@ -229,9 +230,8 @@ public void testUpdateWithValidator() throws Exception { AtomicReference exceptionHolder = new AtomicReference<>(); AtomicReference updateJobResponseHolder = new AtomicReference<>(); // update with the no-op validator - blockingCall(actionListener -> - jobConfigProvider.updateJobWithValidation(jobId, jobUpdate, new ByteSizeValue(32), validator, actionListener), - updateJobResponseHolder, exceptionHolder); + blockingCall(actionListener -> jobConfigProvider.updateJobWithValidation( + jobId, jobUpdate, new ByteSizeValue(32), validator, Version.CURRENT, actionListener), updateJobResponseHolder, exceptionHolder); assertNull(exceptionHolder.get()); assertNotNull(updateJobResponseHolder.get()); @@ -244,7 +244,7 @@ public void testUpdateWithValidator() throws Exception { updateJobResponseHolder.set(null); // Update with a validator that errors blockingCall(actionListener -> jobConfigProvider.updateJobWithValidation(jobId, jobUpdate, new ByteSizeValue(32), - validatorWithAnError, actionListener), + validatorWithAnError, Version.CURRENT, actionListener), updateJobResponseHolder, exceptionHolder); assertNull(updateJobResponseHolder.get()); From d7564a0231b515d6e2b37139ef0f8fa7405c4931 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Sun, 3 Feb 2019 21:33:39 +0100 Subject: [PATCH 06/37] fix testEngineGCDeletesSetting --- .../indices/settings/UpdateSettingsIT.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java b/server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java index fb3eac28b6793..d749ce367cf0b 100644 --- a/server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java @@ -21,6 +21,7 @@ import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse; +import org.elasticsearch.action.delete.DeleteResponse; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.Priority; import org.elasticsearch.common.settings.Setting; @@ -436,17 +437,20 @@ public void testOpenCloseUpdateSettings() throws Exception { public void testEngineGCDeletesSetting() throws InterruptedException { createIndex("test"); - client().prepareIndex("test", "type", "1").setSource("f", 1).get(); // set version to 1 - client().prepareDelete("test", "type", "1").get(); // sets version to 2 - // delete is still in cache this should work & set version to 3 - client().prepareIndex("test", "type", "1").setSource("f", 2).setVersion(2).get(); + client().prepareIndex("test", "type", "1").setSource("f", 1).get(); + DeleteResponse response = client().prepareDelete("test", "type", "1").get(); + long seqNo = response.getSeqNo(); + long primaryTerm = response.getPrimaryTerm(); + // delete is still in cache this should work + client().prepareIndex("test", "type", "1").setSource("f", 2).setIfSeqNo(seqNo).setIfPrimaryTerm(primaryTerm).get(); client().admin().indices().prepareUpdateSettings("test").setSettings(Settings.builder().put("index.gc_deletes", 0)).get(); - client().prepareDelete("test", "type", "1").get(); // sets version to 4 + response = client().prepareDelete("test", "type", "1").get(); + seqNo = response.getSeqNo(); Thread.sleep(300); // wait for cache time to change TODO: this needs to be solved better. To be discussed. // delete is should not be in cache - assertThrows(client().prepareIndex("test", "type", "1").setSource("f", 3) - .setVersion(4), VersionConflictEngineException.class); + assertThrows(client().prepareIndex("test", "type", "1").setSource("f", 3).setIfSeqNo(seqNo).setIfPrimaryTerm(primaryTerm), + VersionConflictEngineException.class); } From a9f47c90234216f04295bf759a0b5fdd2781df4a Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Sun, 3 Feb 2019 21:48:27 +0100 Subject: [PATCH 07/37] fix SimpleVersioningIT --- .../action/search/SearchRequestBuilder.java | 11 +++ .../versioning/SimpleVersioningIT.java | 84 +++---------------- 2 files changed, 23 insertions(+), 72 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java index e3d63cac05069..d1e864e912196 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java @@ -224,6 +224,17 @@ public SearchRequestBuilder setVersion(boolean version) { sourceBuilder().version(version); return this; } + + /** + * Should each {@link org.elasticsearch.search.SearchHit} be returned with the + * sequence number and primary term of the last modification of the document. + */ + public SearchRequestBuilder seqNoAndPrimaryTerm(boolean seqNoAndPrimaryTerm) { + sourceBuilder().seqNoAndPrimaryTerm(seqNoAndPrimaryTerm); + return this; + } + + /** * Sets the boost a specific index will receive when the query is executed against it. diff --git a/server/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java b/server/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java index f562ace967820..62cba30692761 100644 --- a/server/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java +++ b/server/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java @@ -213,11 +213,11 @@ public void testRequireUnitsOnUpdateSettings() throws Exception { } } - public void testInternalVersioningInitialDelete() throws Exception { + public void testCompareAndSEtInitialDelete() throws Exception { createIndex("test"); ensureGreen(); - assertThrows(client().prepareDelete("test", "type", "1").setVersion(17).execute(), + assertThrows(client().prepareDelete("test", "type", "1").setIfSeqNo(17).setIfPrimaryTerm(10).execute(), VersionConflictEngineException.class); IndexResponse indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_1") @@ -225,63 +225,6 @@ public void testInternalVersioningInitialDelete() throws Exception { assertThat(indexResponse.getVersion(), equalTo(1L)); } - public void testInternalVersioning() throws Exception { - createIndex("test"); - ensureGreen(); - - IndexResponse indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_1").execute().actionGet(); - assertThat(indexResponse.getVersion(), equalTo(1L)); - - indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_2").setVersion(1).execute().actionGet(); - assertThat(indexResponse.getVersion(), equalTo(2L)); - - assertThrows( - client().prepareIndex("test", "type", "1").setSource("field1", "value1_1").setVersion(1).execute(), - VersionConflictEngineException.class); - - assertThrows( - client().prepareIndex("test", "type", "1").setSource("field1", "value1_1").setVersion(1).execute(), - VersionConflictEngineException.class); - - assertThrows( - client().prepareIndex("test", "type", "1").setCreate(true).setSource("field1", "value1_1").execute(), - VersionConflictEngineException.class); - - - assertThrows(client().prepareDelete("test", "type", "1").setVersion(1).execute(), VersionConflictEngineException.class); - assertThrows(client().prepareDelete("test", "type", "1").setVersion(1).execute(), VersionConflictEngineException.class); - - client().admin().indices().prepareRefresh().execute().actionGet(); - for (int i = 0; i < 10; i++) { - assertThat(client().prepareGet("test", "type", "1").execute().actionGet().getVersion(), equalTo(2L)); - } - - // search with versioning - for (int i = 0; i < 10; i++) { - SearchResponse searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setVersion(true).execute().actionGet(); - assertThat(searchResponse.getHits().getAt(0).getVersion(), equalTo(2L)); - } - - // search without versioning - for (int i = 0; i < 10; i++) { - SearchResponse searchResponse = client().prepareSearch().setQuery(matchAllQuery()).execute().actionGet(); - assertThat(searchResponse.getHits().getAt(0).getVersion(), equalTo(Versions.NOT_FOUND)); - } - - DeleteResponse deleteResponse = client().prepareDelete("test", "type", "1").setVersion(2).execute().actionGet(); - assertEquals(DocWriteResponse.Result.DELETED, deleteResponse.getResult()); - assertThat(deleteResponse.getVersion(), equalTo(3L)); - - assertThrows(client().prepareDelete("test", "type", "1").setVersion(2).execute(), VersionConflictEngineException.class); - - - // This is intricate - the object was deleted but a delete transaction was with the right version. We add another one - // and thus the transaction is increased. - deleteResponse = client().prepareDelete("test", "type", "1").setVersion(3).execute().actionGet(); - assertEquals(DocWriteResponse.Result.NOT_FOUND, deleteResponse.getResult()); - assertThat(deleteResponse.getVersion(), equalTo(4L)); - } - public void testCompareAndSet() { createIndex("test"); ensureGreen(); @@ -290,7 +233,7 @@ public void testCompareAndSet() { assertThat(indexResponse.getSeqNo(), equalTo(0L)); assertThat(indexResponse.getPrimaryTerm(), equalTo(1L)); - indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_2").setVersion(1).execute().actionGet(); + indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_2").setIfSeqNo(0L).setIfPrimaryTerm(1).get(); assertThat(indexResponse.getSeqNo(), equalTo(1L)); assertThat(indexResponse.getPrimaryTerm(), equalTo(1L)); @@ -353,25 +296,21 @@ public void testSimpleVersioningWithFlush() throws Exception { createIndex("test"); ensureGreen(); - IndexResponse indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_1").execute().actionGet(); - assertThat(indexResponse.getVersion(), equalTo(1L)); + IndexResponse indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_1").get(); + assertThat(indexResponse.getSeqNo(), equalTo(0L)); client().admin().indices().prepareFlush().execute().actionGet(); - indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_2").setVersion(1).execute().actionGet(); - assertThat(indexResponse.getVersion(), equalTo(2L)); + indexResponse = client().prepareIndex("test", "type", "1").setSource("field1", "value1_2").setIfSeqNo(0).setIfPrimaryTerm(1).get(); + assertThat(indexResponse.getSeqNo(), equalTo(1L)); client().admin().indices().prepareFlush().execute().actionGet(); - assertThrows(client().prepareIndex("test", "type", "1").setSource("field1", "value1_1").setVersion(1).execute(), - VersionConflictEngineException.class); - - assertThrows(client().prepareIndex("test", "type", "1").setSource("field1", "value1_1").setVersion(1).execute(), + assertThrows(client().prepareIndex("test", "type", "1").setSource("field1", "value1_1").setIfSeqNo(0).setIfPrimaryTerm(1), VersionConflictEngineException.class); - assertThrows(client().prepareIndex("test", "type", "1").setCreate(true).setSource("field1", "value1_1").execute(), + assertThrows(client().prepareIndex("test", "type", "1").setCreate(true).setSource("field1", "value1_1"), VersionConflictEngineException.class); - assertThrows(client().prepareDelete("test", "type", "1").setVersion(1).execute(), VersionConflictEngineException.class); - assertThrows(client().prepareDelete("test", "type", "1").setVersion(1).execute(), VersionConflictEngineException.class); + assertThrows(client().prepareDelete("test", "type", "1").setIfSeqNo(0).setIfPrimaryTerm(1), VersionConflictEngineException.class); for (int i = 0; i < 10; i++) { assertThat(client().prepareGet("test", "type", "1").execute().actionGet().getVersion(), equalTo(2L)); @@ -380,10 +319,11 @@ public void testSimpleVersioningWithFlush() throws Exception { client().admin().indices().prepareRefresh().execute().actionGet(); for (int i = 0; i < 10; i++) { - SearchResponse searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setVersion(true). + SearchResponse searchResponse = client().prepareSearch().setQuery(matchAllQuery()).setVersion(true).seqNoAndPrimaryTerm(true). execute().actionGet(); assertHitCount(searchResponse, 1); assertThat(searchResponse.getHits().getAt(0).getVersion(), equalTo(2L)); + assertThat(searchResponse.getHits().getAt(0).getSeqNo(), equalTo(1L)); } } From fc97c1ab5d0686e2feefa79df03821829a29ff61 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Sun, 3 Feb 2019 21:56:13 +0100 Subject: [PATCH 08/37] fix BulkWithUpdatesIT --- .../action/bulk/BulkWithUpdatesIT.java | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java b/server/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java index 277c130cebb1b..ed820564e97dd 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java @@ -195,7 +195,7 @@ public void testBulkUpdateSimple() throws Exception { assertThat(((Number) getResponse.getSource().get("field")).longValue(), equalTo(4L)); } - public void testBulkVersioning() throws Exception { + public void testBulkWithCAS() throws Exception { createIndex("test"); ensureGreen(); BulkResponse bulkResponse = client().prepareBulk() @@ -204,20 +204,22 @@ public void testBulkVersioning() throws Exception { .add(client().prepareIndex("test", "type", "1").setSource("field", "2")).get(); assertEquals(DocWriteResponse.Result.CREATED, bulkResponse.getItems()[0].getResponse().getResult()); - assertThat(bulkResponse.getItems()[0].getResponse().getVersion(), equalTo(1L)); + assertThat(bulkResponse.getItems()[0].getResponse().getSeqNo(), equalTo(0L)); assertEquals(DocWriteResponse.Result.CREATED, bulkResponse.getItems()[1].getResponse().getResult()); - assertThat(bulkResponse.getItems()[1].getResponse().getVersion(), equalTo(1L)); + assertThat(bulkResponse.getItems()[1].getResponse().getSeqNo(), equalTo(1L)); assertEquals(DocWriteResponse.Result.UPDATED, bulkResponse.getItems()[2].getResponse().getResult()); - assertThat(bulkResponse.getItems()[2].getResponse().getVersion(), equalTo(2L)); + assertThat(bulkResponse.getItems()[2].getResponse().getSeqNo(), equalTo(2L)); bulkResponse = client().prepareBulk() - .add(client().prepareUpdate("test", "type", "1").setVersion(4L).setDoc(Requests.INDEX_CONTENT_TYPE, "field", "2")) + .add(client().prepareUpdate("test", "type", "1").setIfSeqNo(40L).setIfPrimaryTerm(20) + .setDoc(Requests.INDEX_CONTENT_TYPE, "field", "2")) .add(client().prepareUpdate("test", "type", "2").setDoc(Requests.INDEX_CONTENT_TYPE, "field", "2")) - .add(client().prepareUpdate("test", "type", "1").setVersion(2L).setDoc(Requests.INDEX_CONTENT_TYPE, "field", "3")).get(); + .add(client().prepareUpdate("test", "type", "1").setIfSeqNo(2L).setIfPrimaryTerm(1) + .setDoc(Requests.INDEX_CONTENT_TYPE, "field", "3")).get(); assertThat(bulkResponse.getItems()[0].getFailureMessage(), containsString("version conflict")); - assertThat(bulkResponse.getItems()[1].getResponse().getVersion(), equalTo(2L)); - assertThat(bulkResponse.getItems()[2].getResponse().getVersion(), equalTo(3L)); + assertThat(bulkResponse.getItems()[1].getResponse().getSeqNo(), equalTo(3L)); + assertThat(bulkResponse.getItems()[2].getResponse().getSeqNo(), equalTo(4L)); bulkResponse = client().prepareBulk() .add(client().prepareIndex("test", "type", "e1") @@ -237,9 +239,9 @@ public void testBulkVersioning() throws Exception { bulkResponse = client().prepareBulk() .add(client().prepareUpdate("test", "type", "e1") - .setDoc(Requests.INDEX_CONTENT_TYPE, "field", "2").setVersion(10)) // INTERNAL + .setDoc(Requests.INDEX_CONTENT_TYPE, "field", "2").setIfSeqNo(10L).setIfPrimaryTerm(1)) .add(client().prepareUpdate("test", "type", "e1") - .setDoc(Requests.INDEX_CONTENT_TYPE, "field", "3").setVersion(13).setVersionType(VersionType.INTERNAL)) + .setDoc(Requests.INDEX_CONTENT_TYPE, "field", "3").setIfSeqNo(20L).setIfPrimaryTerm(1)) .get(); assertThat(bulkResponse.getItems()[0].getFailureMessage(), containsString("version conflict")); From 3617f0650c37b3ee16d00bcd2ac56eb98d5674f0 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Sun, 3 Feb 2019 22:04:58 +0100 Subject: [PATCH 09/37] move DatafeedConfigProvider to seq No --- .../action/TransportUpdateDatafeedAction.java | 2 +- .../persistence/DatafeedConfigProvider.java | 28 +++++++++++++------ .../integration/DatafeedConfigProviderIT.java | 7 +++-- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateDatafeedAction.java index 09a8f219afcf4..443e66b81e785 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateDatafeedAction.java @@ -87,7 +87,7 @@ protected void masterOperation(UpdateDatafeedAction.Request request, ClusterStat CheckedConsumer updateConsumer = ok -> { datafeedConfigProvider.updateDatefeedConfig(request.getUpdate().getId(), request.getUpdate(), headers, - jobConfigProvider::validateDatafeedJob, + jobConfigProvider::validateDatafeedJob, clusterService.state().nodes().getMinNodeVersion(), ActionListener.wrap( updatedConfig -> listener.onResponse(new PutDatafeedAction.Response(updatedConfig)), listener::onFailure diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/persistence/DatafeedConfigProvider.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/persistence/DatafeedConfigProvider.java index 36e71de8bcba1..7d11173e258b1 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/persistence/DatafeedConfigProvider.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/persistence/DatafeedConfigProvider.java @@ -8,6 +8,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.DocWriteResponse; @@ -19,6 +20,7 @@ import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.index.IndexAction; import org.elasticsearch.action.index.IndexRequest; +import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchResponse; @@ -262,10 +264,12 @@ public void onFailure(Exception e) { * @param headers Datafeed headers applied with the update * @param validator BiConsumer that accepts the updated config and can perform * extra validations. {@code validator} must call the passed listener + * @param minClusterNodeVersion minimum version of nodes in cluster * @param updatedConfigListener Updated datafeed config listener */ public void updateDatefeedConfig(String datafeedId, DatafeedUpdate update, Map headers, BiConsumer> validator, + Version minClusterNodeVersion, ActionListener updatedConfigListener) { GetRequest getRequest = new GetRequest(AnomalyDetectorsIndex.configIndexName(), ElasticsearchMappings.DOC_TYPE, DatafeedConfig.documentId(datafeedId)); @@ -277,7 +281,9 @@ public void onResponse(GetResponse getResponse) { updatedConfigListener.onFailure(ExceptionsHelper.missingDatafeedException(datafeedId)); return; } - long version = getResponse.getVersion(); + final long version = getResponse.getVersion(); + final long seqNo = getResponse.getSeqNo(); + final long primaryTerm = getResponse.getPrimaryTerm(); BytesReference source = getResponse.getSourceAsBytesRef(); DatafeedConfig.Builder configBuilder; try { @@ -298,7 +304,7 @@ public void onResponse(GetResponse getResponse) { ActionListener validatedListener = ActionListener.wrap( ok -> { - indexUpdatedConfig(updatedConfig, version, ActionListener.wrap( + indexUpdatedConfig(updatedConfig, version, seqNo, primaryTerm, minClusterNodeVersion, ActionListener.wrap( indexResponse -> { assert indexResponse.getResult() == DocWriteResponse.Result.UPDATED; updatedConfigListener.onResponse(updatedConfig); @@ -318,17 +324,23 @@ public void onFailure(Exception e) { }); } - private void indexUpdatedConfig(DatafeedConfig updatedConfig, long version, ActionListener listener) { + private void indexUpdatedConfig(DatafeedConfig updatedConfig, long version, long seqNo, long primaryTerm, + Version minClusterNodeVersion, ActionListener listener) { try (XContentBuilder builder = XContentFactory.jsonBuilder()) { XContentBuilder updatedSource = updatedConfig.toXContent(builder, new ToXContent.MapParams(TO_XCONTENT_PARAMS)); - IndexRequest indexRequest = client.prepareIndex(AnomalyDetectorsIndex.configIndexName(), + IndexRequestBuilder indexRequest = client.prepareIndex(AnomalyDetectorsIndex.configIndexName(), ElasticsearchMappings.DOC_TYPE, DatafeedConfig.documentId(updatedConfig.getId())) .setSource(updatedSource) - .setVersion(version) - .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) - .request(); + .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); + + if (minClusterNodeVersion.onOrAfter(Version.V_6_7_0)) { + indexRequest.setIfSeqNo(seqNo); + indexRequest.setIfPrimaryTerm(primaryTerm); + } else { + indexRequest.setVersion(version); + } - executeAsyncWithOrigin(client, ML_ORIGIN, IndexAction.INSTANCE, indexRequest, listener); + executeAsyncWithOrigin(client, ML_ORIGIN, IndexAction.INSTANCE, indexRequest.request(), listener); } catch (IOException e) { listener.onFailure( diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedConfigProviderIT.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedConfigProviderIT.java index 9496f4ca0d8f2..00d62b7e0a933 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedConfigProviderIT.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedConfigProviderIT.java @@ -7,6 +7,7 @@ import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.ResourceNotFoundException; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.DocWriteResponse; import org.elasticsearch.action.delete.DeleteResponse; @@ -86,7 +87,7 @@ public void testCrud() throws InterruptedException { AtomicReference configHolder = new AtomicReference<>(); blockingCall(actionListener -> datafeedConfigProvider.updateDatefeedConfig(datafeedId, update.build(), updateHeaders, - (updatedConfig, listener) -> listener.onResponse(Boolean.TRUE), actionListener), + (updatedConfig, listener) -> listener.onResponse(Boolean.TRUE), Version.CURRENT, actionListener), configHolder, exceptionHolder); assertNull(exceptionHolder.get()); assertThat(configHolder.get().getIndices(), equalTo(updateIndices)); @@ -167,7 +168,7 @@ public void testUpdateWhenApplyingTheUpdateThrows() throws Exception { AtomicReference configHolder = new AtomicReference<>(); blockingCall(actionListener -> datafeedConfigProvider.updateDatefeedConfig(datafeedId, update.build(), Collections.emptyMap(), - (updatedConfig, listener) -> listener.onResponse(Boolean.TRUE), actionListener), + (updatedConfig, listener) -> listener.onResponse(Boolean.TRUE), Version.CURRENT, actionListener), configHolder, exceptionHolder); assertNull(configHolder.get()); assertNotNull(exceptionHolder.get()); @@ -193,7 +194,7 @@ public void testUpdateWithValidatorFunctionThatErrors() throws Exception { AtomicReference exceptionHolder = new AtomicReference<>(); blockingCall(actionListener -> datafeedConfigProvider.updateDatefeedConfig(datafeedId, update.build(), Collections.emptyMap(), - validateErrorFunction, actionListener), + validateErrorFunction, Version.CURRENT, actionListener), configHolder, exceptionHolder); assertNull(configHolder.get()); From de81b455944e891717d1e56d6e14b45376d8de41 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Sun, 3 Feb 2019 22:29:17 +0100 Subject: [PATCH 10/37] remove watch test with versions --- .../80_put_get_watch_with_passwords.yml | 106 ------------------ 1 file changed, 106 deletions(-) diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/80_put_get_watch_with_passwords.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/80_put_get_watch_with_passwords.yml index 74b3a20f5cff8..21e6caadd61f9 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/80_put_get_watch_with_passwords.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/80_put_get_watch_with_passwords.yml @@ -115,112 +115,6 @@ setup: } ---- -"Test putting a watch with a redacted password with old version returns an error": - - # version 1 - - do: - xpack.watcher.put_watch: - id: "watch_old_version" - body: > - { - "trigger": { - "schedule" : { "cron" : "0 0 0 1 * ? 2099" } - }, - "input": { - "http" : { - "request" : { - "host" : "host.domain", - "port" : 9200, - "path" : "/myservice", - "auth" : { - "basic" : { - "username" : "user", - "password" : "pass" - } - } - } - } - }, - "actions": { - "logging": { - "logging": { - "text": "Log me Amadeus!" - } - } - } - } - - # version 2 - - do: - xpack.watcher.put_watch: - id: "watch_old_version" - body: > - { - "trigger": { - "schedule" : { "cron" : "0 0 0 1 * ? 2099" } - }, - "input": { - "http" : { - "request" : { - "host" : "host.domain", - "port" : 9200, - "path" : "/myservice", - "auth" : { - "basic" : { - "username" : "user", - "password" : "pass" - } - } - } - } - }, - "actions": { - "logging": { - "logging": { - "text": "Log me Amadeus!" - } - } - } - } - - - # using optimistic concurrency control, this one will loose - # as if two users in the watch UI tried to update the same watch - - do: - catch: conflict - xpack.watcher.put_watch: - id: "watch_old_version" - version: 1 - body: > - { - "trigger": { - "schedule" : { "cron" : "0 0 0 1 * ? 2099" } - }, - "input": { - "http" : { - "request" : { - "host" : "host.domain", - "port" : 9200, - "path" : "/myservice", - "auth" : { - "basic" : { - "username" : "user", - "password" : "::es_redacted::" - } - } - } - } - }, - "actions": { - "logging": { - "logging": { - "text": "Log me Amadeus!" - } - } - } - } - --- "Test putting a watch with a redacted password with old seq no returns an error": - skip: From 575267dfe9803ec09cc077d328bf4b8e6ef25fa7 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 07:53:34 +0100 Subject: [PATCH 11/37] fix testFailingVersionedUpdatedOnBulk --- .../java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java b/server/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java index ed820564e97dd..9038d51d0db0a 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java @@ -473,7 +473,7 @@ public void testFailingVersionedUpdatedOnBulk() throws Exception { return; } BulkRequestBuilder requestBuilder = client().prepareBulk(); - requestBuilder.add(client().prepareUpdate("test", "type", "1").setVersion(1) + requestBuilder.add(client().prepareUpdate("test", "type", "1").setIfSeqNo(0L).setIfPrimaryTerm(1) .setDoc(Requests.INDEX_CONTENT_TYPE, "field", threadID)); responses[threadID] = requestBuilder.get(); From 0852123c354df92eeeb596880d7ddca0d989204a Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 08:10:28 +0100 Subject: [PATCH 12/37] remove "Test putting a watch with a redacted password with current version works" --- .../80_put_get_watch_with_passwords.yml | 95 ------------------- 1 file changed, 95 deletions(-) diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/80_put_get_watch_with_passwords.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/80_put_get_watch_with_passwords.yml index 21e6caadd61f9..ebef6c87d7022 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/80_put_get_watch_with_passwords.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/80_put_get_watch_with_passwords.yml @@ -284,98 +284,3 @@ setup: - match: { hits.hits.0._source.input.http.request.auth.basic.username: "new_user" } - match: { hits.hits.0._source.input.http.request.auth.basic.password: "pass" } ---- -"Test putting a watch with a redacted password with current version works": - - - do: - xpack.watcher.put_watch: - id: "my_watch_with_version" - body: > - { - "trigger": { - "schedule" : { "cron" : "0 0 0 1 * ? 2099" } - }, - "input": { - "http" : { - "request" : { - "host" : "host.domain", - "port" : 9200, - "path" : "/myservice", - "auth" : { - "basic" : { - "username" : "user", - "password" : "pass" - } - } - } - } - }, - "actions": { - "logging": { - "logging": { - "text": "Log me Amadeus!" - } - } - } - } - - - match: { _id: "my_watch_with_version" } - - match: { _version: 1 } - - # this resembles the exact update from the UI and thus should work, no password change, any change in the watch - # but correct version provided - - do: - xpack.watcher.put_watch: - id: "my_watch_with_version" - version: 1 - body: > - { - "trigger": { - "schedule" : { "cron" : "0 0 0 1 * ? 2099" } - }, - "input": { - "http" : { - "request" : { - "host" : "host.domain", - "port" : 9200, - "path" : "/myservice", - "auth" : { - "basic" : { - "username" : "user", - "password" : "::es_redacted::" - } - } - } - } - }, - "actions": { - "logging": { - "logging": { - "text": "Log me Amadeus!" - } - } - } - } - - - match: { _id: "my_watch_with_version" } - - match: { _version: 2 } - - - do: - search: - rest_total_hits_as_int: true - index: .watches - body: > - { - "query": { - "term": { - "_id": { - "value": "my_watch_with_version" - } - } - } - } - - - match: { hits.total: 1 } - - match: { hits.hits.0._id: "my_watch_with_version" } - - match: { hits.hits.0._source.input.http.request.auth.basic.password: "pass" } - From 24f92bc7723d276437a3694d4cdf98f436e78205 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 10:38:39 +0100 Subject: [PATCH 13/37] Move TokenService to seqno powered cas --- .../action/search/SearchRequestBuilder.java | 9 +++++++++ .../xpack/security/authc/TokenService.java | 15 ++++++++++----- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java index e3d63cac05069..4e9c598ba9c00 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java @@ -225,6 +225,15 @@ public SearchRequestBuilder setVersion(boolean version) { return this; } + /** + * Should each {@link org.elasticsearch.search.SearchHit} be returned with the + * sequence number and primary term of the last modification of the document. + */ + public SearchRequestBuilder seqNoAndPrimaryTerm(boolean seqNoAndPrimaryTerm) { + sourceBuilder().seqNoAndPrimaryTerm(seqNoAndPrimaryTerm); + return this; + } + /** * Sets the boost a specific index will receive when the query is executed against it. * diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java index 52c1081367451..daa20aeb9e19e 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java @@ -30,6 +30,7 @@ import org.elasticsearch.action.support.WriteRequest.RefreshPolicy; import org.elasticsearch.action.support.master.AcknowledgedRequest; import org.elasticsearch.action.update.UpdateRequest; +import org.elasticsearch.action.update.UpdateRequestBuilder; import org.elasticsearch.action.update.UpdateResponse; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.AckedClusterStateUpdateTask; @@ -744,13 +745,17 @@ private void innerRefresh(String tokenDocId, Authentication userAuth, ActionList try (StreamInput in = StreamInput.wrap(Base64.getDecoder().decode(authString))) { in.setVersion(authVersion); Authentication authentication = new Authentication(in); - UpdateRequest updateRequest = + UpdateRequestBuilder updateRequest = client.prepareUpdate(SecurityIndexManager.SECURITY_INDEX_NAME, TYPE, tokenDocId) - .setVersion(response.getVersion()) .setDoc("refresh_token", Collections.singletonMap("refreshed", true)) - .setRefreshPolicy(RefreshPolicy.WAIT_UNTIL) - .request(); - executeAsyncWithOrigin(client.threadPool().getThreadContext(), SECURITY_ORIGIN, updateRequest, + .setRefreshPolicy(RefreshPolicy.WAIT_UNTIL); + if (clusterService.state().nodes().getMinNodeVersion().onOrAfter(Version.V_6_7_0)) { + updateRequest.setIfSeqNo(response.getSeqNo()); + updateRequest.setIfPrimaryTerm(response.getPrimaryTerm()); + } else { + updateRequest.setVersion(response.getVersion()); + } + executeAsyncWithOrigin(client.threadPool().getThreadContext(), SECURITY_ORIGIN, updateRequest.request(), ActionListener.wrap( updateResponse -> createUserToken(authentication, userAuth, listener, metadata, true), e -> { From 96c9bc185425789a292fa3696fa403ed8f7e24f2 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 11:02:42 +0100 Subject: [PATCH 14/37] remove duplicate method --- .../action/search/SearchRequestBuilder.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java index 275a521b43426..96c93c974cabb 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java @@ -225,17 +225,6 @@ public SearchRequestBuilder setVersion(boolean version) { return this; } - /** - * Should each {@link org.elasticsearch.search.SearchHit} be returned with the - * sequence number and primary term of the last modification of the document. - */ - public SearchRequestBuilder seqNoAndPrimaryTerm(boolean seqNoAndPrimaryTerm) { - sourceBuilder().seqNoAndPrimaryTerm(seqNoAndPrimaryTerm); - return this; - } - - - /** * Should each {@link org.elasticsearch.search.SearchHit} be returned with the * sequence number and primary term of the last modification of the document. From 03163cebb69a76d5d38b20869dc54c8a487150e3 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 11:28:54 +0100 Subject: [PATCH 15/37] add docs --- .../migration/migrate_7_0/api.asciidoc | 16 +++++++++++ docs/resiliency/index.asciidoc | 28 +++++++++---------- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/docs/reference/migration/migrate_7_0/api.asciidoc b/docs/reference/migration/migrate_7_0/api.asciidoc index bb151edb778e2..a963bcf818165 100644 --- a/docs/reference/migration/migrate_7_0/api.asciidoc +++ b/docs/reference/migration/migrate_7_0/api.asciidoc @@ -2,6 +2,22 @@ [[breaking_70_api_changes]] === API changes +[float] +==== Internal Versioning is no longer supported for optimistic concurrency control + +Elasticsearch maintains a numeric version field for each document it stores. That field +is incremented by one with every change to the document. Until 7.0.0 the API allowed using +that field for optimistic concurrency control, i.e., making a write operation conditional +on the current document version. Sadly, that approach is flowed because the value of the +version doesn't always uniquely represent a change to the document. If a primary fails +while handling a write operation, it may expose a version that will then be reused by the +new primary. + +Due to that issue, internal versioning can no longer be used and is replaced by a new +method based on sequence numbers. See <> for more details. + +Note that the `EXTERNAL` versioning type is still fully supported. + [float] ==== Camel case and underscore parameters deprecated in 6.x have been removed A number of duplicate parameters deprecated in 6.x have been removed from diff --git a/docs/resiliency/index.asciidoc b/docs/resiliency/index.asciidoc index 27aa620a34fd2..ff7dca16b971d 100644 --- a/docs/resiliency/index.asciidoc +++ b/docs/resiliency/index.asciidoc @@ -102,20 +102,6 @@ space. The following issues have been identified: Other safeguards are tracked in the meta-issue {GIT}11511[#11511]. -[float] -=== The _version field may not uniquely identify document content during a network partition (STATUS: ONGOING) - -When a primary has been partitioned away from the cluster there is a short period of time until it detects this. During that time it will continue -indexing writes locally, thereby updating document versions. When it tries to replicate the operation, however, it will discover that it is -partitioned away. It won't acknowledge the write and will wait until the partition is resolved to negotiate with the master on how to proceed. -The master will decide to either fail any replicas which failed to index the operations on the primary or tell the primary that it has to -step down because a new primary has been chosen in the meantime. Since the old primary has already written documents, clients may already have read from -the old primary before it shuts itself down. The version numbers of these reads may not be unique if the new primary has already accepted -writes for the same document (see {GIT}19269[#19269]). - -We are currently implementing Sequence numbers {GIT}10708[#10708] which better track primary changes. Sequence numbers thus provide a basis -for uniquely identifying writes even in the presence of network partitions and will replace `_version` in operations that require this. - [float] === Relocating shards omitted by reporting infrastructure (STATUS: ONGOING) @@ -154,6 +140,20 @@ shard. == Completed +[float] +=== The _version field may not uniquely identify document content during a network partition (STATUS: DONE, v7.0.0) + +When a primary has been partitioned away from the cluster there is a short period of time until it detects this. During that time it will continue +indexing writes locally, thereby updating document versions. When it tries to replicate the operation, however, it will discover that it is +partitioned away. It won't acknowledge the write and will wait until the partition is resolved to negotiate with the master on how to proceed. +The master will decide to either fail any replicas which failed to index the operations on the primary or tell the primary that it has to +step down because a new primary has been chosen in the meantime. Since the old primary has already written documents, clients may already have read from +the old primary before it shuts itself down. The version numbers of these reads may not be unique if the new primary has already accepted +writes for the same document (see {GIT}19269[#19269]). + +We are currently implementing Sequence numbers {GIT}10708[#10708] which better track primary changes. Sequence numbers thus provide a basis +for uniquely identifying writes even in the presence of network partitions and will replace `_version` in operations that require this. + [float] === Repeated network partitions can cause cluster state updates to be lost (STATUS: DONE, v7.0.0) From 914da06f3ea84c3520be85894d68dc997ef58263 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 11:35:21 +0100 Subject: [PATCH 16/37] force one shard in testBulkWithCAS --- .../java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java b/server/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java index 9038d51d0db0a..f74137d4a418d 100644 --- a/server/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java +++ b/server/src/test/java/org/elasticsearch/action/bulk/BulkWithUpdatesIT.java @@ -31,6 +31,7 @@ import org.elasticsearch.action.update.UpdateRequestBuilder; import org.elasticsearch.action.update.UpdateResponse; import org.elasticsearch.client.Requests; +import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.VersionType; @@ -196,7 +197,7 @@ public void testBulkUpdateSimple() throws Exception { } public void testBulkWithCAS() throws Exception { - createIndex("test"); + createIndex("test", Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1).build()); ensureGreen(); BulkResponse bulkResponse = client().prepareBulk() .add(client().prepareIndex("test", "type", "1").setCreate(true).setSource("field", "1")) From 1b00d76c35248fc13b1b19a73ef63af128537b29 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 11:53:46 +0100 Subject: [PATCH 17/37] fix doc reference --- docs/reference/migration/migrate_7_0/api.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/migration/migrate_7_0/api.asciidoc b/docs/reference/migration/migrate_7_0/api.asciidoc index a963bcf818165..cb192d40b49c1 100644 --- a/docs/reference/migration/migrate_7_0/api.asciidoc +++ b/docs/reference/migration/migrate_7_0/api.asciidoc @@ -14,7 +14,7 @@ while handling a write operation, it may expose a version that will then be reus new primary. Due to that issue, internal versioning can no longer be used and is replaced by a new -method based on sequence numbers. See <> for more details. +method based on sequence numbers. See <> for more details. Note that the `EXTERNAL` versioning type is still fully supported. From 7974e3c01ea0413296d86a0cd8fe1a537ce116c2 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 12:18:40 +0100 Subject: [PATCH 18/37] remove version yml tests for updates --- .../test/update/35_other_versions.yml | 29 ------------------- .../update/36_other_versions_with_types.yml | 27 ----------------- 2 files changed, 56 deletions(-) delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/update/35_other_versions.yml delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/update/36_other_versions_with_types.yml diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/update/35_other_versions.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/update/35_other_versions.yml deleted file mode 100644 index 9740aa39edeb3..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/update/35_other_versions.yml +++ /dev/null @@ -1,29 +0,0 @@ ---- -"Not supported versions": - - - skip: - version: " - 6.99.99" - reason: types are required in requests before 7.0.0 - - - do: - catch: /Validation|Invalid/ - update: - index: test_1 - id: 1 - version: 2 - version_type: external - body: - doc: { foo: baz } - upsert: { foo: bar } - - - do: - catch: /Validation|Invalid/ - update: - index: test_1 - id: 1 - version: 2 - version_type: external_gte - body: - doc: { foo: baz } - upsert: { foo: bar } - diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/update/36_other_versions_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/update/36_other_versions_with_types.yml deleted file mode 100644 index c0ec082b91a4f..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/update/36_other_versions_with_types.yml +++ /dev/null @@ -1,27 +0,0 @@ ---- -"Not supported versions": - - - do: - catch: /Validation|Invalid/ - update: - index: test_1 - type: test - id: 1 - version: 2 - version_type: external - body: - doc: { foo: baz } - upsert: { foo: bar } - - - do: - catch: /Validation|Invalid/ - update: - index: test_1 - type: test - id: 1 - version: 2 - version_type: external_gte - body: - doc: { foo: baz } - upsert: { foo: bar } - From d84384284d428c8c81cb45c16aea57ffe2933acb Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 12:27:36 +0100 Subject: [PATCH 19/37] remove version usage in TokenService --- .../elasticsearch/xpack/security/authc/TokenService.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java index daa20aeb9e19e..2f6ad113fa108 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java @@ -749,12 +749,8 @@ private void innerRefresh(String tokenDocId, Authentication userAuth, ActionList client.prepareUpdate(SecurityIndexManager.SECURITY_INDEX_NAME, TYPE, tokenDocId) .setDoc("refresh_token", Collections.singletonMap("refreshed", true)) .setRefreshPolicy(RefreshPolicy.WAIT_UNTIL); - if (clusterService.state().nodes().getMinNodeVersion().onOrAfter(Version.V_6_7_0)) { - updateRequest.setIfSeqNo(response.getSeqNo()); - updateRequest.setIfPrimaryTerm(response.getPrimaryTerm()); - } else { - updateRequest.setVersion(response.getVersion()); - } + updateRequest.setIfSeqNo(response.getSeqNo()); + updateRequest.setIfPrimaryTerm(response.getPrimaryTerm()); executeAsyncWithOrigin(client.threadPool().getThreadContext(), SECURITY_ORIGIN, updateRequest.request(), ActionListener.wrap( updateResponse -> createUserToken(authentication, userAuth, listener, metadata, true), From 2690b88e0956049a88b2f362ad9d3b8704b694da Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 12:28:52 +0100 Subject: [PATCH 20/37] remove version usage in TransportUpdateFilterAction --- .../action/TransportUpdateFilterAction.java | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java index d8d5fe216b2a4..50f4d955af27f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.ml.action; import org.elasticsearch.ResourceNotFoundException; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.get.GetAction; import org.elasticsearch.action.get.GetRequest; @@ -68,14 +67,14 @@ public TransportUpdateFilterAction(TransportService transportService, ActionFilt @Override protected void doExecute(Task task, UpdateFilterAction.Request request, ActionListener listener) { - ActionListener filterListener = ActionListener.wrap(filterWithVersion -> { + ActionListener filterListener = ActionListener.wrap(filterWithVersion -> { updateFilter(filterWithVersion, request, listener); }, listener::onFailure); getFilterWithVersion(request.getFilterId(), filterListener); } - private void updateFilter(FilterWithVersion filterWithVersion, UpdateFilterAction.Request request, + private void updateFilter(FilterWithSeqNo filterWithVersion, UpdateFilterAction.Request request, ActionListener listener) { MlFilter filter = filterWithVersion.filter; @@ -100,19 +99,15 @@ private void updateFilter(FilterWithVersion filterWithVersion, UpdateFilterActio MlFilter updatedFilter = MlFilter.builder(filter.getId()).setDescription(description).setItems(items).build(); indexUpdatedFilter( - updatedFilter, filterWithVersion.version, filterWithVersion.seqNo, filterWithVersion.primaryTerm, request, listener); + updatedFilter, filterWithVersion.seqNo, filterWithVersion.primaryTerm, request, listener); } - private void indexUpdatedFilter(MlFilter filter, final long version, final long seqNo, final long primaryTerm, + private void indexUpdatedFilter(MlFilter filter, final long seqNo, final long primaryTerm, UpdateFilterAction.Request request, ActionListener listener) { IndexRequest indexRequest = new IndexRequest(MlMetaIndex.INDEX_NAME, MlMetaIndex.TYPE, filter.documentId()); - if (clusterService.state().nodes().getMinNodeVersion().onOrAfter(Version.V_6_7_0)) { - indexRequest.setIfSeqNo(seqNo); - indexRequest.setIfPrimaryTerm(primaryTerm); - } else { - indexRequest.version(version); - } + indexRequest.setIfSeqNo(seqNo); + indexRequest.setIfPrimaryTerm(primaryTerm); indexRequest.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); try (XContentBuilder builder = XContentFactory.jsonBuilder()) { @@ -145,7 +140,7 @@ public void onFailure(Exception e) { }); } - private void getFilterWithVersion(String filterId, ActionListener listener) { + private void getFilterWithVersion(String filterId, ActionListener listener) { GetRequest getRequest = new GetRequest(MlMetaIndex.INDEX_NAME, MlMetaIndex.TYPE, MlFilter.documentId(filterId)); executeAsyncWithOrigin(client, ML_ORIGIN, GetAction.INSTANCE, getRequest, new ActionListener() { @Override @@ -157,7 +152,7 @@ public void onResponse(GetResponse getDocResponse) { XContentParser parser = XContentFactory.xContent(XContentType.JSON) .createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, stream)) { MlFilter filter = MlFilter.LENIENT_PARSER.apply(parser, null).build(); - listener.onResponse(new FilterWithVersion(filter, getDocResponse)); + listener.onResponse(new FilterWithSeqNo(filter, getDocResponse)); } } else { this.onFailure(new ResourceNotFoundException(Messages.getMessage(Messages.FILTER_NOT_FOUND, filterId))); @@ -174,16 +169,14 @@ public void onFailure(Exception e) { }); } - private static class FilterWithVersion { + private static class FilterWithSeqNo { private final MlFilter filter; - private final long version; private final long seqNo; private final long primaryTerm; - private FilterWithVersion(MlFilter filter, GetResponse getDocResponse) { + private FilterWithSeqNo(MlFilter filter, GetResponse getDocResponse) { this.filter = filter; - this.version = getDocResponse.getVersion(); this.seqNo = getDocResponse.getSeqNo(); this.primaryTerm = getDocResponse.getPrimaryTerm(); From aa3d01beddd7f5b5c82e0b20cdac4f38e73f5ec7 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 15:19:54 +0100 Subject: [PATCH 21/37] remove cluster checks and potential usages of versions in ml --- .../ml/action/TransportOpenJobAction.java | 2 +- .../action/TransportUpdateDatafeedAction.java | 2 +- .../action/TransportUpdateFilterAction.java | 2 -- .../persistence/DatafeedConfigProvider.java | 21 ++++++------------ .../xpack/ml/job/JobManager.java | 4 ++-- .../ml/job/persistence/JobConfigProvider.java | 22 ++++++------------- .../integration/DatafeedConfigProviderIT.java | 7 +++--- .../ml/integration/JobConfigProviderIT.java | 9 ++++---- 8 files changed, 25 insertions(+), 44 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java index f873d8699b9b4..d365733eac0b4 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java @@ -514,7 +514,7 @@ public void onTimeout(TimeValue timeout) { private void clearJobFinishedTime(String jobId, ActionListener listener) { JobUpdate update = new JobUpdate.Builder(jobId).setClearFinishTime(true).build(); - jobConfigProvider.updateJob(jobId, update, null, clusterService.state().nodes().getMinNodeVersion(), ActionListener.wrap( + jobConfigProvider.updateJob(jobId, update, null, ActionListener.wrap( job -> listener.onResponse(new AcknowledgedResponse(true)), e -> { logger.error("[" + jobId + "] Failed to clear finished_time", e); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateDatafeedAction.java index 443e66b81e785..09a8f219afcf4 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateDatafeedAction.java @@ -87,7 +87,7 @@ protected void masterOperation(UpdateDatafeedAction.Request request, ClusterStat CheckedConsumer updateConsumer = ok -> { datafeedConfigProvider.updateDatefeedConfig(request.getUpdate().getId(), request.getUpdate(), headers, - jobConfigProvider::validateDatafeedJob, clusterService.state().nodes().getMinNodeVersion(), + jobConfigProvider::validateDatafeedJob, ActionListener.wrap( updatedConfig -> listener.onResponse(new PutDatafeedAction.Response(updatedConfig)), listener::onFailure diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java index 50f4d955af27f..fe5ae7eb6e8bf 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportUpdateFilterAction.java @@ -53,7 +53,6 @@ public class TransportUpdateFilterAction extends HandledTransportAction) UpdateFilterAction.Request::new); this.client = client; this.jobManager = jobManager; - this.clusterService = clusterService; } @Override diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/persistence/DatafeedConfigProvider.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/persistence/DatafeedConfigProvider.java index 7d11173e258b1..7237ab0eb9818 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/persistence/DatafeedConfigProvider.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/persistence/DatafeedConfigProvider.java @@ -8,7 +8,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.ElasticsearchParseException; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.DocWriteResponse; @@ -264,13 +263,11 @@ public void onFailure(Exception e) { * @param headers Datafeed headers applied with the update * @param validator BiConsumer that accepts the updated config and can perform * extra validations. {@code validator} must call the passed listener - * @param minClusterNodeVersion minimum version of nodes in cluster * @param updatedConfigListener Updated datafeed config listener */ public void updateDatefeedConfig(String datafeedId, DatafeedUpdate update, Map headers, - BiConsumer> validator, - Version minClusterNodeVersion, - ActionListener updatedConfigListener) { + BiConsumer> validator, + ActionListener updatedConfigListener) { GetRequest getRequest = new GetRequest(AnomalyDetectorsIndex.configIndexName(), ElasticsearchMappings.DOC_TYPE, DatafeedConfig.documentId(datafeedId)); @@ -304,7 +301,7 @@ public void onResponse(GetResponse getResponse) { ActionListener validatedListener = ActionListener.wrap( ok -> { - indexUpdatedConfig(updatedConfig, version, seqNo, primaryTerm, minClusterNodeVersion, ActionListener.wrap( + indexUpdatedConfig(updatedConfig, seqNo, primaryTerm, ActionListener.wrap( indexResponse -> { assert indexResponse.getResult() == DocWriteResponse.Result.UPDATED; updatedConfigListener.onResponse(updatedConfig); @@ -324,8 +321,8 @@ public void onFailure(Exception e) { }); } - private void indexUpdatedConfig(DatafeedConfig updatedConfig, long version, long seqNo, long primaryTerm, - Version minClusterNodeVersion, ActionListener listener) { + private void indexUpdatedConfig(DatafeedConfig updatedConfig, long seqNo, long primaryTerm, + ActionListener listener) { try (XContentBuilder builder = XContentFactory.jsonBuilder()) { XContentBuilder updatedSource = updatedConfig.toXContent(builder, new ToXContent.MapParams(TO_XCONTENT_PARAMS)); IndexRequestBuilder indexRequest = client.prepareIndex(AnomalyDetectorsIndex.configIndexName(), @@ -333,12 +330,8 @@ private void indexUpdatedConfig(DatafeedConfig updatedConfig, long version, long .setSource(updatedSource) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); - if (minClusterNodeVersion.onOrAfter(Version.V_6_7_0)) { - indexRequest.setIfSeqNo(seqNo); - indexRequest.setIfPrimaryTerm(primaryTerm); - } else { - indexRequest.setVersion(version); - } + indexRequest.setIfSeqNo(seqNo); + indexRequest.setIfPrimaryTerm(primaryTerm); executeAsyncWithOrigin(client, ML_ORIGIN, IndexAction.INSTANCE, indexRequest.request(), listener); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java index 6696bfe1ad96a..ccd0d594eb382 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java @@ -333,7 +333,7 @@ public void updateJob(UpdateJobAction.Request request, ActionListener { jobConfigProvider.updateJobWithValidation(request.getJobId(), request.getJobUpdate(), maxModelMemoryLimit, - this::validate, clusterService.state().nodes().getMinNodeVersion(), ActionListener.wrap( + this::validate, ActionListener.wrap( updatedJob -> postJobUpdate(request, updatedJob, actionListener), actionListener::onFailure )); @@ -603,7 +603,7 @@ public void revertSnapshot(RevertModelSnapshotAction.Request request, ActionList .setModelSnapshotId(modelSnapshot.getSnapshotId()) .build(); - jobConfigProvider.updateJob(request.getJobId(), update, maxModelMemoryLimit, clusterService.state().nodes().getMinNodeVersion(), + jobConfigProvider.updateJob(request.getJobId(), update, maxModelMemoryLimit, ActionListener.wrap(job -> { auditor.info(request.getJobId(), Messages.getMessage(Messages.JOB_AUDIT_REVERTED, modelSnapshot.getDescription())); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobConfigProvider.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobConfigProvider.java index e5ee8855969a3..9423768b8ed4f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobConfigProvider.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobConfigProvider.java @@ -10,7 +10,6 @@ import org.apache.lucene.search.join.ScoreMode; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.DocWriteResponse; @@ -227,11 +226,9 @@ public void onFailure(Exception e) { * @param maxModelMemoryLimit The maximum model memory allowed. This can be {@code null} * if the job's {@link org.elasticsearch.xpack.core.ml.job.config.AnalysisLimits} * are not changed. - * @param minClusterNodeVersion the minimum version of nodes in the cluster * @param updatedJobListener Updated job listener */ public void updateJob(String jobId, JobUpdate update, ByteSizeValue maxModelMemoryLimit, - Version minClusterNodeVersion, ActionListener updatedJobListener) { GetRequest getRequest = new GetRequest(AnomalyDetectorsIndex.configIndexName(), ElasticsearchMappings.DOC_TYPE, Job.documentId(jobId)); @@ -266,7 +263,7 @@ public void onResponse(GetResponse getResponse) { return; } - indexUpdatedJob(updatedJob, version, seqNo, primaryTerm, minClusterNodeVersion, updatedJobListener); + indexUpdatedJob(updatedJob, seqNo, primaryTerm, updatedJobListener); } @Override @@ -287,18 +284,17 @@ public interface UpdateValidator { } /** - * Similar to {@link #updateJob(String, JobUpdate, ByteSizeValue, Version, ActionListener)} but + * Similar to {@link #updateJob(String, JobUpdate, ByteSizeValue, ActionListener)} but * with an extra validation step which is called before the updated is applied. * * @param jobId The Id of the job to update * @param update The job update * @param maxModelMemoryLimit The maximum model memory allowed * @param validator The job update validator - * @param minClusterNodeVersion the minimum version of a node ifn the cluster * @param updatedJobListener Updated job listener */ public void updateJobWithValidation(String jobId, JobUpdate update, ByteSizeValue maxModelMemoryLimit, - UpdateValidator validator, Version minClusterNodeVersion, ActionListener updatedJobListener) { + UpdateValidator validator, ActionListener updatedJobListener) { GetRequest getRequest = new GetRequest(AnomalyDetectorsIndex.configIndexName(), ElasticsearchMappings.DOC_TYPE, Job.documentId(jobId)); @@ -334,7 +330,7 @@ public void onResponse(GetResponse getResponse) { return; } - indexUpdatedJob(updatedJob, version, seqNo, primaryTerm, minClusterNodeVersion, updatedJobListener); + indexUpdatedJob(updatedJob, seqNo, primaryTerm, updatedJobListener); }, updatedJobListener::onFailure )); @@ -347,7 +343,7 @@ public void onFailure(Exception e) { }); } - private void indexUpdatedJob(Job updatedJob, long version, long seqNo, long primaryTerm, Version minClusterNodeVersion, + private void indexUpdatedJob(Job updatedJob, long seqNo, long primaryTerm, ActionListener updatedJobListener) { try (XContentBuilder builder = XContentFactory.jsonBuilder()) { XContentBuilder updatedSource = updatedJob.toXContent(builder, ToXContent.EMPTY_PARAMS); @@ -355,12 +351,8 @@ private void indexUpdatedJob(Job updatedJob, long version, long seqNo, long prim ElasticsearchMappings.DOC_TYPE, Job.documentId(updatedJob.getId())) .setSource(updatedSource) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); - if (minClusterNodeVersion.onOrAfter(Version.V_6_7_0)) { - indexRequest.setIfSeqNo(seqNo); - indexRequest.setIfPrimaryTerm(primaryTerm); - } else { - indexRequest.setVersion(version); - } + indexRequest.setIfSeqNo(seqNo); + indexRequest.setIfPrimaryTerm(primaryTerm); executeAsyncWithOrigin(client, ML_ORIGIN, IndexAction.INSTANCE, indexRequest.request(), ActionListener.wrap( indexResponse -> { diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedConfigProviderIT.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedConfigProviderIT.java index 00d62b7e0a933..9496f4ca0d8f2 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedConfigProviderIT.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedConfigProviderIT.java @@ -7,7 +7,6 @@ import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.ResourceNotFoundException; -import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.DocWriteResponse; import org.elasticsearch.action.delete.DeleteResponse; @@ -87,7 +86,7 @@ public void testCrud() throws InterruptedException { AtomicReference configHolder = new AtomicReference<>(); blockingCall(actionListener -> datafeedConfigProvider.updateDatefeedConfig(datafeedId, update.build(), updateHeaders, - (updatedConfig, listener) -> listener.onResponse(Boolean.TRUE), Version.CURRENT, actionListener), + (updatedConfig, listener) -> listener.onResponse(Boolean.TRUE), actionListener), configHolder, exceptionHolder); assertNull(exceptionHolder.get()); assertThat(configHolder.get().getIndices(), equalTo(updateIndices)); @@ -168,7 +167,7 @@ public void testUpdateWhenApplyingTheUpdateThrows() throws Exception { AtomicReference configHolder = new AtomicReference<>(); blockingCall(actionListener -> datafeedConfigProvider.updateDatefeedConfig(datafeedId, update.build(), Collections.emptyMap(), - (updatedConfig, listener) -> listener.onResponse(Boolean.TRUE), Version.CURRENT, actionListener), + (updatedConfig, listener) -> listener.onResponse(Boolean.TRUE), actionListener), configHolder, exceptionHolder); assertNull(configHolder.get()); assertNotNull(exceptionHolder.get()); @@ -194,7 +193,7 @@ public void testUpdateWithValidatorFunctionThatErrors() throws Exception { AtomicReference exceptionHolder = new AtomicReference<>(); blockingCall(actionListener -> datafeedConfigProvider.updateDatefeedConfig(datafeedId, update.build(), Collections.emptyMap(), - validateErrorFunction, Version.CURRENT, actionListener), + validateErrorFunction, actionListener), configHolder, exceptionHolder); assertNull(configHolder.get()); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobConfigProviderIT.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobConfigProviderIT.java index 3e20bdd73de07..f6ff80edeec02 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobConfigProviderIT.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobConfigProviderIT.java @@ -8,7 +8,6 @@ import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.ResourceNotFoundException; -import org.elasticsearch.Version; import org.elasticsearch.action.DocWriteResponse; import org.elasticsearch.action.delete.DeleteResponse; import org.elasticsearch.action.index.IndexResponse; @@ -149,7 +148,7 @@ public void testCrud() throws InterruptedException { AtomicReference updateJobResponseHolder = new AtomicReference<>(); blockingCall(actionListener -> jobConfigProvider.updateJob - (jobId, jobUpdate, new ByteSizeValue(32), Version.CURRENT, actionListener), updateJobResponseHolder, exceptionHolder); + (jobId, jobUpdate, new ByteSizeValue(32), actionListener), updateJobResponseHolder, exceptionHolder); assertNull(exceptionHolder.get()); assertEquals("This job has been updated", updateJobResponseHolder.get().getDescription()); @@ -206,7 +205,7 @@ public void testUpdateWithAValidationError() throws Exception { .build(); AtomicReference updateJobResponseHolder = new AtomicReference<>(); - blockingCall(actionListener -> jobConfigProvider.updateJob(jobId, invalidUpdate, new ByteSizeValue(32), Version.CURRENT, + blockingCall(actionListener -> jobConfigProvider.updateJob(jobId, invalidUpdate, new ByteSizeValue(32), actionListener), updateJobResponseHolder, exceptionHolder); assertNull(updateJobResponseHolder.get()); assertNotNull(exceptionHolder.get()); @@ -231,7 +230,7 @@ public void testUpdateWithValidator() throws Exception { AtomicReference updateJobResponseHolder = new AtomicReference<>(); // update with the no-op validator blockingCall(actionListener -> jobConfigProvider.updateJobWithValidation( - jobId, jobUpdate, new ByteSizeValue(32), validator, Version.CURRENT, actionListener), updateJobResponseHolder, exceptionHolder); + jobId, jobUpdate, new ByteSizeValue(32), validator, actionListener), updateJobResponseHolder, exceptionHolder); assertNull(exceptionHolder.get()); assertNotNull(updateJobResponseHolder.get()); @@ -244,7 +243,7 @@ public void testUpdateWithValidator() throws Exception { updateJobResponseHolder.set(null); // Update with a validator that errors blockingCall(actionListener -> jobConfigProvider.updateJobWithValidation(jobId, jobUpdate, new ByteSizeValue(32), - validatorWithAnError, Version.CURRENT, actionListener), + validatorWithAnError, actionListener), updateJobResponseHolder, exceptionHolder); assertNull(updateJobResponseHolder.get()); From 2c0ca272f422a1ad58dfb04b61fa5c78c116619b Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 15:35:05 +0100 Subject: [PATCH 22/37] remove duplicates post merge --- .../action/search/SearchRequestBuilder.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java index 24dd93605e3bb..96c93c974cabb 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java @@ -234,15 +234,6 @@ public SearchRequestBuilder seqNoAndPrimaryTerm(boolean seqNoAndPrimaryTerm) { return this; } - /** - * Should each {@link org.elasticsearch.search.SearchHit} be returned with the - * sequence number and primary term of the last modification of the document. - */ - public SearchRequestBuilder seqNoAndPrimaryTerm(boolean seqNoAndPrimaryTerm) { - sourceBuilder().seqNoAndPrimaryTerm(seqNoAndPrimaryTerm); - return this; - } - /** * Sets the boost a specific index will receive when the query is executed against it. * From 515f95f4915ffc1652e76c797a74882ba769c06e Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 17:39:28 +0100 Subject: [PATCH 23/37] add create + update if_seq_no yml tests --- .../test/create/30_if_seq_no.yml | 18 ++++++++++ .../test/create/30_internal_version.yml | 36 ------------------- .../test/create/31_if_seq_no_with_types.yml | 16 +++++++++ .../create/31_internal_version_with_types.yml | 35 ------------------ .../{20_internal_version.yml => 20_cas.yml} | 10 +++--- .../test/delete/21_cas_with_types.yml | 30 ++++++++++++++++ .../delete/21_internal_version_with_types.yml | 28 --------------- 7 files changed, 70 insertions(+), 103 deletions(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/create/30_if_seq_no.yml delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/create/30_internal_version.yml create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/create/31_if_seq_no_with_types.yml delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/create/31_internal_version_with_types.yml rename rest-api-spec/src/main/resources/rest-api-spec/test/delete/{20_internal_version.yml => 20_cas.yml} (71%) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/delete/21_cas_with_types.yml delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/delete/21_internal_version_with_types.yml diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/create/30_if_seq_no.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/create/30_if_seq_no.yml new file mode 100644 index 0000000000000..f22e1ce96b0da --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/create/30_if_seq_no.yml @@ -0,0 +1,18 @@ +--- +"Create with if seq no set": + - skip: + version: " - 6.99.99" + reason: types are required in requests before 7.0.0 + + - do: + catch: bad_request + create: + index: test + id: 3 + body: { foo: bar } + if_seq_no: 5 + if_primary_term: 1 + + - match: { status: 400 } + - match: { error.type: action_request_validation_exception } + - match: { error.reason: "Validation Failed: 1: create operations do not support compare and set. use index instead;" } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/create/30_internal_version.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/create/30_internal_version.yml deleted file mode 100644 index 52e8e464da094..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/create/30_internal_version.yml +++ /dev/null @@ -1,36 +0,0 @@ ---- -"Internal version": - - skip: - version: " - 6.99.99" - reason: types are required in requests before 7.0.0 - - do: - create: - index: test_1 - id: 1 - body: { foo: bar } - - - match: { _version: 1} - - - do: - catch: conflict - create: - index: test_1 - id: 1 - body: { foo: bar } - ---- -"Internal versioning with explicit version": - - skip: - version: " - 6.99.99" - reason: types are required in requests before 7.0.0 - - do: - catch: bad_request - create: - index: test - id: 3 - body: { foo: bar } - version: 5 - - - match: { status: 400 } - - match: { error.type: action_request_validation_exception } - - match: { error.reason: "Validation Failed: 1: create operations do not support explicit versions. use index instead;" } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/create/31_if_seq_no_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/create/31_if_seq_no_with_types.yml new file mode 100644 index 0000000000000..206a91412d2b4 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/create/31_if_seq_no_with_types.yml @@ -0,0 +1,16 @@ +--- +"Create with if seq no set": + + - do: + catch: bad_request + create: + index: test + type: test + id: 3 + body: { foo: bar } + if_seq_no: 5 + if_primary_term: 1 + + - match: { status: 400 } + - match: { error.type: action_request_validation_exception } + - match: { error.reason: "Validation Failed: 1: create operations do not support compare and set. use index instead;" } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/create/31_internal_version_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/create/31_internal_version_with_types.yml deleted file mode 100644 index 83772828bc8f4..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/create/31_internal_version_with_types.yml +++ /dev/null @@ -1,35 +0,0 @@ ---- -"Internal version": - - - do: - create: - index: test_1 - type: test - id: 1 - body: { foo: bar } - - - match: { _version: 1} - - - do: - catch: conflict - create: - index: test_1 - type: test - id: 1 - body: { foo: bar } - ---- -"Internal versioning with explicit version": - - - do: - catch: bad_request - create: - index: test - type: test - id: 3 - body: { foo: bar } - version: 5 - - - match: { status: 400 } - - match: { error.type: action_request_validation_exception } - - match: { error.reason: "Validation Failed: 1: create operations do not support explicit versions. use index instead;" } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/delete/20_internal_version.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/delete/20_cas.yml similarity index 71% rename from rest-api-spec/src/main/resources/rest-api-spec/test/delete/20_internal_version.yml rename to rest-api-spec/src/main/resources/rest-api-spec/test/delete/20_cas.yml index afe69b4fe82e5..f3c7b0acbcccd 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/delete/20_internal_version.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/delete/20_cas.yml @@ -11,19 +11,21 @@ id: 1 body: { foo: bar } - - match: { _version: 1} + - match: { _seq_no: 0 } - do: catch: conflict delete: index: test_1 id: 1 - version: 2 + if_seq_no: 2 + if_primary_term: 1 - do: delete: index: test_1 id: 1 - version: 1 + if_seq_no: 0 + if_primary_term: 1 - - match: { _version: 2 } + - match: { _seq_no: 1 } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/delete/21_cas_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/delete/21_cas_with_types.yml new file mode 100644 index 0000000000000..ef352a9bad6b1 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/delete/21_cas_with_types.yml @@ -0,0 +1,30 @@ +--- +"Internal version": + + - do: + index: + index: test_1 + type: test + id: 1 + body: { foo: bar } + + - match: { _seq_no: 0 } + + - do: + catch: conflict + delete: + index: test_1 + type: test + id: 1 + if_seq_no: 2 + if_primary_term: 1 + + - do: + delete: + index: test_1 + type: test + id: 1 + if_seq_no: 0 + if_primary_term: 1 + + - match: { _seq_no: 1 } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/delete/21_internal_version_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/delete/21_internal_version_with_types.yml deleted file mode 100644 index 3d9ddb79366f7..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/delete/21_internal_version_with_types.yml +++ /dev/null @@ -1,28 +0,0 @@ ---- -"Internal version": - - - do: - index: - index: test_1 - type: test - id: 1 - body: { foo: bar } - - - match: { _version: 1} - - - do: - catch: conflict - delete: - index: test_1 - type: test - id: 1 - version: 2 - - - do: - delete: - index: test_1 - type: test - id: 1 - version: 1 - - - match: { _version: 2 } From 7124c9f72bb0e81b0db9782c754d812811957bca Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 17:41:28 +0100 Subject: [PATCH 24/37] remove rest internal version tests --- .../test/create/30_if_seq_no.yml | 18 ---------- .../test/create/31_if_seq_no_with_types.yml | 16 --------- .../test/index/30_internal_version.yml | 36 ------------------- .../index/31_internal_version_with_types.yml | 36 ------------------- .../test/update/30_internal_version.yml | 31 ---------------- .../update/31_internal_version_with_types.yml | 30 ---------------- 6 files changed, 167 deletions(-) delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/create/30_if_seq_no.yml delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/create/31_if_seq_no_with_types.yml delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/index/30_internal_version.yml delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/index/31_internal_version_with_types.yml delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/update/30_internal_version.yml delete mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/update/31_internal_version_with_types.yml diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/create/30_if_seq_no.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/create/30_if_seq_no.yml deleted file mode 100644 index f22e1ce96b0da..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/create/30_if_seq_no.yml +++ /dev/null @@ -1,18 +0,0 @@ ---- -"Create with if seq no set": - - skip: - version: " - 6.99.99" - reason: types are required in requests before 7.0.0 - - - do: - catch: bad_request - create: - index: test - id: 3 - body: { foo: bar } - if_seq_no: 5 - if_primary_term: 1 - - - match: { status: 400 } - - match: { error.type: action_request_validation_exception } - - match: { error.reason: "Validation Failed: 1: create operations do not support compare and set. use index instead;" } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/create/31_if_seq_no_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/create/31_if_seq_no_with_types.yml deleted file mode 100644 index 206a91412d2b4..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/create/31_if_seq_no_with_types.yml +++ /dev/null @@ -1,16 +0,0 @@ ---- -"Create with if seq no set": - - - do: - catch: bad_request - create: - index: test - type: test - id: 3 - body: { foo: bar } - if_seq_no: 5 - if_primary_term: 1 - - - match: { status: 400 } - - match: { error.type: action_request_validation_exception } - - match: { error.reason: "Validation Failed: 1: create operations do not support compare and set. use index instead;" } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/index/30_internal_version.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/index/30_internal_version.yml deleted file mode 100644 index adc4f3f4b15c0..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/index/30_internal_version.yml +++ /dev/null @@ -1,36 +0,0 @@ ---- -"Internal version": - - - skip: - version: " - 6.99.99" - reason: types are required in requests before 7.0.0 - - - do: - index: - index: test_1 - id: 1 - body: { foo: bar } - - match: { _version: 1} - - - do: - index: - index: test_1 - id: 1 - body: { foo: bar } - - match: { _version: 2} - - - do: - catch: conflict - index: - index: test_1 - id: 1 - body: { foo: bar } - version: 1 - - do: - index: - index: test_1 - id: 1 - body: { foo: bar } - version: 2 - - - match: { _version: 3 } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/index/31_internal_version_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/index/31_internal_version_with_types.yml deleted file mode 100644 index 1767fbebbf966..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/index/31_internal_version_with_types.yml +++ /dev/null @@ -1,36 +0,0 @@ ---- -"Internal version": - - - do: - index: - index: test_1 - type: test - id: 1 - body: { foo: bar } - - match: { _version: 1} - - - do: - index: - index: test_1 - type: test - id: 1 - body: { foo: bar } - - match: { _version: 2} - - - do: - catch: conflict - index: - index: test_1 - type: test - id: 1 - body: { foo: bar } - version: 1 - - do: - index: - index: test_1 - type: test - id: 1 - body: { foo: bar } - version: 2 - - - match: { _version: 3 } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/update/30_internal_version.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/update/30_internal_version.yml deleted file mode 100644 index 7b474d6bc09dc..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/update/30_internal_version.yml +++ /dev/null @@ -1,31 +0,0 @@ ---- -"Internal version": - - - skip: - version: " - 6.99.99" - reason: types are required in requests before 7.0.0 - - - do: - catch: missing - update: - index: test_1 - id: 1 - version: 1 - body: - doc: { foo: baz } - - - do: - index: - index: test_1 - id: 1 - body: - doc: { foo: baz } - - - do: - catch: conflict - update: - index: test_1 - id: 1 - version: 2 - body: - doc: { foo: baz } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/update/31_internal_version_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/update/31_internal_version_with_types.yml deleted file mode 100644 index 17c4806c693ac..0000000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/update/31_internal_version_with_types.yml +++ /dev/null @@ -1,30 +0,0 @@ ---- -"Internal version": - - - do: - catch: missing - update: - index: test_1 - type: test - id: 1 - version: 1 - body: - doc: { foo: baz } - - - do: - index: - index: test_1 - type: test - id: 1 - body: - doc: { foo: baz } - - - do: - catch: conflict - update: - index: test_1 - type: test - id: 1 - version: 2 - body: - doc: { foo: baz } From f2bc6f748ac65b914c807596a5ec9f4023301478 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 18:12:26 +0100 Subject: [PATCH 25/37] fix documentation + rollback resiliency --- .../migration/migrate_7_0/api.asciidoc | 4 +-- docs/resiliency/index.asciidoc | 28 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/docs/reference/migration/migrate_7_0/api.asciidoc b/docs/reference/migration/migrate_7_0/api.asciidoc index cb192d40b49c1..6c1d03760f904 100644 --- a/docs/reference/migration/migrate_7_0/api.asciidoc +++ b/docs/reference/migration/migrate_7_0/api.asciidoc @@ -8,7 +8,7 @@ Elasticsearch maintains a numeric version field for each document it stores. That field is incremented by one with every change to the document. Until 7.0.0 the API allowed using that field for optimistic concurrency control, i.e., making a write operation conditional -on the current document version. Sadly, that approach is flowed because the value of the +on the current document version. Sadly, that approach is flawed because the value of the version doesn't always uniquely represent a change to the document. If a primary fails while handling a write operation, it may expose a version that will then be reused by the new primary. @@ -16,7 +16,7 @@ new primary. Due to that issue, internal versioning can no longer be used and is replaced by a new method based on sequence numbers. See <> for more details. -Note that the `EXTERNAL` versioning type is still fully supported. +Note that the `external` versioning type is still fully supported. [float] ==== Camel case and underscore parameters deprecated in 6.x have been removed diff --git a/docs/resiliency/index.asciidoc b/docs/resiliency/index.asciidoc index ff7dca16b971d..27aa620a34fd2 100644 --- a/docs/resiliency/index.asciidoc +++ b/docs/resiliency/index.asciidoc @@ -102,6 +102,20 @@ space. The following issues have been identified: Other safeguards are tracked in the meta-issue {GIT}11511[#11511]. +[float] +=== The _version field may not uniquely identify document content during a network partition (STATUS: ONGOING) + +When a primary has been partitioned away from the cluster there is a short period of time until it detects this. During that time it will continue +indexing writes locally, thereby updating document versions. When it tries to replicate the operation, however, it will discover that it is +partitioned away. It won't acknowledge the write and will wait until the partition is resolved to negotiate with the master on how to proceed. +The master will decide to either fail any replicas which failed to index the operations on the primary or tell the primary that it has to +step down because a new primary has been chosen in the meantime. Since the old primary has already written documents, clients may already have read from +the old primary before it shuts itself down. The version numbers of these reads may not be unique if the new primary has already accepted +writes for the same document (see {GIT}19269[#19269]). + +We are currently implementing Sequence numbers {GIT}10708[#10708] which better track primary changes. Sequence numbers thus provide a basis +for uniquely identifying writes even in the presence of network partitions and will replace `_version` in operations that require this. + [float] === Relocating shards omitted by reporting infrastructure (STATUS: ONGOING) @@ -140,20 +154,6 @@ shard. == Completed -[float] -=== The _version field may not uniquely identify document content during a network partition (STATUS: DONE, v7.0.0) - -When a primary has been partitioned away from the cluster there is a short period of time until it detects this. During that time it will continue -indexing writes locally, thereby updating document versions. When it tries to replicate the operation, however, it will discover that it is -partitioned away. It won't acknowledge the write and will wait until the partition is resolved to negotiate with the master on how to proceed. -The master will decide to either fail any replicas which failed to index the operations on the primary or tell the primary that it has to -step down because a new primary has been chosen in the meantime. Since the old primary has already written documents, clients may already have read from -the old primary before it shuts itself down. The version numbers of these reads may not be unique if the new primary has already accepted -writes for the same document (see {GIT}19269[#19269]). - -We are currently implementing Sequence numbers {GIT}10708[#10708] which better track primary changes. Sequence numbers thus provide a basis -for uniquely identifying writes even in the presence of network partitions and will replace `_version` in operations that require this. - [float] === Repeated network partitions can cause cluster state updates to be lost (STATUS: DONE, v7.0.0) From 0dddeb14885eba449ddf2bbcbbf5ae451fdae3ab Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 18:19:43 +0100 Subject: [PATCH 26/37] fix WatcherRequestConvertersTests.testPutWatch --- .../org/elasticsearch/client/WatcherRequestConvertersTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/WatcherRequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/WatcherRequestConvertersTests.java index b4dc612e341ea..19483cc201a5f 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/WatcherRequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/WatcherRequestConvertersTests.java @@ -91,6 +91,7 @@ public void testPutWatch() throws Exception { long seqNo = randomNonNegativeLong(); long ifPrimaryTerm = randomLongBetween(1, 200); putWatchRequest.setIfSeqNo(seqNo); + putWatchRequest.setIfPrimaryTerm(ifPrimaryTerm); expectedParams.put("if_seq_no", String.valueOf(seqNo)); expectedParams.put("if_primary_term", String.valueOf(ifPrimaryTerm)); } From e31f8262b5575e83d575b096a52d85ea450dac60 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Mon, 4 Feb 2019 18:42:01 +0100 Subject: [PATCH 27/37] E -> e --- .../java/org/elasticsearch/versioning/SimpleVersioningIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java b/server/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java index 62cba30692761..ffd876383ad9e 100644 --- a/server/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java +++ b/server/src/test/java/org/elasticsearch/versioning/SimpleVersioningIT.java @@ -213,7 +213,7 @@ public void testRequireUnitsOnUpdateSettings() throws Exception { } } - public void testCompareAndSEtInitialDelete() throws Exception { + public void testCompareAndSetInitialDelete() throws Exception { createIndex("test"); ensureGreen(); From c61a3948e5a05acd15956a9d617ba0e8ec8271f3 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Tue, 5 Feb 2019 07:52:44 +0100 Subject: [PATCH 28/37] fix testUpdateWhileReindexing --- .../index/reindex/UpdateByQueryWhileModifyingTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/UpdateByQueryWhileModifyingTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/UpdateByQueryWhileModifyingTests.java index 987830ddd3bc9..1c3456fe20c5f 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/UpdateByQueryWhileModifyingTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/UpdateByQueryWhileModifyingTests.java @@ -67,7 +67,7 @@ public void testUpdateWhileReindexing() throws Exception { IndexRequestBuilder index = client().prepareIndex("test", "test", "test").setSource("test", value.get()) .setRefreshPolicy(IMMEDIATE); /* - * Update by query increments the version number so concurrent + * Update by query changes the document so concurrent * indexes might get version conflict exceptions so we just * blindly retry. */ @@ -75,7 +75,7 @@ public void testUpdateWhileReindexing() throws Exception { while (true) { attempts++; try { - index.setVersion(get.getVersion()).get(); + index.setIfSeqNo(get.getSeqNo()).setIfPrimaryTerm(get.getPrimaryTerm()).get(); break; } catch (VersionConflictEngineException e) { if (attempts >= MAX_ATTEMPTS) { From e8aa73a7118d10d3afdf4cfe8031574c4ffa837f Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Tue, 5 Feb 2019 08:41:45 +0100 Subject: [PATCH 29/37] fix RequestConvertersTests.testBulk --- .../elasticsearch/client/RequestConvertersTests.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java index 95971ad40ced0..8282644a0f097 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java @@ -893,11 +893,13 @@ public void testBulk() throws IOException { if (randomBoolean()) { docWriteRequest.routing(randomAlphaOfLength(10)); } - if (randomBoolean()) { - docWriteRequest.version(randomNonNegativeLong()); - } - if (randomBoolean()) { - docWriteRequest.versionType(randomFrom(VersionType.values())); + if (opType != DocWriteRequest.OpType.UPDATE) { + if (randomBoolean()) { + docWriteRequest.version(randomNonNegativeLong()); + } + if (randomBoolean()) { + docWriteRequest.versionType(randomFrom(VersionType.values())); + } } bulkRequest.add(docWriteRequest); } From a46a763fd41ad1da460817df913fd0d942c15e0a Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Tue, 5 Feb 2019 09:38:10 +0100 Subject: [PATCH 30/37] remove version updates from RequestConvertersTests.testUpdate --- .../java/org/elasticsearch/client/RequestConvertersTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java index 34d45af4d66eb..8169caf0eac61 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java @@ -766,8 +766,6 @@ public void testUpdate() throws IOException { } } setRandomWaitForActiveShards(updateRequest::waitForActiveShards, expectedParams); - setRandomVersion(updateRequest, expectedParams); - setRandomVersionType(updateRequest::versionType, expectedParams); if (randomBoolean()) { int retryOnConflict = randomIntBetween(0, 5); updateRequest.retryOnConflict(retryOnConflict); From 7e35372cbbe16ca34742624be7cd5592586129c8 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Tue, 5 Feb 2019 12:08:53 +0100 Subject: [PATCH 31/37] Wire if_seq_no and if_primary_term in rest client bulk --- .../client/RequestConverters.java | 5 +++ .../java/org/elasticsearch/client/CrudIT.java | 16 ++++--- .../client/RequestConvertersTests.java | 15 +++++-- .../rest-api-spec/test/bulk/80_cas.yml | 42 +++++++++++++++++ .../test/bulk/81_cas_with_types.yml | 45 +++++++++++++++++++ .../rest-api-spec/test/index/30_cas.yml | 2 +- 6 files changed, 114 insertions(+), 11 deletions(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/bulk/80_cas.yml create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/bulk/81_cas_with_types.yml diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java index a30fec41b0bf3..7cfca939913e6 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java @@ -191,6 +191,11 @@ static Request bulk(BulkRequest bulkRequest) throws IOException { } } + if (action.ifSeqNo() != SequenceNumbers.UNASSIGNED_SEQ_NO) { + metadata.field("if_seq_no", action.ifSeqNo()); + metadata.field("if_primary_term", action.ifPrimaryTerm()); + } + if (opType == DocWriteRequest.OpType.INDEX || opType == DocWriteRequest.OpType.CREATE) { IndexRequest indexRequest = (IndexRequest) action; if (Strings.hasLength(indexRequest.getPipeline())) { diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java index 3bd3c79072dc9..6addd8558c879 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java @@ -104,11 +104,13 @@ public void testDelete() throws IOException { { // Testing deletion String docId = "id"; - highLevelClient().index( + IndexResponse indexResponse = highLevelClient().index( new IndexRequest("index").id(docId).source(Collections.singletonMap("foo", "bar")), RequestOptions.DEFAULT); + assertThat(indexResponse.getSeqNo(), greaterThanOrEqualTo(0L)); DeleteRequest deleteRequest = new DeleteRequest("index", docId); if (randomBoolean()) { - deleteRequest.version(1L); + deleteRequest.setIfSeqNo(indexResponse.getSeqNo()); + deleteRequest.setIfPrimaryTerm(indexResponse.getPrimaryTerm()); } DeleteResponse deleteResponse = execute(deleteRequest, highLevelClient()::delete, highLevelClient()::deleteAsync); assertEquals("index", deleteResponse.getIndex()); @@ -131,7 +133,7 @@ public void testDelete() throws IOException { String docId = "version_conflict"; highLevelClient().index( new IndexRequest("index").id( docId).source(Collections.singletonMap("foo", "bar")), RequestOptions.DEFAULT); - DeleteRequest deleteRequest = new DeleteRequest("index", docId).version(2); + DeleteRequest deleteRequest = new DeleteRequest("index", docId).setIfSeqNo(2).setIfPrimaryTerm(2); ElasticsearchException exception = expectThrows(ElasticsearchException.class, () -> execute(deleteRequest, highLevelClient()::delete, highLevelClient()::deleteAsync)); assertEquals(RestStatus.CONFLICT, exception.status()); @@ -519,7 +521,7 @@ public void testIndex() throws IOException { ElasticsearchStatusException exception = expectThrows(ElasticsearchStatusException.class, () -> { IndexRequest wrongRequest = new IndexRequest("index").id("id"); wrongRequest.source(XContentBuilder.builder(xContentType.xContent()).startObject().field("field", "test").endObject()); - wrongRequest.version(5L); + wrongRequest.setIfSeqNo(1L).setIfPrimaryTerm(5L); execute(wrongRequest, highLevelClient()::index, highLevelClient()::indexAsync); }); @@ -820,7 +822,8 @@ public void testBulk() throws IOException { if (opType == DocWriteRequest.OpType.INDEX) { IndexRequest indexRequest = new IndexRequest("index").id(id).source(source, xContentType); if (erroneous) { - indexRequest.version(12L); + indexRequest.setIfSeqNo(12L); + indexRequest.setIfPrimaryTerm(12L); } bulkRequest.add(indexRequest); @@ -1130,7 +1133,8 @@ public void afterBulk(long executionId, BulkRequest request, Throwable failure) if (opType == DocWriteRequest.OpType.INDEX) { IndexRequest indexRequest = new IndexRequest("index").id(id).source(xContentType, "id", i); if (erroneous) { - indexRequest.version(12L); + indexRequest.setIfSeqNo(12L); + indexRequest.setIfPrimaryTerm(12L); } processor.add(indexRequest); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java index b58e5ae8852d3..327a12727d23e 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java @@ -892,10 +892,15 @@ public void testBulk() throws IOException { docWriteRequest.routing(randomAlphaOfLength(10)); } if (randomBoolean()) { - docWriteRequest.version(randomNonNegativeLong()); - } - if (randomBoolean()) { - docWriteRequest.versionType(randomFrom(VersionType.values())); + if (randomBoolean()) { + docWriteRequest.version(randomNonNegativeLong()); + } + if (randomBoolean()) { + docWriteRequest.versionType(randomFrom(VersionType.values())); + } + } else if (randomBoolean()) { + docWriteRequest.setIfSeqNo(randomNonNegativeLong()); + docWriteRequest.setIfPrimaryTerm(randomLongBetween(1, 200)); } bulkRequest.add(docWriteRequest); } @@ -925,6 +930,8 @@ public void testBulk() throws IOException { assertEquals(originalRequest.routing(), parsedRequest.routing()); assertEquals(originalRequest.version(), parsedRequest.version()); assertEquals(originalRequest.versionType(), parsedRequest.versionType()); + assertEquals(originalRequest.ifSeqNo(), parsedRequest.ifSeqNo()); + assertEquals(originalRequest.ifPrimaryTerm(), parsedRequest.ifPrimaryTerm()); DocWriteRequest.OpType opType = originalRequest.opType(); if (opType == DocWriteRequest.OpType.INDEX) { diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/bulk/80_cas.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/bulk/80_cas.yml new file mode 100644 index 0000000000000..902621cfba578 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/bulk/80_cas.yml @@ -0,0 +1,42 @@ +--- +"Compare And Swap Sequence Numbers": + + - skip: + version: " - 6.99.99" + reason: typeless API are add in 7.0.0 + + - do: + index: + index: test_1 + id: 1 + body: { foo: bar } + - match: { _version: 1} + - set: { _seq_no: seqno } + - set: { _primary_term: primary_term } + + - do: + bulk: + body: + - index: + _index: test_1 + _id: 1 + if_seq_no: 10000 + if_primary_term: $primary_term + - foo: bar2 + + - match: { errors: true } + - match: { items.0.index.status: 409 } + - match: { items.0.index.error.type: version_conflict_engine_exception } + + - do: + bulk: + body: + - index: + _index: test_1 + _id: 1 + if_seq_no: $seqno + if_primary_term: $primary_term + - foo: bar2 + + - match: { errors: false} + - match: { items.0.index.status: 200 } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/bulk/81_cas_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/bulk/81_cas_with_types.yml new file mode 100644 index 0000000000000..101316e7bf504 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/bulk/81_cas_with_types.yml @@ -0,0 +1,45 @@ +--- +"Compare And Swap Sequence Numbers": + + - skip: + version: " - 6.6.99" + reason: cas operations with sequence numbers was added in 6.7 + + - do: + index: + index: test_1 + type: _doc + id: 1 + body: { foo: bar } + - match: { _version: 1} + - set: { _seq_no: seqno } + - set: { _primary_term: primary_term } + + - do: + bulk: + body: + - index: + _index: test_1 + _type: _doc + _id: 1 + if_seq_no: 10000 + if_primary_term: $primary_term + - foo: bar2 + + - match: { errors: true } + - match: { items.0.index.status: 409 } + - match: { items.0.index.error.type: version_conflict_engine_exception } + + - do: + bulk: + body: + - index: + _index: test_1 + _type: _doc + _id: 1 + if_seq_no: $seqno + if_primary_term: $primary_term + - foo: bar2 + + - match: { errors: false} + - match: { items.0.index.status: 200 } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/index/30_cas.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/index/30_cas.yml index a43ec1437a50b..550582e9816eb 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/index/30_cas.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/index/30_cas.yml @@ -3,7 +3,7 @@ - skip: version: " - 6.99.99" - reason: cas ops are introduced in 7.0.0 + reason: typesless api was introduces in 7.0 - do: index: From 2df14d48231d91af98b0aca112b438ac2fab91cb Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Tue, 5 Feb 2019 13:24:18 +0100 Subject: [PATCH 32/37] wire index and delete --- .../client/RequestConverters.java | 4 ++++ .../java/org/elasticsearch/client/CrudIT.java | 5 +++-- .../client/RequestConvertersTests.java | 22 +++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java index 7cfca939913e6..860788e0157a5 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java @@ -108,6 +108,8 @@ static Request delete(DeleteRequest deleteRequest) { parameters.withTimeout(deleteRequest.timeout()); parameters.withVersion(deleteRequest.version()); parameters.withVersionType(deleteRequest.versionType()); + parameters.withIfSeqNo(deleteRequest.ifSeqNo()); + parameters.withIfPrimaryTerm(deleteRequest.ifPrimaryTerm()); parameters.withRefreshPolicy(deleteRequest.getRefreshPolicy()); parameters.withWaitForActiveShards(deleteRequest.waitForActiveShards()); return request; @@ -324,6 +326,8 @@ static Request index(IndexRequest indexRequest) { parameters.withTimeout(indexRequest.timeout()); parameters.withVersion(indexRequest.version()); parameters.withVersionType(indexRequest.versionType()); + parameters.withIfSeqNo(indexRequest.ifSeqNo()); + parameters.withIfPrimaryTerm(indexRequest.ifPrimaryTerm()); parameters.withPipeline(indexRequest.getPipeline()); parameters.withRefreshPolicy(indexRequest.getRefreshPolicy()); parameters.withWaitForActiveShards(indexRequest.waitForActiveShards()); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java index 6addd8558c879..8daea5ff415a8 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java @@ -138,7 +138,7 @@ public void testDelete() throws IOException { () -> execute(deleteRequest, highLevelClient()::delete, highLevelClient()::deleteAsync)); assertEquals(RestStatus.CONFLICT, exception.status()); assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[_doc][" + docId + "]: " + - "version conflict, current version [1] is different than the one provided [2]]", exception.getMessage()); + "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)); } { @@ -527,7 +527,8 @@ public void testIndex() throws IOException { }); assertEquals(RestStatus.CONFLICT, exception.status()); assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[_doc][id]: " + - "version conflict, current version [2] is different than the one provided [5]]", exception.getMessage()); + "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)); } { diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java index 327a12727d23e..d347049edbf00 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.client; +import com.carrotsearch.randomizedtesting.annotations.Repeat; import org.apache.http.HttpEntity; import org.apache.http.client.methods.HttpDelete; import org.apache.http.client.methods.HttpGet; @@ -131,6 +132,7 @@ import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.nullValue; +@Repeat(iterations = 200) public class RequestConvertersTests extends ESTestCase { public void testPing() { Request request = RequestConverters.ping(); @@ -281,6 +283,7 @@ public void testDelete() { setRandomRefreshPolicy(deleteRequest::setRefreshPolicy, expectedParams); setRandomVersion(deleteRequest, expectedParams); setRandomVersionType(deleteRequest::versionType, expectedParams); + setRandomIfSeqNoAndTerm(deleteRequest, expectedParams); if (frequently()) { if (randomBoolean()) { @@ -631,6 +634,7 @@ public void testIndex() throws IOException { } else { setRandomVersion(indexRequest, expectedParams); setRandomVersionType(indexRequest::versionType, expectedParams); + setRandomIfSeqNoAndTerm(indexRequest, expectedParams); } if (frequently()) { @@ -768,6 +772,7 @@ public void testUpdate() throws IOException { setRandomWaitForActiveShards(updateRequest::waitForActiveShards, expectedParams); setRandomVersion(updateRequest, expectedParams); setRandomVersionType(updateRequest::versionType, expectedParams); + setRandomIfSeqNoAndTerm(updateRequest, expectedParams); if (randomBoolean()) { int retryOnConflict = randomIntBetween(0, 5); updateRequest.retryOnConflict(retryOnConflict); @@ -798,6 +803,7 @@ public void testUpdate() throws IOException { assertEquals(updateRequest.docAsUpsert(), parsedUpdateRequest.docAsUpsert()); assertEquals(updateRequest.detectNoop(), parsedUpdateRequest.detectNoop()); assertEquals(updateRequest.fetchSource(), parsedUpdateRequest.fetchSource()); + assertIfSeqNoAndTerm(updateRequest, parsedUpdateRequest); assertEquals(updateRequest.script(), parsedUpdateRequest.script()); if (updateRequest.doc() != null) { assertToXContentEquivalent(updateRequest.doc().source(), parsedUpdateRequest.doc().source(), xContentType); @@ -811,6 +817,22 @@ public void testUpdate() throws IOException { } } + private void assertIfSeqNoAndTerm(DocWriteRequestrequest, DocWriteRequest parsedRequest) { + assertEquals(request.ifSeqNo(), parsedRequest.ifSeqNo()); + assertEquals(request.ifPrimaryTerm(), parsedRequest.ifPrimaryTerm()); + } + + private void setRandomIfSeqNoAndTerm(DocWriteRequest request, Map expectedParams) { + if (randomBoolean()) { + final long seqNo = randomNonNegativeLong(); + request.setIfSeqNo(seqNo); + expectedParams.put("if_seq_no", Long.toString(seqNo)); + final long primaryTerm = randomLongBetween(1, 200); + request.setIfPrimaryTerm(primaryTerm); + expectedParams.put("if_primary_term", Long.toString(primaryTerm)); + } + } + public void testUpdateWithType() throws IOException { String index = randomAlphaOfLengthBetween(3, 10); String type = randomAlphaOfLengthBetween(3, 10); From 949ddb901d234b069f6ea2f7ae48f51392af928e Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Tue, 5 Feb 2019 13:36:14 +0100 Subject: [PATCH 33/37] feedback --- .../org/elasticsearch/client/RequestConvertersTests.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java index d347049edbf00..761b90e69a534 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.client; -import com.carrotsearch.randomizedtesting.annotations.Repeat; import org.apache.http.HttpEntity; import org.apache.http.client.methods.HttpDelete; import org.apache.http.client.methods.HttpGet; @@ -132,7 +131,6 @@ import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.nullValue; -@Repeat(iterations = 200) public class RequestConvertersTests extends ESTestCase { public void testPing() { Request request = RequestConverters.ping(); @@ -817,12 +815,12 @@ public void testUpdate() throws IOException { } } - private void assertIfSeqNoAndTerm(DocWriteRequestrequest, DocWriteRequest parsedRequest) { + private static void assertIfSeqNoAndTerm(DocWriteRequestrequest, DocWriteRequest parsedRequest) { assertEquals(request.ifSeqNo(), parsedRequest.ifSeqNo()); assertEquals(request.ifPrimaryTerm(), parsedRequest.ifPrimaryTerm()); } - private void setRandomIfSeqNoAndTerm(DocWriteRequest request, Map expectedParams) { + private static void setRandomIfSeqNoAndTerm(DocWriteRequest request, Map expectedParams) { if (randomBoolean()) { final long seqNo = randomNonNegativeLong(); request.setIfSeqNo(seqNo); From 621457f508960da24a8a47966c82337659e7a30e Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Tue, 5 Feb 2019 13:48:49 +0100 Subject: [PATCH 34/37] lint --- .../src/test/java/org/elasticsearch/client/CrudIT.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java index 8daea5ff415a8..e2102236cc422 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java @@ -138,7 +138,8 @@ public void testDelete() throws IOException { () -> execute(deleteRequest, highLevelClient()::delete, highLevelClient()::deleteAsync)); assertEquals(RestStatus.CONFLICT, exception.status()); assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[_doc][" + docId + "]: " + - "version conflict, required seqNo [2], primary term [2]. current document has seqNo [3] and primary term [1]]", exception.getMessage()); + "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)); } { From 6d8106698a7a3c64319e76ec485e134e0f8b058c Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Tue, 5 Feb 2019 14:47:58 +0100 Subject: [PATCH 35/37] fix CRUDDocumentationIT --- .../documentation/CRUDDocumentationIT.java | 16 +++++++++------- .../high-level/document/update.asciidoc | 5 +++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CRUDDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CRUDDocumentationIT.java index 1cd76e48ffff5..93025a5dcc809 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CRUDDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CRUDDocumentationIT.java @@ -170,7 +170,6 @@ public void testIndex() throws Exception { // tag::index-response String index = indexResponse.getIndex(); String id = indexResponse.getId(); - long version = indexResponse.getVersion(); if (indexResponse.getResult() == DocWriteResponse.Result.CREATED) { // <1> } else if (indexResponse.getResult() == DocWriteResponse.Result.UPDATED) { @@ -220,7 +219,8 @@ public void testIndex() throws Exception { IndexRequest request = new IndexRequest("posts") .id("1") .source("field", "value") - .version(1); + .setIfSeqNo(10L) + .setIfPrimaryTerm(20); try { IndexResponse response = client.index(request, RequestOptions.DEFAULT); } catch(ElasticsearchException e) { @@ -432,7 +432,8 @@ public void testUpdate() throws Exception { // tag::update-conflict UpdateRequest request = new UpdateRequest("posts", "1") .doc("field", "value") - .version(1); + .setIfSeqNo(101L) + .setIfPrimaryTerm(200L); try { UpdateResponse updateResponse = client.update( request, RequestOptions.DEFAULT); @@ -499,9 +500,10 @@ public void testUpdate() throws Exception { request.setRefreshPolicy(WriteRequest.RefreshPolicy.WAIT_UNTIL); // <1> request.setRefreshPolicy("wait_for"); // <2> // end::update-request-refresh - // tag::update-request-version - request.version(2); // <1> - // end::update-request-version + // tag::update-request-cas + request.setIfSeqNo(2L); // <1> + request.setIfPrimaryTerm(1L); // <2> + // end::update-request-request-cas // tag::update-request-detect-noop request.detectNoop(false); // <1> // end::update-request-detect-noop @@ -630,7 +632,7 @@ public void testDelete() throws Exception { // tag::delete-conflict try { DeleteResponse deleteResponse = client.delete( - new DeleteRequest("posts", "1").version(2), + new DeleteRequest("posts", "1").setIfSeqNo(100).setIfPrimaryTerm(2), RequestOptions.DEFAULT); } catch (ElasticsearchException exception) { if (exception.status() == RestStatus.CONFLICT) { diff --git a/docs/java-rest/high-level/document/update.asciidoc b/docs/java-rest/high-level/document/update.asciidoc index 3112d85512217..35300512dfc3e 100644 --- a/docs/java-rest/high-level/document/update.asciidoc +++ b/docs/java-rest/high-level/document/update.asciidoc @@ -140,9 +140,10 @@ include-tagged::{doc-tests-file}[{api}-request-source-exclude] ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- -include-tagged::{doc-tests-file}[{api}-request-version] +include-tagged::{doc-tests-file}[{api}-request-cas] -------------------------------------------------- -<1> Version +<1> ifSeqNo +<2> ifPrimaryTerm ["source","java",subs="attributes,callouts,macros"] -------------------------------------------------- From 57bafed7e7bfbde73b8e334af95d9c33f20cff74 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Tue, 5 Feb 2019 14:58:38 +0100 Subject: [PATCH 36/37] update requests don't have parameters in url --- .../java/org/elasticsearch/client/RequestConvertersTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java index 761b90e69a534..9364e2ce2d57c 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java @@ -770,7 +770,7 @@ public void testUpdate() throws IOException { setRandomWaitForActiveShards(updateRequest::waitForActiveShards, expectedParams); setRandomVersion(updateRequest, expectedParams); setRandomVersionType(updateRequest::versionType, expectedParams); - setRandomIfSeqNoAndTerm(updateRequest, expectedParams); + setRandomIfSeqNoAndTerm(updateRequest, new HashMap<>()); // if* params are passed in the body if (randomBoolean()) { int retryOnConflict = randomIntBetween(0, 5); updateRequest.retryOnConflict(retryOnConflict); From f2a74d3b362473fa0f3b92fd2f6ed23dc3b3795b Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Tue, 5 Feb 2019 15:17:15 +0100 Subject: [PATCH 37/37] line lengths, again --- .../elasticsearch/client/documentation/CRUDDocumentationIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CRUDDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CRUDDocumentationIT.java index 93025a5dcc809..32e4ac223b357 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CRUDDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CRUDDocumentationIT.java @@ -632,7 +632,7 @@ public void testDelete() throws Exception { // tag::delete-conflict try { DeleteResponse deleteResponse = client.delete( - new DeleteRequest("posts", "1").setIfSeqNo(100).setIfPrimaryTerm(2), + new DeleteRequest("posts", "1").setIfSeqNo(100).setIfPrimaryTerm(2), RequestOptions.DEFAULT); } catch (ElasticsearchException exception) { if (exception.status() == RestStatus.CONFLICT) {