From 05663e428315f1ba0359b8eeffa7ea6d31f05bdd Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 14 May 2018 14:24:41 +0200 Subject: [PATCH 1/6] Side-step pending deletes check When we split/shrink an index we open several IndexWriter instances causeing file-deletes to be pending on windows. This subsequently fails when we open an IW to bootstrap the index history due to pending deletes. This change sidesteps the check since we know our history goes forward in terms of files and segments. Closes #30416 --- .../java/org/elasticsearch/index/store/Store.java | 11 +++++++++++ .../action/admin/indices/create/ShrinkIndexIT.java | 2 -- .../action/admin/indices/create/SplitIndexIT.java | 2 -- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/store/Store.java b/server/src/main/java/org/elasticsearch/index/store/Store.java index de29386022cc6..bf51967ec5ab8 100644 --- a/server/src/main/java/org/elasticsearch/index/store/Store.java +++ b/server/src/main/java/org/elasticsearch/index/store/Store.java @@ -759,6 +759,17 @@ private void innerClose() throws IOException { public String toString() { return "store(" + in.toString() + ")"; } + + @Override + public boolean checkPendingDeletions() throws IOException { + if (super.checkPendingDeletions() == false) { + deletesLogger.debug("directory has still pending deletes"); + } + // we skip this check since our IW usage always goes forward. + // we still might run into situations where we have pending deletes ie. in shrink / split case + // and that will cause issues on windows since we open multiple IW instance one after another during the split/shrink recovery + return false; + } } /** diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/ShrinkIndexIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/ShrinkIndexIT.java index 8443ac2bf2e3d..e48f151081f62 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/create/ShrinkIndexIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/create/ShrinkIndexIT.java @@ -23,7 +23,6 @@ import org.apache.lucene.search.SortField; import org.apache.lucene.search.SortedSetSelector; import org.apache.lucene.search.SortedSetSortField; -import org.apache.lucene.util.LuceneTestCase.AwaitsFix; import org.elasticsearch.Version; import org.elasticsearch.action.admin.cluster.reroute.ClusterRerouteResponse; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; @@ -77,7 +76,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; -@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/30416") public class ShrinkIndexIT extends ESIntegTestCase { @Override diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/SplitIndexIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/SplitIndexIT.java index a7f7ed6f52546..fe6e980ab4259 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/create/SplitIndexIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/create/SplitIndexIT.java @@ -24,7 +24,6 @@ import org.apache.lucene.search.SortedSetSelector; import org.apache.lucene.search.SortedSetSortField; import org.apache.lucene.search.join.ScoreMode; -import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.Version; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; @@ -81,7 +80,6 @@ import static org.hamcrest.Matchers.greaterThanOrEqualTo; -@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/30416") public class SplitIndexIT extends ESIntegTestCase { @Override From a7bce4bfc167ac68f3d9baffd8bda8931381eb02 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 14 May 2018 14:34:42 +0200 Subject: [PATCH 2/6] apply feedback --- .../src/main/java/org/elasticsearch/index/store/Store.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/store/Store.java b/server/src/main/java/org/elasticsearch/index/store/Store.java index bf51967ec5ab8..e83caad5e43ec 100644 --- a/server/src/main/java/org/elasticsearch/index/store/Store.java +++ b/server/src/main/java/org/elasticsearch/index/store/Store.java @@ -731,13 +731,13 @@ static final class StoreDirectory extends FilterDirectory { private final Logger deletesLogger; - StoreDirectory(Directory delegateDirectory, Logger deletesLogger) throws IOException { + StoreDirectory(Directory delegateDirectory, Logger deletesLogger) { super(delegateDirectory); this.deletesLogger = deletesLogger; } @Override - public void close() throws IOException { + public void close() { assert false : "Nobody should close this directory except of the Store itself"; } @@ -763,7 +763,7 @@ public String toString() { @Override public boolean checkPendingDeletions() throws IOException { if (super.checkPendingDeletions() == false) { - deletesLogger.debug("directory has still pending deletes"); + deletesLogger.warn("directory has still pending deletes"); } // we skip this check since our IW usage always goes forward. // we still might run into situations where we have pending deletes ie. in shrink / split case From 43759e2824fd9ee432a37928a9febe0d7dc794e9 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 14 May 2018 14:39:43 +0200 Subject: [PATCH 3/6] invert condition --- server/src/main/java/org/elasticsearch/index/store/Store.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/store/Store.java b/server/src/main/java/org/elasticsearch/index/store/Store.java index e83caad5e43ec..0374d74dcf58b 100644 --- a/server/src/main/java/org/elasticsearch/index/store/Store.java +++ b/server/src/main/java/org/elasticsearch/index/store/Store.java @@ -762,7 +762,7 @@ public String toString() { @Override public boolean checkPendingDeletions() throws IOException { - if (super.checkPendingDeletions() == false) { + if (super.checkPendingDeletions()) { deletesLogger.warn("directory has still pending deletes"); } // we skip this check since our IW usage always goes forward. From 883f14c1670db94cdda1bc0bd95d15408ff1a69a Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 14 May 2018 12:58:57 -0400 Subject: [PATCH 4/6] Unskip rest tests --- .../rest-api-spec/test/indices.split/10_basic.yml | 7 ++----- .../rest-api-spec/test/indices.split/20_source_mapping.yml | 7 ++----- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml index 635673c182f2f..f64f51b64f021 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml @@ -107,11 +107,8 @@ setup: --- "Split from 1 to N": - skip: - # when re-enabling uncomment the below skips - version: "all" - reason: "AwaitsFix'ing, see https://github.com/elastic/elasticsearch/issues/30503" - # version: " - 6.99.99" - # reason: expects warnings that pre-7.0.0 will not send + version: " - 6.99.99" + reason: expects warnings that pre-7.0.0 will not send features: "warnings" - do: indices.create: diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml index 433ac040dd1e4..dc7d83bf978f7 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml @@ -1,11 +1,8 @@ --- "Split index ignores target template mapping": - skip: - # when re-enabling uncomment the below skips - version: "all" - reason: "AwaitsFix'ing, see https://github.com/elastic/elasticsearch/issues/30503" - # version: " - 6.99.99" - # reason: expects warnings that pre-7.0.0 will not send + version: " - 6.99.99" + reason: expects warnings that pre-7.0.0 will not send features: "warnings" # create index From 5771a2608dfb866b2731e708c4b5501330c10100 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 14 May 2018 22:50:01 +0200 Subject: [PATCH 5/6] fix it --- .../resources/rest-api-spec/test/indices.split/10_basic.yml | 4 ++-- .../rest-api-spec/test/indices.split/20_source_mapping.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml index f1887bac60fc2..3d905df7d1155 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml @@ -107,8 +107,8 @@ setup: --- "Split from 1 to N": - skip: - version: " - 6.99.99" - reason: expects warnings that pre-7.0.0 will not send + version: " - 6.3.99" + reason: expects warnings that pre-6.4.0 will not send features: "warnings" - do: indices.create: diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml index dc7d83bf978f7..88d3f3c610202 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml @@ -1,8 +1,8 @@ --- "Split index ignores target template mapping": - skip: - version: " - 6.99.99" - reason: expects warnings that pre-7.0.0 will not send + version: " - 6.3.99" + reason: expects warnings that pre-6.4.0 will not send features: "warnings" # create index From 7ba7b164234cbdf75253c1289215abb5f047612f Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 15 May 2018 08:50:16 +0200 Subject: [PATCH 6/6] skip 7.0 feature in mutli cluster tests --- .../resources/rest-api-spec/test/indices.split/10_basic.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml index 3d905df7d1155..8cfe77042dd3f 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml @@ -107,8 +107,8 @@ setup: --- "Split from 1 to N": - skip: - version: " - 6.3.99" - reason: expects warnings that pre-6.4.0 will not send + version: " - 6.99.99" + reason: Automatic preparation for splitting was added in 7.0.0 features: "warnings" - do: indices.create: