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

Default detect_noop to true 

detect_noop is pretty cheap and noop updates compartively expensive so this
feels like a sensible default.

Also had to do some testing and documentation around how _ttl works with
detect_noop.

Closes #11282
This commit is contained in:
Nik Everett 2015-05-22 12:39:27 -04:00
parent 9a1b5cf966
commit 9eb684da51
10 changed files with 149 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java
View File

@ -78,7 +78,7 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
private boolean scriptedUpsert = false;
private boolean docAsUpsert = false;
private boolean detectNoop = false;
private boolean detectNoop = true;
@Nullable
private IndexRequest doc;
@ -243,7 +243,7 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
* The script to execute. Note, make sure not to send different script each
* times and instead use script params if possible with the same
* (automatically compiled) script.
*
*
* @deprecated Use {@link #script(Script)} instead
*/
@Deprecated
@ -256,7 +256,7 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
* The script to execute. Note, make sure not to send different script each
* times and instead use script params if possible with the same
* (automatically compiled) script.
*
*
* @deprecated Use {@link #script(Script)} instead
*/
@Deprecated
@ -267,7 +267,7 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
/**
* The language of the script to execute.
*
*
* @deprecated Use {@link #script(Script)} instead
*/
@Deprecated
@ -286,7 +286,7 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
/**
* Add a script parameter.
*
*
* @deprecated Use {@link #script(Script)} instead
*/
@Deprecated
@ -311,7 +311,7 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
/**
* Sets the script parameters to use with the script.
*
*
* @deprecated Use {@link #script(Script)} instead
*/
@Deprecated
@ -338,7 +338,7 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
* The script to execute. Note, make sure not to send different script each
* times and instead use script params if possible with the same
* (automatically compiled) script.
*
*
* @deprecated Use {@link #script(Script)} instead
*/
@Deprecated
@ -360,7 +360,7 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
* The script type
* @param scriptParams
* The script parameters
*
*
* @deprecated Use {@link #script(Script)} instead
*/
@Deprecated
@ -623,7 +623,7 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
}
/**
* Should this update attempt to detect if it is a noop?
* Should this update attempt to detect if it is a noop? Defaults to true.
* @return this for chaining
*/
public UpdateRequest detectNoop(boolean detectNoop) {
@ -631,6 +631,9 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
return this;
}
/**
* Should this update attempt to detect if it is a noop? Defaults to true.
*/
public boolean detectNoop() {
return detectNoop;
}
@ -699,16 +702,15 @@ public class UpdateRequest extends InstanceShardOperationRequest<UpdateRequest>
this.docAsUpsert = shouldUpsertDoc;
return this;
}
public boolean scriptedUpsert(){
return this.scriptedUpsert;
}
public UpdateRequest scriptedUpsert(boolean scriptedUpsert) {
this.scriptedUpsert = scriptedUpsert;
return this;
}
@Override
public void readFrom(StreamInput in) throws IOException {

11
core/src/main/java/org/elasticsearch/action/update/UpdateRequestBuilder.java
View File

@ -308,6 +308,7 @@ public class UpdateRequestBuilder extends InstanceShardOperationRequestBuilder<U
/**
* Sets whether to perform extra effort to detect noop updates via docAsUpsert.
* Defautls to true.
*/
public UpdateRequestBuilder setDetectNoop(boolean detectNoop) {
request.detectNoop(detectNoop);
@ -322,4 +323,14 @@ public class UpdateRequestBuilder extends InstanceShardOperationRequestBuilder<U
request.scriptedUpsert(scriptedUpsert);
return this;
}
/**
* Set the new ttl of the document. Note that if detectNoop is true (the default)
* and the source of the document isn't changed then the ttl update won't take
* effect.
*/
public UpdateRequestBuilder setTtl(Long ttl) {
request.doc().ttl(ttl);
return this;
}
}

2
core/src/test/java/org/elasticsearch/document/BulkIT.java
View File

@ -195,7 +195,7 @@ public class BulkIT extends ESIntegTestCase {
bulkResponse = client().prepareBulk()
.add(client().prepareUpdate("test", "type", "e1").setDoc("field", "2").setVersion(10)) // INTERNAL
.add(client().prepareUpdate("test", "type", "e1").setDoc("field", "3").setVersion(20).setVersionType(VersionType.FORCE))
.add(client().prepareUpdate("test", "type", "e1").setDoc("field", "3").setVersion(20).setVersionType(VersionType.INTERNAL)).get();
.add(client().prepareUpdate("test", "type", "e1").setDoc("field", "4").setVersion(20).setVersionType(VersionType.INTERNAL)).get();
assertThat(bulkResponse.getItems()[0].getFailureMessage(), containsString("version conflict"));
assertThat(((UpdateResponse) bulkResponse.getItems()[1].getResponse()).getVersion(), equalTo(20l));

6
core/src/test/java/org/elasticsearch/search/aggregations/bucket/ChildrenIT.java
View File

@ -255,9 +255,15 @@ public class ChildrenIT extends ESIntegTestCase {
assertThat(count.getValue(), equalTo(4.));
String idToUpdate = Integer.toString(randomInt(3));
/*
* The whole point of this test is to test these things with deleted
* docs in the index so we turn off detect_noop to make sure that
* the updates cause that.
*/
UpdateResponse updateResponse = client().prepareUpdate(indexName, "child", idToUpdate)
.setParent("1")
.setDoc("count", 1)
.setDetectNoop(false)
.get();
assertThat(updateResponse.getVersion(), greaterThan(1l));
refresh();

78
core/src/test/java/org/elasticsearch/ttl/SimpleTTLIT.java
View File

@ -20,17 +20,21 @@
package org.elasticsearch.ttl;
import com.google.common.base.Predicate;
import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse;
import org.elasticsearch.action.get.GetResponse;
import org.elasticsearch.action.index.IndexResponse;
import org.elasticsearch.action.update.UpdateRequestBuilder;
import org.elasticsearch.action.update.UpdateResponse;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.get.GetField;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
import org.junit.Test;
import org.elasticsearch.test.ESIntegTestCase.Scope;
import java.io.IOException;
import java.util.Locale;
@ -39,9 +43,17 @@ import java.util.concurrent.TimeUnit;
import static org.elasticsearch.common.settings.Settings.settingsBuilder;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.test.ESIntegTestCase.Scope;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.*;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
import static org.hamcrest.Matchers.both;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.lessThan;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
@ClusterScope(scope= Scope.SUITE, numDataNodes = 1)
public class SimpleTTLIT extends ESIntegTestCase {
@ -63,7 +75,6 @@ public class SimpleTTLIT extends ESIntegTestCase {
.build();
}
@Test
public void testSimpleTTL() throws Exception {
assertAcked(prepareCreate("test")
.addMapping("type1", XContentFactory.jsonBuilder()
@ -200,7 +211,7 @@ public class SimpleTTLIT extends ESIntegTestCase {
assertThat(getResponse.isExists(), equalTo(false));
}
@Test // issue 5053
// issue 5053
public void testThatUpdatingMappingShouldNotRemoveTTLConfiguration() throws Exception {
String index = "foo";
String type = "mytype";
@ -220,6 +231,63 @@ public class SimpleTTLIT extends ESIntegTestCase {
assertTTLMappingEnabled(index, type);
}
/**
* Test that updates with detect_noop set to true (the default) that don't
* change the source don't change the ttl. This is unexpected behavior and
* documented in ttl-field.asciidoc. If this behavior changes it is safe to
* rewrite this test to reflect the new behavior and to change the
* documentation.
*/
public void testNoopUpdate() throws IOException {
assertAcked(prepareCreate("test")
.addMapping("type1", XContentFactory.jsonBuilder()
.startObject()
.startObject("type1")
.startObject("_timestamp").field("enabled", true).endObject()
.startObject("_ttl").field("enabled", true).endObject()
.endObject()
.endObject()));
ensureYellow("test");
long aLongTime = 10000000;
long firstTtl = aLongTime * 3;
long secondTtl = aLongTime * 2;
long thirdTtl = aLongTime * 1;
IndexResponse indexResponse = client().prepareIndex("test", "type1", "1").setSource("field1", "value1")
.setTTL(firstTtl).setRefresh(true).get();
assertTrue(indexResponse.isCreated());
assertThat(getTtl("type1", 1), both(lessThan(firstTtl)).and(greaterThan(secondTtl)));
// Updating with the default detect_noop without a change to the document doesn't change the ttl.
UpdateRequestBuilder update = client().prepareUpdate("test", "type1", "1").setDoc("field1", "value1").setTtl(secondTtl);
assertThat(updateAndGetTtl(update), both(lessThan(firstTtl)).and(greaterThan(secondTtl)));
// Updating with the default detect_noop with a change to the document does change the ttl.
update = client().prepareUpdate("test", "type1", "1").setDoc("field1", "value2").setTtl(secondTtl);
assertThat(updateAndGetTtl(update), both(lessThan(secondTtl)).and(greaterThan(thirdTtl)));
// Updating with detect_noop=true without a change to the document doesn't change the ttl.
update = client().prepareUpdate("test", "type1", "1").setDoc("field1", "value2").setTtl(secondTtl).setDetectNoop(true);
assertThat(updateAndGetTtl(update), both(lessThan(secondTtl)).and(greaterThan(thirdTtl)));
// Updating with detect_noop=false without a change to the document does change the ttl.
update = client().prepareUpdate("test", "type1", "1").setDoc("field1", "value2").setTtl(thirdTtl).setDetectNoop(false);
assertThat(updateAndGetTtl(update), lessThan(thirdTtl));
}
private long updateAndGetTtl(UpdateRequestBuilder update) {
UpdateResponse updateResponse = update.setFields("_ttl").get();
assertThat(updateResponse.getShardInfo().getFailed(), equalTo(0));
// You can't actually fetch _ttl from an update so we use a get.
return getTtl(updateResponse.getType(), updateResponse.getId());
}
private long getTtl(String type, Object id) {
GetResponse getResponse = client().prepareGet("test", type, id.toString()).setFields("_ttl").setRealtime(true).execute()
.actionGet();
return ((Number) getResponse.getField("_ttl").getValue()).longValue();
}
private void assertTTLMappingEnabled(String index, String type) throws IOException {
String errMsg = String.format(Locale.ROOT, "Expected ttl field mapping to be enabled for %s/%s", index, type);

27
core/src/test/java/org/elasticsearch/update/UpdateNoopIT.java
View File

@ -19,6 +19,7 @@
package org.elasticsearch.update;
import org.elasticsearch.action.update.UpdateRequestBuilder;
import org.elasticsearch.action.update.UpdateResponse;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
@ -44,8 +45,10 @@ public class UpdateNoopIT extends ESIntegTestCase {
updateAndCheckSource(4, fields("bar", null));
updateAndCheckSource(4, fields("bar", null));
updateAndCheckSource(5, fields("bar", "foo"));
// detect_noop defaults to true
updateAndCheckSource(5, null, fields("bar", "foo"));
assertEquals(3, totalNoopUpdates());
assertEquals(4, totalNoopUpdates());
}
@Test
@ -210,7 +213,8 @@ public class UpdateNoopIT extends ESIntegTestCase {
}
/**
* Totally empty requests are noop if and only if detect noops is true.
* Totally empty requests are noop if and only if detect noops is true and
* its true by default.
*/
@Test
public void totallyEmpty() throws Exception {
@ -223,6 +227,7 @@ public class UpdateNoopIT extends ESIntegTestCase {
.endObject());
update(true, 1, XContentFactory.jsonBuilder().startObject().endObject());
update(false, 2, XContentFactory.jsonBuilder().startObject().endObject());
update(null, 2, XContentFactory.jsonBuilder().startObject().endObject());
}
private XContentBuilder fields(Object... fields) throws IOException {
@ -237,17 +242,23 @@ public class UpdateNoopIT extends ESIntegTestCase {
}
private void updateAndCheckSource(long expectedVersion, XContentBuilder xContentBuilder) {
UpdateResponse updateResponse = update(true, expectedVersion, xContentBuilder);
updateAndCheckSource(expectedVersion, true, xContentBuilder);
}
private void updateAndCheckSource(long expectedVersion, Boolean detectNoop, XContentBuilder xContentBuilder) {
UpdateResponse updateResponse = update(detectNoop, expectedVersion, xContentBuilder);
assertEquals(updateResponse.getGetResult().sourceRef().toUtf8(), xContentBuilder.bytes().toUtf8());
}
private UpdateResponse update(boolean detectNoop, long expectedVersion, XContentBuilder xContentBuilder) {
UpdateResponse updateResponse = client().prepareUpdate("test", "type1", "1")
private UpdateResponse update(Boolean detectNoop, long expectedVersion, XContentBuilder xContentBuilder) {
UpdateRequestBuilder updateRequest = client().prepareUpdate("test", "type1", "1")
.setDoc(xContentBuilder.bytes().toUtf8())
.setDocAsUpsert(true)
.setDetectNoop(detectNoop)
.setFields("_source")
.execute().actionGet();
.setFields("_source");
if (detectNoop != null) {
updateRequest.setDetectNoop(detectNoop);
}
UpdateResponse updateResponse = updateRequest.get();
assertThat(updateResponse.getGetResult(), notNullValue());
assertEquals(expectedVersion, updateResponse.getVersion());
return updateResponse;

18
docs/reference/docs/update.asciidoc
View File

@ -114,25 +114,23 @@ If both `doc` and `script` is specified, then `doc` is ignored. Best is
to put your field pairs of the partial document in the script itself.
[float]
=== Detecting noop
By default if `doc` is specified then the document is always updated even
if the merging process doesn't cause any changes. Specifying `detect_noop`
as `true` will cause Elasticsearch to check if there are changes and, if
there aren't, turn the update request into a noop. For example:
=== Detecting noop updates
If `doc` is specified its value is merged with the existing `_source`. By
default the document is only reindexed if the new `_source` field differs from
the old. Setting `detect_noop` to `false` will cause Elasticsearch to always
update the document even if it hasn't changed. For example:
[source,js]
--------------------------------------------------
curl -XPOST 'localhost:9200/test/type1/1/_update' -d '{
"doc" : {
"name" : "new_name"
},
"detect_noop": true
"detect_noop": false
}'
--------------------------------------------------
If `name` was `new_name` before the request was sent then the entire update
request is ignored.
If `name` was `new_name` before the request was sent then document is still
reindexed.
[[upserts]]
[float]

4
docs/reference/mapping/fields/ttl-field.asciidoc
View File

@ -104,3 +104,7 @@ may still be retrieved before they are purged.
How many deletions are handled by a single <<docs-bulk,`bulk`>> request. The
default value is `10000`.
==== Note on `detect_noop`
If an update tries to update just the `_ttl` without changing the `_source` of
the document it's expiration time won't be updated if `detect_noop` is `true`.
In 2.1 `detect_noop` defaults to `true`.

12
docs/reference/migration/migrate_2_1.asciidoc
View File

@ -24,4 +24,14 @@ GET /my_index/_search?scroll=2m
Scroll requests sorted by `_doc` have been optimized to more efficiently resume
from where the previous request stopped, so this will have the same performance
characteristics as the former `scan` search type.
characteristics as the former `scan` search type.
=== Update changes
==== Updates now `detect_noop` by default
We've switched the default value of the `detect_noop` option from `false` to
`true`. This means that Elasticsearch will ignore updates that don't change
source unless you explicitly set `"detect_noop": false`. `detect_noop` was
always computationally cheap compared to the expense of the update which can be
thought of as a delete operation followed by an index operation.

4
rest-api-spec/src/main/resources/rest-api-spec/test/update/75_ttl.yaml
View File

@ -56,7 +56,7 @@
- lte: { _ttl: 100000}
- gt: { _ttl: 10000}
# duration
# seconds
- do:
update:
@ -66,6 +66,7 @@
body:
doc: { foo: baz }
upsert: { foo: bar }
detect_noop: false
ttl: 20s
- do:
@ -89,4 +90,3 @@
body: { foo: bar }
ttl: 20s
timestamp: 2013-06-23T18:14:40