Skip to content

Commit

Permalink
Version types EXTERNAL & EXTERNAL_GTE test for version equality i…
Browse files Browse the repository at this point in the history
…n read operation & disallow them in the Update API

Separate version check logic for reads and writes for all version types, which allows different behavior in these cases.
Change `VersionType.EXTERNAL` & `VersionType.EXTERNAL_GTE` to behave the same as `VersionType.INTERNAL` for read operations.
The previous behavior was fit for writes but is useless in reads.

This commit also makes the usage of `EXTERNAL` & `EXTERNAL_GTE` in the update api raise a validation error as it make cause data to
be lost.

Closes #5663 , Closes #5661, Closes #5929
  • Loading branch information
bleskes committed Apr 25, 2014
1 parent 080c4ad commit 051beb5
Show file tree
Hide file tree
Showing 22 changed files with 460 additions and 223 deletions.
12 changes: 12 additions & 0 deletions docs/reference/docs/get.asciidoc
Expand Up @@ -212,3 +212,15 @@ redirected to one of the replicas within that shard id and returns the
result. The replicas are the primary shard and its replicas within that
shard id group. This means that the more replicas we will have, the
better GET scaling we will have.


[float]
[[get-versioning]
=== Versioning support

You can use the `version` parameter to retrieve the document only if
it's current version is equal to the specified one. This behavior is the same
for all version types with the exception of version type `FORCE` which always
retrieves the document.

Note that Elasticsearch do not store older versions of documents. Only the current version can be retrieved.
8 changes: 8 additions & 0 deletions docs/reference/docs/update.asciidoc
Expand Up @@ -165,6 +165,14 @@ including:
Support `_source` to return the full updated
source.

`version` & `version_type`:: 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 it's version matches the one specified.
By setting version type to `force` you can force the new version of the document
after update (use with care! with `force` there is no guaranty the document
didn't change).Version types `external` & `external_gte` are not supported.


And also support `retry_on_conflict` which controls how many times to
retry if there is a version conflict between getting the document and
Expand Down
82 changes: 41 additions & 41 deletions rest-api-spec/api/update.json
Expand Up @@ -7,85 +7,85 @@
"paths": ["/{index}/{type}/{id}/_update"],
"parts": {
"id": {
"type" : "string",
"required" : true,
"description" : "Document ID"
"type": "string",
"required": true,
"description": "Document ID"
},
"index": {
"type" : "string",
"required" : true,
"description" : "The name of the index"
"type": "string",
"required": true,
"description": "The name of the index"
},
"type": {
"type" : "string",
"required" : true,
"description" : "The type of the document"
"type": "string",
"required": true,
"description": "The type of the document"
}
},
"params": {
"consistency": {
"type" : "enum",
"options" : ["one", "quorum", "all"],
"description" : "Explicit write consistency setting for the operation"
"type": "enum",
"options": ["one", "quorum", "all"],
"description": "Explicit write consistency setting for the operation"
},
"fields": {
"type": "list",
"description" : "A comma-separated list of fields to return in the response"
"description": "A comma-separated list of fields to return in the response"
},
"lang": {
"type" : "string",
"description" : "The script language (default: mvel)"
"type": "string",
"description": "The script language (default: mvel)"
},
"parent": {
"type" : "string",
"description" : "ID of the parent document"
"type": "string",
"description": "ID of the parent document"
},
"refresh": {
"type" : "boolean",
"description" : "Refresh the index after performing the operation"
"type": "boolean",
"description": "Refresh the index after performing the operation"
},
"replication": {
"type" : "enum",
"options" : ["sync","async"],
"default" : "sync",
"description" : "Specific replication type"
"type": "enum",
"options": ["sync", "async"],
"default": "sync",
"description": "Specific replication type"
},
"retry_on_conflict": {
"type" : "number",
"description" : "Specify how many times should the operation be retried when a conflict occurs (default: 0)"
"type": "number",
"description": "Specify how many times should the operation be retried when a conflict occurs (default: 0)"
},
"routing": {
"type" : "string",
"description" : "Specific routing value"
"type": "string",
"description": "Specific routing value"
},
"script": {
"description" : "The URL-encoded script definition (instead of using request body)"
"description": "The URL-encoded script definition (instead of using request body)"
},
"timeout": {
"type" : "time",
"description" : "Explicit operation timeout"
"type": "time",
"description": "Explicit operation timeout"
},
"timestamp": {
"type" : "time",
"description" : "Explicit timestamp for the document"
"type": "time",
"description": "Explicit timestamp for the document"
},
"ttl": {
"type" : "duration",
"description" : "Expiration time for the document"
"type": "duration",
"description": "Expiration time for the document"
},
"version" : {
"type" : "number",
"description" : "Explicit version number for concurrency control"
"version": {
"type": "number",
"description": "Explicit version number for concurrency control"
},
"version_type": {
"type" : "enum",
"options" : ["internal", "external", "external_gte", "force"],
"description" : "Specific version type"
"type": "enum",
"options": ["internal", "force"],
"description": "Specific version type"
}
}
},
"body": {
"description" : "The request definition using either `script` or partial `doc`"
"description": "The request definition using either `script` or partial `doc`"
}
}
}
115 changes: 115 additions & 0 deletions rest-api-spec/test/get/90_versions.yaml
@@ -0,0 +1,115 @@
---
"Versions":

