Deprecate support for internal versioning for concurrency control (#3…

…8451)

Relates #38254
Relates #10708

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

@@ -105,7 +105,8 @@ public void testDelete() throws IOException {
             IndexResponse indexResponse = highLevelClient().index(
                     new IndexRequest("index", "type", docId).source(Collections.singletonMap("foo", "bar")), RequestOptions.DEFAULT);
             DeleteRequest deleteRequest = new DeleteRequest("index", "type", docId);
-            if (randomBoolean()) {
+            final boolean useSeqNo = randomBoolean();
+            if (useSeqNo) {
                 deleteRequest.setIfSeqNo(indexResponse.getSeqNo());
                 deleteRequest.setIfPrimaryTerm(indexResponse.getPrimaryTerm());
             } else {
@@ -117,6 +118,11 @@ public void testDelete() throws IOException {
             assertEquals("type", deleteResponse.getType());
             assertEquals(docId, deleteResponse.getId());
             assertEquals(DocWriteResponse.Result.DELETED, deleteResponse.getResult());
+            if (useSeqNo == false) {
+                assertWarnings("Usage of internal versioning for optimistic concurrency control is deprecated and will be removed." +
+                    " Please use the `if_seq_no` and `if_primary_term` parameters instead." +
+                    " (request for index [index], type [type], id [id])");
+            }
         }
         {
             // Testing non existing document
@@ -153,6 +159,9 @@ public void testDelete() throws IOException {
             } else {
                 assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[type][" + docId + "]: " +
                     "version conflict, current version [1] is different than the one provided [2]]", exception.getMessage());
+                assertWarnings("Usage of internal versioning for optimistic concurrency control is deprecated and will be removed." +
+                    " Please use the `if_seq_no` and `if_primary_term` parameters instead." +
+                    " (request for index [index], type [type], id [version_conflict])");
             }
             assertEquals("index", exception.getMetadata("es.index").get(0));
         }
@@ -430,6 +439,9 @@ public void testMultiGet() throws IOException {
     @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/pull/38451 contains a fix. sliencing for now")
     public void testIndex() throws IOException {
         final XContentType xContentType = randomFrom(XContentType.values());
+        highLevelClient().indices().create(
+            new CreateIndexRequest("index").settings(Collections.singletonMap("index.number_of_shards", "1")),
+            RequestOptions.DEFAULT);
         {
             IndexRequest indexRequest = new IndexRequest("index", "type");
             indexRequest.source(XContentBuilder.builder(xContentType.xContent()).startObject().field("test", "test").endObject());
@@ -480,7 +492,7 @@ public void testIndex() throws IOException {
                 IndexRequest wrongRequest = new IndexRequest("index", "type", "id");
                 wrongRequest.source(XContentBuilder.builder(xContentType.xContent()).startObject().field("field", "test").endObject());
                 if (seqNosForConflict) {
-                    wrongRequest.setIfSeqNo(2).setIfPrimaryTerm(2);
+                    wrongRequest.setIfSeqNo(5).setIfPrimaryTerm(2);
                 } else {
                     wrongRequest.version(5);
                 }
@@ -491,11 +503,14 @@ public void testIndex() throws IOException {
             assertEquals(RestStatus.CONFLICT, exception.status());
             if (seqNosForConflict) {
                 assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[type][id]: " +
-                        "version conflict, required seqNo [1], primary term [5]. current document has seqNo [2] and primary term [1]]",
+                        "version conflict, required seqNo [5], primary term [2]. current document has seqNo [2] and primary term [1]]",
                     exception.getMessage());
             } else {
                 assertEquals("Elasticsearch exception [type=version_conflict_engine_exception, reason=[type][id]: " +
                     "version conflict, current version [2] is different than the one provided [5]]", exception.getMessage());
+                assertWarnings("Usage of internal versioning for optimistic concurrency control is deprecated and will be removed. " +
+                    "Please use the `if_seq_no` and `if_primary_term` parameters instead. " +
+                    "(request for index [index], type [type], id [id])");
             }
             assertEquals("index", exception.getMetadata("es.index").get(0));
         }

client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/CRUDDocumentationIT.java

@@ -224,7 +224,8 @@ public void testIndex() throws Exception {
             // tag::index-conflict
             IndexRequest request = new IndexRequest("posts", "doc", "1")
                     .source("field", "value")
-                    .version(1);
+                    .setIfSeqNo(100L)
+                    .setIfPrimaryTerm(1L);
             try {
                 IndexResponse response = client.index(request, RequestOptions.DEFAULT);
             } catch(ElasticsearchException e) {
@@ -434,7 +435,8 @@ public void testUpdate() throws Exception {
             // tag::update-conflict
             UpdateRequest request = new UpdateRequest("posts", "doc", "1")
                     .doc("field", "value")
-                    .version(1);
+                    .setIfSeqNo(100L)
+                    .setIfPrimaryTerm(1L);
             try {
                 UpdateResponse updateResponse = client.update(
                         request, RequestOptions.DEFAULT);
@@ -639,8 +641,9 @@ public void testDelete() throws Exception {
             // tag::delete-conflict
             try {
                 DeleteResponse deleteResponse = client.delete(
-                        new DeleteRequest("posts", "doc", "1").version(2),
-                        RequestOptions.DEFAULT);
+                    new DeleteRequest("posts", "doc", "1")
+                        .setIfSeqNo(100L).setIfPrimaryTerm(2L),
+                    RequestOptions.DEFAULT);
             } catch (ElasticsearchException exception) {
                 if (exception.status() == RestStatus.CONFLICT) {
                     // <1>

docs/reference/docs/index_.asciidoc

@@ -389,6 +389,8 @@ the different version types and their semantics.
 
 `internal`:: only index the document if the given version is identical to the version
 of the stored document.
+deprecated[6.7.0, Please use `if_seq_no` & `if_primary_term` instead. See <<optimistic-concurrency-control>> for more details.]
+
 
 `external` or `external_gt`:: only index the document if the given version is strictly higher
 than the version of the stored document *or* if there is no existing document. The given

docs/reference/docs/update.asciidoc

@@ -338,6 +338,8 @@ The update API uses the Elasticsearch's versioning support internally to make
 sure the document doesn't change during the update. You can use the `version`
 parameter to specify that the document should only be updated if its version
 matches the one specified.
+deprecated[6.7.0, Please use `if_seq_no` & `if_primary_term` instead. See <<optimistic-concurrency-control>> for more details.]
+
 
 [NOTE]
 .The update API does not support versioning other than internal

docs/reference/migration/migrate_6_7.asciidoc

@@ -7,11 +7,25 @@
 This section discusses the changes that you need to be aware of when migrating
 your application to Elasticsearch 6.7.
 
+* <<breaking_67_indexing_changes>>
 * <<breaking_67_plugin_changes>>
 * <<breaking_67_settings_changes>>
 
 See also <<release-highlights>> and <<es-release-notes>>.
 
+[float]
+[[breaking_67_indexing_changes]]
+=== Indexing changes
+
+[float]
+==== Deprecated usage of `internal` versioning for optimistic concurrency control
+
+`internal` version may not uniquely identify a document's version if an indexed document
+wasn't fully replicated when a primary fails. As such it is unsafe to use for
+optimistic concurrency control, is deprecated and the option will no longer be available
+in Elasticsearch 7.0.0. Please use the `if_seq_no` and `if_primary_term` parameters instead.
+See <<optimistic-concurrency-control>> for more details.
+
 [float]
 [[breaking_67_plugin_changes]]
 === Plugin changes

rest-api-spec/src/main/resources/rest-api-spec/test/bulk/60_deprecated.yml

@@ -3,8 +3,8 @@
 "Deprecated parameters should produce warning in Bulk query":
 
    - skip:
-       version: " - 6.0.99"
-       reason:  some parameters are deprecated starting from 6.1, their equivalents without underscore are used instead
+       version: " - 6.6.99"
+       reason:  versioned operations were deprecated in 6.7
        features: "warnings"
 
    - do:
@@ -16,6 +16,8 @@
                { "doc": { "f1": "v2" } }
        warnings:
            - "Deprecated field [_version] used, expected [version] instead"
+           - "Usage of internal versioning for optimistic concurrency control is deprecated and will be removed. Please use the `if_seq_no` and `if_primary_term` parameters instead. (request for index [test_index], type [test_type], id [test_id_1])"
+           - "Usage of internal versioning for optimistic concurrency control is deprecated and will be removed. Please use the `if_seq_no` and `if_primary_term` parameters instead. (request for index [test_index], type [test_type], id [test_id_2])"
 
    - do:
        bulk:

rest-api-spec/src/main/resources/rest-api-spec/test/delete/20_internal_version.yml

@@ -1,5 +1,9 @@
 ---
 "Internal version":
+ - skip:
+      version: " - 6.6.99"
+      reason:  versioned operations were deprecated in 6.7
+      features: warnings
 
  - do:
       index:
@@ -17,12 +21,16 @@
           type:    test
           id:      1
           version: 2
+      warnings:
+        - "Usage of internal versioning for optimistic concurrency control is deprecated and will be removed. Please use the `if_seq_no` and `if_primary_term` parameters instead. (request for index [test_1], type [test], id [1])"
 
  - do:
       delete:
           index:   test_1
           type:    test
           id:      1
           version: 1
+      warnings:
+        - "Usage of internal versioning for optimistic concurrency control is deprecated and will be removed. Please use the `if_seq_no` and `if_primary_term` parameters instead. (request for index [test_1], type [test], id [1])"
 
  - match: { _version: 2 }

rest-api-spec/src/main/resources/rest-api-spec/test/delete/21_cas.yml

@@ -0,0 +1,33 @@
+---
+"Compare and set with sequence numbers":
+  - skip:
+      version: " - 6.5.99"
+      reason: sequence numbers can be used for CAS as of 6.6.0
+
+  - 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 }

rest-api-spec/src/main/resources/rest-api-spec/test/index/30_internal_version.yml

@@ -1,5 +1,9 @@
 ---
 "Internal version":
+ - skip:
+     version: " - 6.6.99"
+     reason:  versioned operations were deprecated in 6.7
+     features: warnings
 
  - do:
       index:
@@ -15,6 +19,7 @@
           type:   test
           id:     1
           body:   { foo: bar }
+
  - match:   { _version: 2}
 
  - do:
@@ -25,12 +30,17 @@
           id:      1
           body:    { foo: bar }
           version: 1
+      warnings:
+        - "Usage of internal versioning for optimistic concurrency control is deprecated and will be removed. Please use the `if_seq_no` and `if_primary_term` parameters instead. (request for index [test_1], type [test], id [1])"
+
  - do:
       index:
           index:   test_1
           type:    test
           id:      1
           body:    { foo: bar }
           version: 2
+      warnings:
+        - "Usage of internal versioning for optimistic concurrency control is deprecated and will be removed. Please use the `if_seq_no` and `if_primary_term` parameters instead. (request for index [test_1], type [test], id [1])"
 
  - match: { _version: 3 }

rest-api-spec/src/main/resources/rest-api-spec/test/update/30_internal_version.yml

@@ -1,5 +1,9 @@
 ---
 "Internal version":
+ - skip:
+      version: " - 6.6.99"
+      reason:  versioned operations were deprecated in 6.7
+      features: warnings
 
  - do:
       catch:        missing
@@ -10,6 +14,8 @@
           version:  1
           body:
             doc:    { foo: baz }
+      warnings:
+        - "Usage of internal versioning for optimistic concurrency control is deprecated and will be removed. Please use the `if_seq_no` and `if_primary_term` parameters instead. (request for index [test_1], type [test], id [1])"
 
  - do:
       index:
@@ -28,3 +34,5 @@
           version:  2
           body:
             doc:    { foo: baz }
+      warnings:
+        - "Usage of internal versioning for optimistic concurrency control is deprecated and will be removed. Please use the `if_seq_no` and `if_primary_term` parameters instead. (request for index [test_1], type [test], id [1])"

server/src/main/java/org/elasticsearch/action/DocWriteRequest.java

@@ -24,6 +24,7 @@
 import org.elasticsearch.action.update.UpdateRequest;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.lucene.uid.Versions;
 import org.elasticsearch.index.VersionType;
 
@@ -252,6 +253,17 @@ static void writeDocumentRequest(StreamOutput out, DocWriteRequest request)  thr
         }
     }
 
+    static void logDeprecationWarnings(DocWriteRequest request, DeprecationLogger logger) {
+        if (request.versionType() == VersionType.INTERNAL &&
+            request.version() != Versions.MATCH_ANY &&
+            request.version() != Versions.MATCH_DELETED) {
+            logger.deprecatedAndMaybeLog("occ_internal_version",
+                "Usage of internal versioning for optimistic concurrency control is deprecated and will be removed. Please use" +
+                    " the `if_seq_no` and `if_primary_term` parameters instead. (request for index [{}], type [{}], id [{}])",
+                request.index(), request.type(), request.id());
+        }
+    }
+
     static ActionRequestValidationException validateSeqNoBasedCASParams(
         DocWriteRequest request, ActionRequestValidationException validationException) {
         if (request.versionType().validateVersionForWrites(request.version()) == false) {

server/src/main/java/org/elasticsearch/action/delete/DeleteRequest.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.action.delete;
 
+import org.apache.logging.log4j.LogManager;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.CompositeIndicesRequest;
@@ -28,6 +29,7 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.lucene.uid.Versions;
 import org.elasticsearch.index.VersionType;
 import org.elasticsearch.index.shard.ShardId;
@@ -51,6 +53,7 @@
  */
 public class DeleteRequest extends ReplicatedWriteRequest<DeleteRequest>
         implements DocWriteRequest<DeleteRequest>, CompositeIndicesRequest {
+    private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(LogManager.getLogger(DeleteRequest.class));
 
     private String type;
     private String id;
@@ -99,6 +102,8 @@ public ActionRequestValidationException validate() {
 
         validationException = DocWriteRequest.validateSeqNoBasedCASParams(this, validationException);
 
+        DocWriteRequest.logDeprecationWarnings(this, DEPRECATION_LOGGER);
+
         return validationException;
     }
 

server/src/main/java/org/elasticsearch/action/index/IndexRequest.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.action.index;
 
+import org.apache.logging.log4j.LogManager;
 import org.elasticsearch.ElasticsearchGenerationException;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionRequestValidationException;
@@ -36,6 +37,7 @@
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.lucene.uid.Versions;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -73,6 +75,7 @@
  * @see org.elasticsearch.client.Client#index(IndexRequest)
  */
 public class IndexRequest extends ReplicatedWriteRequest<IndexRequest> implements DocWriteRequest<IndexRequest>, CompositeIndicesRequest {
+    private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(LogManager.getLogger(IndexRequest.class));
 
     /**
      * Max length of the source document to include into string()
@@ -198,6 +201,8 @@ public ActionRequestValidationException validate() {
         }
 
 
+        DocWriteRequest.logDeprecationWarnings(this, DEPRECATION_LOGGER);
+
         return validationException;
     }
 

server/src/main/java/org/elasticsearch/action/update/UpdateRequest.java

@@ -160,6 +160,9 @@ public ActionRequestValidationException validate() {
         if (doc == null && docAsUpsert) {
             validationException = addValidationError("doc must be specified if doc_as_upsert is enabled", validationException);
         }
+
+        DocWriteRequest.logDeprecationWarnings(this, DEPRECATION_LOGGER);
+
         return validationException;
     }
 

server/src/test/java/org/elasticsearch/action/bulk/BulkRequestTests.java

@@ -316,6 +316,8 @@ public void testToValidateUpsertRequestAndVersionInBulkRequest() throws IOExcept
         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"));
+        assertWarnings("Usage of internal versioning for optimistic concurrency control is deprecated and will be removed. " +
+            "Please use the `if_seq_no` and `if_primary_term` parameters instead. (request for index [index], type [type], id [id])");
     }
 
     public void testBulkTerminatedByNewline() throws Exception {