From 9a49cb42e938c658a2f7ffa73922c969d2737f5e Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 14 May 2018 12:22:30 +0200 Subject: [PATCH 1/2] Use a private Directory for split / shrink Lucene IW checks for pending deletes before it starts. In the split / shrink case we might delete files which can cause this check to trip. Since we know that these files are not subject for reusal we can use a private directory instance for this operation to prevent unnecessary busy waiting but still keep the checks for subsequent useage. Closes #30416 --- .../org/elasticsearch/index/shard/StoreRecovery.java | 10 +++++++--- .../main/java/org/elasticsearch/index/store/Store.java | 10 ++++++++++ .../action/admin/indices/create/ShrinkIndexIT.java | 1 - .../action/admin/indices/create/SplitIndexIT.java | 2 -- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java b/server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java index 54718c545a44e..92e0e7ab7b5b5 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java +++ b/server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java @@ -123,13 +123,17 @@ boolean recoverFromLocalShards(BiConsumer mappingUpdate return executeRecovery(indexShard, () -> { logger.debug("starting recovery from local shards {}", shards); try { - final Directory directory = indexShard.store().directory(); // don't close this directory!! final Directory[] sources = shards.stream().map(LocalShardSnapshot::getSnapshotDirectory).toArray(Directory[]::new); final long maxSeqNo = shards.stream().mapToLong(LocalShardSnapshot::maxSeqNo).max().getAsLong(); final long maxUnsafeAutoIdTimestamp = shards.stream().mapToLong(LocalShardSnapshot::maxUnsafeAutoIdTimestamp).max().getAsLong(); - addIndices(indexShard.recoveryState().getIndex(), directory, indexSort, sources, maxSeqNo, maxUnsafeAutoIdTimestamp, - indexShard.indexSettings().getIndexMetaData(), indexShard.shardId().id(), isSplit, hasNested); + try (final Directory directory = indexShard.store().createNewDirectory()) { + // we create a new directory since we might delete old files we never write again + // but this might cause issues on windows since Lucenes IW now checks for pending deletes before it + // starts up and we might be still holding on to them on windows. for this reason we use a private directory here + addIndices(indexShard.recoveryState().getIndex(), directory, indexSort, sources, maxSeqNo, maxUnsafeAutoIdTimestamp, + indexShard.indexSettings().getIndexMetaData(), indexShard.shardId().id(), isSplit, hasNested); + } internalRecoverFromStore(indexShard); // just trigger a merge to do housekeeping on the // copied segments - we will also see them in stats etc. 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..bb6ce504d60f4 100644 --- a/server/src/main/java/org/elasticsearch/index/store/Store.java +++ b/server/src/main/java/org/elasticsearch/index/store/Store.java @@ -147,6 +147,7 @@ public class Store extends AbstractIndexShardComponent implements Closeable, Ref private final ShardLock shardLock; private final OnClose onClose; private final SingleObjectCache statsCache; + private final DirectoryService directoryService; private final AbstractRefCounted refCounter = new AbstractRefCounted("store") { @Override @@ -165,6 +166,7 @@ public Store(ShardId shardId, IndexSettings indexSettings, DirectoryService dire super(shardId, indexSettings); final Settings settings = indexSettings.getSettings(); this.directory = new StoreDirectory(directoryService.newDirectory(), Loggers.getLogger("index.store.deletes", settings, shardId)); + this.directoryService = directoryService; this.shardLock = shardLock; this.onClose = onClose; final TimeValue refreshInterval = indexSettings.getValue(INDEX_STORE_STATS_REFRESH_INTERVAL_SETTING); @@ -176,6 +178,14 @@ public Store(ShardId shardId, IndexSettings indexSettings, DirectoryService dire assert shardLock.getShardId().equals(shardId); } + /** + * Returns a new directory instance that is not maintained by this store. The caller is responsible for closing it. + */ + public Directory createNewDirectory() throws IOException { + ensureOpen(); + return directoryService.newDirectory(); + } + public Directory directory() { ensureOpen(); return directory; 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..89b6695908225 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 @@ -77,7 +77,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 2f9475422f2a91eb96f905f1793bb2185c1002c0 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 14 May 2018 14:21:13 +0200 Subject: [PATCH 2/2] remove redundant modifier --- .../main/java/org/elasticsearch/index/shard/StoreRecovery.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java b/server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java index 92e0e7ab7b5b5..8a7a77ad1337f 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java +++ b/server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java @@ -127,7 +127,7 @@ boolean recoverFromLocalShards(BiConsumer mappingUpdate final long maxSeqNo = shards.stream().mapToLong(LocalShardSnapshot::maxSeqNo).max().getAsLong(); final long maxUnsafeAutoIdTimestamp = shards.stream().mapToLong(LocalShardSnapshot::maxUnsafeAutoIdTimestamp).max().getAsLong(); - try (final Directory directory = indexShard.store().createNewDirectory()) { + try (Directory directory = indexShard.store().createNewDirectory()) { // we create a new directory since we might delete old files we never write again // but this might cause issues on windows since Lucenes IW now checks for pending deletes before it // starts up and we might be still holding on to them on windows. for this reason we use a private directory here