- 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:
get:
index: test_1
type: test
id: 1
version: 2
- match: { _id: "1" }

- do:
catch: conflict
get:
index: test_1
type: test
id: 1
version: 1

- do:
get:
index: test_1
type: test
id: 1
version: 2
version_type: external
- match: { _id: "1" }

- do:
catch: conflict
get:
index: test_1
type: test
id: 1
version: 10
version_type: external

- do:
catch: conflict
get:
index: test_1
type: test
id: 1
version: 1
version_type: external

- do:
get:
index: test_1
type: test
id: 1
version: 2
version_type: external_gte
- match: { _id: "1" }

- do:
catch: conflict
get:
index: test_1
type: test
id: 1
version: 10
version_type: external_gte

- do:
catch: conflict
get:
index: test_1
type: test
id: 1
version: 1
version_type: external_gte

- do:
get:
index: test_1
type: test
id: 1
version: 2
version_type: force
- match: { _id: "1" }

- do:
get:
index: test_1
type: test
id: 1
version: 10
version_type: force
- match: { _id: "1" }

- do:
get:
index: test_1
type: test
id: 1
version: 1
version_type: force
- match: { _id: "1" }
23 changes: 0 additions & 23 deletions rest-api-spec/test/update/30_internal_version.yaml
Expand Up @@ -12,29 +12,8 @@
doc: { foo: baz }
upsert: { foo: bar }

- do:
update:
index: test_1
type: test
id: 1
body:
doc: { foo: baz }
upsert: { foo: bar }

- match: { _version: 1}

- do:
catch: conflict
update:
index: test_1
type: test
id: 1
version: 2
body:
doc: { foo: baz }
upsert: { foo: bar }

- do:
update:
index: test_1
type: test
Expand All @@ -43,5 +22,3 @@
body:
doc: { foo: baz }
upsert: { foo: bar }

- match: { _version: 2}
@@ -1,7 +1,8 @@
---
"External version":
"Not supported versions":

- do:
catch: /Validation/
update:
index: test_1
type: test
Expand All @@ -12,29 +13,15 @@
doc: { foo: baz }
upsert: { foo: bar }

- match: { _version: 2 }

- do:
catch: conflict
catch: /Validation/
update:
index: test_1
type: test
id: 1
version: 2
version_type: external
body:
doc: { foo: baz }
upsert: { foo: bar }

- do:
update:
index: test_1
type: test
id: 1
version: 3
version_type: external
version_type: external_gte

This comment has been minimized.

Copy link
@spalger

spalger Apr 25, 2014

Contributor

@bleskes I validate enum type params client side (no need to make the request) so this test fails.

Perhaps catch: request (the request should fail) or catch: param (there should be a param validation error) would be a better idea.

body:
doc: { foo: baz }
upsert: { foo: bar }

- match: { _version: 3 }
Expand Up @@ -436,7 +436,7 @@ private WriteResult shardIndexOperation(BulkShardRequest request, IndexRequest i
throw new WriteFailure(t, mappingsToUpdate);
}

assert indexRequest.versionType().validateVersion(indexRequest.version());
assert indexRequest.versionType().validateVersionForWrites(indexRequest.version());


IndexResponse indexResponse = new IndexResponse(indexRequest.index(), indexRequest.type(), indexRequest.id(), version, created);
Expand All @@ -450,7 +450,7 @@ private WriteResult shardDeleteOperation(DeleteRequest deleteRequest, IndexShard
deleteRequest.versionType(delete.versionType().versionTypeForReplicationAndRecovery());
deleteRequest.version(delete.version());

assert deleteRequest.versionType().validateVersion(deleteRequest.version());
assert deleteRequest.versionType().validateVersionForWrites(deleteRequest.version());

DeleteResponse deleteResponse = new DeleteResponse(deleteRequest.index(), deleteRequest.type(), deleteRequest.id(), delete.version(), delete.found());
return new WriteResult(deleteResponse, null, null);
Expand Down
Expand Up @@ -94,8 +94,8 @@ public ActionRequestValidationException validate() {
if (id == null) {
validationException = addValidationError("id is missing", validationException);
}
if (!versionType.validateVersion(version)) {
validationException = addValidationError("illegal version value [" + version + "] for version type ["+ versionType.name() + "]", validationException);
if (!versionType.validateVersionForWrites(version)) {
validationException = addValidationError("illegal version value [" + version + "] for version type [" + versionType.name() + "]", validationException);
}
return validationException;
}
Expand Down
Expand Up @@ -191,7 +191,7 @@ protected PrimaryResponse<DeleteResponse, DeleteRequest> shardOperationOnPrimary
request.versionType(delete.versionType().versionTypeForReplicationAndRecovery());
request.version(delete.version());

assert request.versionType().validateVersion(request.version());
assert request.versionType().validateVersionForWrites(request.version());

if (request.refresh()) {
try {
Expand Down
Expand Up @@ -120,7 +120,7 @@ protected void shardOperationOnReplica(ReplicaOperationRequest shardRequest) {
// IndexDeleteAction doesn't support version type at the moment. Hard coded for the INTERNAL version
delete = new Engine.Delete(delete, VersionType.INTERNAL.versionTypeForReplicationAndRecovery());

assert delete.versionType().validateVersion(delete.version());
assert delete.versionType().validateVersionForWrites(delete.version());

indexShard.delete(delete);

Expand Down

0 comments on commit 051beb5

Please sign in to comment.