Skip to content

Commit

Permalink
Don't refres on _flush _force_merge and _upgrade
Browse files Browse the repository at this point in the history
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 (elastic#26972) we artificially
add a refresh call for bwc. This change removes this sideeffect in 7.0.
  • Loading branch information
s1monw committed Oct 13, 2017
1 parent a789583 commit 774273a
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
}
}

/**
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
7 changes: 4 additions & 3 deletions core/src/test/java/org/elasticsearch/get/GetActionIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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",
Expand All @@ -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();
Expand All @@ -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);

Expand Down

0 comments on commit 774273a

Please sign in to comment.