From d470bdbff61d9036d76a7530d95d9e458ad90ddf Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 9 Apr 2015 14:54:40 +0200 Subject: [PATCH] Only flush for checkindex if we have uncommitted changes Today we force a flush before check index to ensure we have an index to check on. Yet if the index is large and the FS is slow this can have significant impact on the index deletion performance. This commit introduces a check if there are any uncommitted changes in order to skip the additional commit. Closes #10505 --- .../org/elasticsearch/index/engine/Engine.java | 6 ++++++ .../index/engine/InternalEngine.java | 5 +++++ .../elasticsearch/index/engine/ShadowEngine.java | 5 +++++ .../OldIndexBackwardsCompatibilityTests.java | 15 --------------- .../test/store/MockFSDirectoryService.java | 10 ++++------ 5 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/engine/Engine.java b/src/main/java/org/elasticsearch/index/engine/Engine.java index ca7d10130ad..83ebfb72a7c 100644 --- a/src/main/java/org/elasticsearch/index/engine/Engine.java +++ b/src/main/java/org/elasticsearch/index/engine/Engine.java @@ -1061,4 +1061,10 @@ public abstract class Engine implements Closeable { } } } + + /** + * Returns true the internal writer has any uncommitted changes. Otherwise false + * @return + */ + public abstract boolean hasUncommittedChanges(); } diff --git a/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index 9ae31e24244..2c293d986f3 100644 --- a/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -940,6 +940,11 @@ public class InternalEngine extends Engine { } } + @Override + public boolean hasUncommittedChanges() { + return indexWriter.hasUncommittedChanges(); + } + @Override protected SearcherManager getSearcherManager() { return searcherManager; diff --git a/src/main/java/org/elasticsearch/index/engine/ShadowEngine.java b/src/main/java/org/elasticsearch/index/engine/ShadowEngine.java index ba23903ee4b..c49758398bc 100644 --- a/src/main/java/org/elasticsearch/index/engine/ShadowEngine.java +++ b/src/main/java/org/elasticsearch/index/engine/ShadowEngine.java @@ -216,4 +216,9 @@ public class ShadowEngine extends Engine { } } } + + @Override + public boolean hasUncommittedChanges() { + return false; + } } diff --git a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java index e5811c9e5b3..916779f9a59 100644 --- a/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java +++ b/src/test/java/org/elasticsearch/bwcompat/OldIndexBackwardsCompatibilityTests.java @@ -158,7 +158,6 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio } void unloadIndex(String indexName) throws Exception { - client().admin().indices().prepareFlush(indexName).setWaitIfOngoing(true).setForce(true).get(); // temporary for debugging ElasticsearchAssertions.assertAcked(client().admin().indices().prepareDelete(indexName).get()); ElasticsearchAssertions.assertAllFilesClosed(); } @@ -201,20 +200,6 @@ public class OldIndexBackwardsCompatibilityTests extends ElasticsearchIntegratio Collections.shuffle(indexes, getRandom()); for (String index : indexes) { - if (index.equals("index-0.90.13.zip") == false) { - long startTime = System.currentTimeMillis(); - logger.info("--> Testing old index " + index); - assertOldIndexWorks(index); - logger.info("--> Done testing " + index + ", took " + ((System.currentTimeMillis() - startTime) / 1000.0) + " seconds"); - } - } - } - - @TestLogging("test.engine:TRACE,index.engine:TRACE,test.engine.lucene:TRACE,index.engine.lucene:TRACE") - public void testShitSlowIndex() throws Exception { - setupCluster(); - for (int i = 0; i < 5; i++) { - String index = "index-0.90.13.zip"; long startTime = System.currentTimeMillis(); logger.info("--> Testing old index " + index); assertOldIndexWorks(index); diff --git a/src/test/java/org/elasticsearch/test/store/MockFSDirectoryService.java b/src/test/java/org/elasticsearch/test/store/MockFSDirectoryService.java index 89393fa24ac..718342f4c59 100644 --- a/src/test/java/org/elasticsearch/test/store/MockFSDirectoryService.java +++ b/src/test/java/org/elasticsearch/test/store/MockFSDirectoryService.java @@ -26,17 +26,13 @@ import org.apache.lucene.store.Directory; import org.apache.lucene.store.LockFactory; import org.apache.lucene.store.StoreRateLimiting; import org.apache.lucene.util.AbstractRandomizedTest; -import org.apache.lucene.util.IOUtils; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.BytesStreamOutput; -import org.elasticsearch.common.lease.Releasable; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.index.engine.Engine; -import org.elasticsearch.index.engine.InternalEngine; import org.elasticsearch.index.settings.IndexSettings; import org.elasticsearch.index.shard.IndexShardException; import org.elasticsearch.index.shard.IndexShardState; @@ -91,8 +87,10 @@ public class MockFSDirectoryService extends FsDirectoryService { // When the the internal engine closes we do a rollback, which removes uncommitted segments // By doing a commit flush we perform a Lucene commit, but don't clear the translog, // so that even in tests where don't flush we can check the integrity of the Lucene index - logger.info("{} flushing in order to run checkindex", indexShard.shardId()); - Releasables.close(indexShard.engine().snapshotIndex()); // Keep translog for tests that rely on replaying it + if (indexShard.engine().hasUncommittedChanges()) { // only if we have any changes + logger.info("{} flushing in order to run checkindex", indexShard.shardId()); + Releasables.close(indexShard.engine().snapshotIndex()); // Keep translog for tests that rely on replaying it + } logger.info("{} flush finished in beforeIndexShardClosed", indexShard.shardId()); canRun = true; }