From 774273aac9ca6cbab3d9645f7e9ba3959b99422f Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Fri, 13 Oct 2017 10:09:41 +0200 Subject: [PATCH] Don't refres on `_flush` `_force_merge` and `_upgrade` Today all these API calls have a sideeffect of making documents visible to search requests. While this is sometimes desired it's an unnecessary sideeffect and now that we have an internal (engine-private) index reader (#26972) we artificially add a refresh call for bwc. This change removes this sideeffect in 7.0. --- .../org/elasticsearch/index/shard/IndexShard.java | 6 ------ .../segments/IndicesSegmentsRequestTests.java | 1 + .../java/org/elasticsearch/get/GetActionIT.java | 7 ++++--- .../elasticsearch/indices/stats/IndexStatsIT.java | 1 + .../elasticsearch/search/nested/SimpleNestedIT.java | 13 ++++--------- 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java index 16c9ab3964a01..ca1ba44db43d8 100644 --- a/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -1005,7 +1005,6 @@ public Engine.CommitId flush(FlushRequest request) { } final long time = System.nanoTime(); final Engine.CommitId commitId = engine.flush(force, waitIfOngoing); - engine.refresh("flush"); // TODO this is technically wrong we should remove this in 7.0 flushMetric.inc(System.nanoTime() - time); return commitId; } @@ -1036,9 +1035,6 @@ public void forceMerge(ForceMergeRequest forceMerge) throws IOException { Engine engine = getEngine(); engine.forceMerge(forceMerge.flush(), forceMerge.maxNumSegments(), forceMerge.onlyExpungeDeletes(), false, false); - if (forceMerge.flush()) { - engine.refresh("force_merge"); // TODO this is technically wrong we should remove this in 7.0 - } } /** @@ -1055,8 +1051,6 @@ public org.apache.lucene.util.Version upgrade(UpgradeRequest upgrade) throws IOE engine.forceMerge(true, // we need to flush at the end to make sure the upgrade is durable Integer.MAX_VALUE, // we just want to upgrade the segments, not actually optimize to a single segment false, true, upgrade.upgradeOnlyAncientSegments()); - engine.refresh("upgrade"); // TODO this is technically wrong we should remove this in 7.0 - org.apache.lucene.util.Version version = minimumCompatibleVersion(); if (logger.isTraceEnabled()) { logger.trace("upgraded segments for {} from version {} to version {}", shardId, previousVersion, version); diff --git a/core/src/test/java/org/elasticsearch/action/admin/indices/segments/IndicesSegmentsRequestTests.java b/core/src/test/java/org/elasticsearch/action/admin/indices/segments/IndicesSegmentsRequestTests.java index 4a2895ad7ee41..e6d8dea13d2fb 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/indices/segments/IndicesSegmentsRequestTests.java +++ b/core/src/test/java/org/elasticsearch/action/admin/indices/segments/IndicesSegmentsRequestTests.java @@ -55,6 +55,7 @@ public void setupIndex() { client().prepareIndex("test", "type1", id).setSource("text", "sometext").get(); } client().admin().indices().prepareFlush("test").get(); + client().admin().indices().prepareRefresh().get(); } public void testBasic() { diff --git a/core/src/test/java/org/elasticsearch/get/GetActionIT.java b/core/src/test/java/org/elasticsearch/get/GetActionIT.java index 9de24ae4e8704..c6135c8f2286a 100644 --- a/core/src/test/java/org/elasticsearch/get/GetActionIT.java +++ b/core/src/test/java/org/elasticsearch/get/GetActionIT.java @@ -131,16 +131,17 @@ public void testSimpleGet() { assertThat(response.getField("field1").getValues().get(0).toString(), equalTo("value1")); assertThat(response.getField("field2"), nullValue()); - logger.info("--> flush the index, so we load it from it"); - flush(); - logger.info("--> realtime get 1 (loaded from index)"); + logger.info("--> realtime get 1"); response = client().prepareGet(indexOrAlias(), "type1", "1").get(); assertThat(response.isExists(), equalTo(true)); assertThat(response.getIndex(), equalTo("test")); assertThat(response.getSourceAsMap().get("field1").toString(), equalTo("value1")); assertThat(response.getSourceAsMap().get("field2").toString(), equalTo("value2")); + logger.info("--> refresh the index, so we load it from it"); + refresh(); + logger.info("--> non realtime get 1 (loaded from index)"); response = client().prepareGet(indexOrAlias(), "type1", "1").setRealtime(false).get(); assertThat(response.isExists(), equalTo(true)); diff --git a/core/src/test/java/org/elasticsearch/indices/stats/IndexStatsIT.java b/core/src/test/java/org/elasticsearch/indices/stats/IndexStatsIT.java index 4c42e2fa4d451..9dce712130650 100644 --- a/core/src/test/java/org/elasticsearch/indices/stats/IndexStatsIT.java +++ b/core/src/test/java/org/elasticsearch/indices/stats/IndexStatsIT.java @@ -573,6 +573,7 @@ public void testSegmentsStats() { client().admin().indices().prepareFlush().get(); client().admin().indices().prepareForceMerge().setMaxNumSegments(1).execute().actionGet(); + client().admin().indices().prepareRefresh().get(); stats = client().admin().indices().prepareStats().setSegments(true).get(); assertThat(stats.getTotal().getSegments(), notNullValue()); diff --git a/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java b/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java index 56bb837b4e047..68ef78f4273a8 100644 --- a/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java +++ b/core/src/test/java/org/elasticsearch/search/nested/SimpleNestedIT.java @@ -83,12 +83,10 @@ public void testSimpleNested() throws Exception { .endObject()).execute().actionGet(); waitForRelocation(ClusterHealthStatus.GREEN); - // flush, so we fetch it from the index (as see that we filter nested docs) - flush(); GetResponse getResponse = client().prepareGet("test", "type1", "1").get(); assertThat(getResponse.isExists(), equalTo(true)); assertThat(getResponse.getSourceAsBytes(), notNullValue()); - + refresh(); // check the numDocs assertDocumentCount("test", 3); @@ -126,8 +124,7 @@ public void testSimpleNested() throws Exception { .endArray() .endObject()).execute().actionGet(); waitForRelocation(ClusterHealthStatus.GREEN); - // flush, so we fetch it from the index (as see that we filter nested docs) - flush(); + refresh(); assertDocumentCount("test", 6); searchResponse = client().prepareSearch("test").setQuery(nestedQuery("nested1", @@ -151,8 +148,7 @@ public void testSimpleNested() throws Exception { DeleteResponse deleteResponse = client().prepareDelete("test", "type1", "2").execute().actionGet(); assertEquals(DocWriteResponse.Result.DELETED, deleteResponse.getResult()); - // flush, so we fetch it from the index (as see that we filter nested docs) - flush(); + refresh(); assertDocumentCount("test", 3); searchResponse = client().prepareSearch("test").setQuery(nestedQuery("nested1", termQuery("nested1.n_field1", "n_value1_1"), ScoreMode.Avg)).execute().actionGet(); @@ -179,11 +175,10 @@ public void testMultiNested() throws Exception { .endArray() .endObject()).execute().actionGet(); - // flush, so we fetch it from the index (as see that we filter nested docs) - flush(); GetResponse getResponse = client().prepareGet("test", "type1", "1").execute().actionGet(); assertThat(getResponse.isExists(), equalTo(true)); waitForRelocation(ClusterHealthStatus.GREEN); + refresh(); // check the numDocs assertDocumentCount("test", 7);