Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate support for internal versioning for concurrency control #38451

Merged
merged 25 commits into from
Feb 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
69f6086
wip
bleskes Jan 31, 2019
c4e4b24
Merge remote-tracking branch 'upstream/6.x' into cas_deprecate_versions
bleskes Feb 5, 2019
e62feba
Merge remote-tracking branch 'upstream/6.x' into cas_deprecate_versions
bleskes Feb 5, 2019
8678df5
add delete cas rest test
bleskes Feb 5, 2019
69bfe53
add some deprecation warnings
bleskes Feb 5, 2019
cd4c85e
Wire if_seq_no and if_primary_term in rest client bulk
bleskes Feb 5, 2019
898d509
wire index and delete
bleskes Feb 5, 2019
7a0e29c
feedback
bleskes Feb 5, 2019
e4384da
update requests don't have parameters in url
bleskes Feb 5, 2019
ee601c4
Merge remote-tracking branch 'upstream/6.x' into cas_deprecate_versions
bleskes Feb 5, 2019
509c898
Merge branch 'cas_bulk_rest_6.x' into cas_deprecate_versions
bleskes Feb 5, 2019
3be42ab
indenting
bleskes Feb 5, 2019
ce44cdc
fix warning strings in rest
bleskes Feb 5, 2019
da22e35
Merge remote-tracking branch 'upstream/6.x' into cas_deprecate_versions
bleskes Feb 5, 2019
9a42490
doc tweak
bleskes Feb 5, 2019
812cbf3
fix header warnings CRUDDocumentationIT
bleskes Feb 5, 2019
d5ae6df
Merge remote-tracking branch 'upstream/6.x' into cas_deprecate_versions
bleskes Feb 5, 2019
e0193f7
line length
bleskes Feb 5, 2019
356a14a
white space
bleskes Feb 5, 2019
94f2d4d
typos
bleskes Feb 5, 2019
7a29205
fix warning headers
bleskes Feb 5, 2019
3d7d569
add float to doc
bleskes Feb 5, 2019
de31f5d
add warnings to watcher test
bleskes Feb 5, 2019
de91a27
fix crudIT
bleskes Feb 5, 2019
edbf803
skip versioned tests in mix clusters until we find a better solutions…
bleskes Feb 5, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -429,6 +438,9 @@ public void testMultiGet() throws IOException {

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());
Expand Down Expand Up @@ -479,7 +491,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);
}
Expand All @@ -490,11 +502,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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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>
Expand Down
2 changes: 2 additions & 0 deletions docs/reference/docs/index_.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions docs/reference/docs/update.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions docs/reference/migration/migrate_6_7.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
---
"Internal version":
- skip:
version: " - 6.6.99"
reason: versioned operations were deprecated in 6.7
features: warnings

- do:
index:
Expand All @@ -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 }
Original file line number Diff line number Diff line change
@@ -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 }
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
---
"Internal version":
- skip:
version: " - 6.6.99"
reason: versioned operations were deprecated in 6.7
features: warnings

- do:
index:
Expand All @@ -15,6 +19,7 @@
type: test
id: 1
body: { foo: bar }

- match: { _version: 2}

- do:
Expand All @@ -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 }
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
---
"Internal version":
- skip:
version: " - 6.6.99"
reason: versioned operations were deprecated in 6.7
features: warnings

- do:
catch: missing
Expand All @@ -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:
Expand All @@ -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])"
12 changes: 12 additions & 0 deletions server/src/main/java/org/elasticsearch/action/DocWriteRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -99,6 +102,8 @@ public ActionRequestValidationException validate() {

validationException = DocWriteRequest.validateSeqNoBasedCASParams(this, validationException);

DocWriteRequest.logDeprecationWarnings(this, DEPRECATION_LOGGER);

return validationException;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -198,6 +201,8 @@ public ActionRequestValidationException validate() {
}


DocWriteRequest.logDeprecationWarnings(this, DEPRECATION_LOGGER);

return validationException;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading