From abaf816d001bbf4290b6bb63365dcb615746bee6 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 9 Jan 2016 11:06:41 -0500 Subject: [PATCH] Remove and forbid use of IndexWriter#isLocked This commit removes and now forbids use of org.apache.lucene.index.IndexWriter#isLocked as this method was deprecated in LUCENE-6508. The deprecation is due to the fact that checking if a lock is held before acquiring that lock is subject to a time-of-check-to-time-of-use race condition. There were three uses of IndexWriter#isLocked in the code base: - a logging statement in o.e.i.e.InternalEngine where we are already in an exceptional condition that the lock was held; in this case, logging whether or not the directory is locked is superfluous - in o.e.c.l.u.VersionsTests where we were verifying that a write lock is released upon closing an IndexWriter; in this case, the check is not needed as successfully closing an IndexWriter releases its write lock - in o.e.t.s.MockFSDirectoryService where we were verifying that a directory is not write-locked before (implicitly) trying to obtain such a write lock in org.apache.lucene.index.CheckIndex# (this is the exact type of a situation that is subject to a race condition); in this case we can proceed by just (implicitly) trying to obtain the write lock and failing if we encounter a LockObtainFailedException --- buildSrc/src/main/resources/forbidden/all-signatures.txt | 1 + .../org/elasticsearch/index/engine/InternalEngine.java | 3 +-- .../elasticsearch/common/lucene/uid/VersionsTests.java | 2 -- .../elasticsearch/test/store/MockFSDirectoryService.java | 8 ++++---- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/buildSrc/src/main/resources/forbidden/all-signatures.txt b/buildSrc/src/main/resources/forbidden/all-signatures.txt index 76cd83cdede..f72de510721 100644 --- a/buildSrc/src/main/resources/forbidden/all-signatures.txt +++ b/buildSrc/src/main/resources/forbidden/all-signatures.txt @@ -45,6 +45,7 @@ org.apache.lucene.search.NumericRangeFilter org.apache.lucene.search.PrefixFilter org.apache.lucene.search.QueryWrapperFilter org.apache.lucene.search.join.BitDocIdSetCachingWrapperFilter +org.apache.lucene.index.IndexWriter#isLocked(org.apache.lucene.store.Directory) java.nio.file.Paths @ Use org.elasticsearch.common.io.PathUtils.get() instead. java.nio.file.FileSystems#getDefault() @ use org.elasticsearch.common.io.PathUtils.getDefaultFileSystem() instead. diff --git a/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index d93ce36686e..b2d81f07107 100644 --- a/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -959,8 +959,7 @@ public class InternalEngine extends Engine { }); return new IndexWriter(store.directory(), iwc); } catch (LockObtainFailedException ex) { - boolean isLocked = IndexWriter.isLocked(store.directory()); - logger.warn("Could not lock IndexWriter isLocked [{}]", ex, isLocked); + logger.warn("could not lock IndexWriter", ex); throw ex; } } diff --git a/core/src/test/java/org/elasticsearch/common/lucene/uid/VersionsTests.java b/core/src/test/java/org/elasticsearch/common/lucene/uid/VersionsTests.java index fb3c021fd5d..d6abcfe7735 100644 --- a/core/src/test/java/org/elasticsearch/common/lucene/uid/VersionsTests.java +++ b/core/src/test/java/org/elasticsearch/common/lucene/uid/VersionsTests.java @@ -56,7 +56,6 @@ import java.util.List; import java.util.Map; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; @@ -292,7 +291,6 @@ public class VersionsTests extends ESTestCase { } iw.close(); - assertThat(IndexWriter.isLocked(iw.getDirectory()), is(false)); ir.close(); dir.close(); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/store/MockFSDirectoryService.java b/test/framework/src/main/java/org/elasticsearch/test/store/MockFSDirectoryService.java index 58a72789f65..3128a2220ae 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/store/MockFSDirectoryService.java +++ b/test/framework/src/main/java/org/elasticsearch/test/store/MockFSDirectoryService.java @@ -26,6 +26,7 @@ import org.apache.lucene.index.IndexWriter; import org.apache.lucene.store.BaseDirectoryWrapper; import org.apache.lucene.store.Directory; import org.apache.lucene.store.LockFactory; +import org.apache.lucene.store.LockObtainFailedException; import org.apache.lucene.store.MockDirectoryWrapper; import org.apache.lucene.store.StoreRateLimiting; import org.apache.lucene.util.LuceneTestCase; @@ -113,10 +114,6 @@ public class MockFSDirectoryService extends FsDirectoryService { if (!Lucene.indexExists(dir)) { return; } - if (IndexWriter.isLocked(dir)) { - ESTestCase.checkIndexFailed = true; - throw new IllegalStateException("IndexWriter is still open on shard " + shardId); - } try (CheckIndex checkIndex = new CheckIndex(dir)) { BytesStreamOutput os = new BytesStreamOutput(); PrintStream out = new PrintStream(os, false, StandardCharsets.UTF_8.name()); @@ -134,6 +131,9 @@ public class MockFSDirectoryService extends FsDirectoryService { logger.debug("check index [success]\n{}", new String(os.bytes().toBytes(), StandardCharsets.UTF_8)); } } + } catch (LockObtainFailedException e) { + ESTestCase.checkIndexFailed = true; + throw new IllegalStateException("IndexWriter is still open on shard " + shardId, e); } } catch (Exception e) { logger.warn("failed to check index", e);