You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ro...@apache.org on 2018/04/23 08:00:41 UTC
lucene-solr:branch_7_3: LUCENE-8254: LRUQueryCache can leak locks
Repository: lucene-solr
Updated Branches:
refs/heads/branch_7_3 566c07f7d -> 696ff57f4
LUCENE-8254: LRUQueryCache can leak locks
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/696ff57f
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/696ff57f
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/696ff57f
Branch: refs/heads/branch_7_3
Commit: 696ff57f4cfaebd842926a80e71d4e0970a73544
Parents: 566c07f
Author: Alan Woodward <ro...@apache.org>
Authored: Mon Apr 16 10:57:12 2018 +0100
Committer: Alan Woodward <ro...@apache.org>
Committed: Mon Apr 23 08:59:32 2018 +0100
----------------------------------------------------------------------
lucene/CHANGES.txt | 7 +-
.../org/apache/lucene/search/LRUQueryCache.java | 22 +++---
.../apache/lucene/search/TestLRUQueryCache.java | 70 ++++++++++++++++++--
3 files changed, 82 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/696ff57f/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 2883621..04fba40 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -4,7 +4,12 @@ For more information on past and future Lucene versions, please see:
http://s.apache.org/luceneversions
======================= Lucene 7.3.1 =======================
-(No Changes)
+
+Bug fixes
+
+* LUCENE-8254: LRUQueryCache could cause IndexReader to hang on close, when
+ shared with another reader with no CacheHelper (Alan Woodward, Simon Willnauer,
+ Adrien Grand)
======================= Lucene 7.3.0 =======================
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/696ff57f/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java b/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java
index 5c828d6..2575b67 100644
--- a/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java
+++ b/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java
@@ -720,16 +720,17 @@ public class LRUQueryCache implements QueryCache, Accountable {
return in.scorerSupplier(context);
}
- // If the lock is already busy, prefer using the uncached version than waiting
- if (lock.tryLock() == false) {
- return in.scorerSupplier(context);
- }
-
final IndexReader.CacheHelper cacheHelper = context.reader().getCoreCacheHelper();
if (cacheHelper == null) {
// this reader has no cache helper
return in.scorerSupplier(context);
}
+
+ // If the lock is already busy, prefer using the uncached version than waiting
+ if (lock.tryLock() == false) {
+ return in.scorerSupplier(context);
+ }
+
DocIdSet docIdSet;
try {
docIdSet = get(in.getQuery(), context, cacheHelper);
@@ -800,16 +801,17 @@ public class LRUQueryCache implements QueryCache, Accountable {
return in.bulkScorer(context);
}
- // If the lock is already busy, prefer using the uncached version than waiting
- if (lock.tryLock() == false) {
- return in.bulkScorer(context);
- }
-
final IndexReader.CacheHelper cacheHelper = context.reader().getCoreCacheHelper();
if (cacheHelper == null) {
// this reader has no cacheHelper
return in.bulkScorer(context);
}
+
+ // If the lock is already busy, prefer using the uncached version than waiting
+ if (lock.tryLock() == false) {
+ return in.bulkScorer(context);
+ }
+
DocIdSet docIdSet;
try {
docIdSet = get(in.getQuery(), context, cacheHelper);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/696ff57f/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java b/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java
index 0009701..81a7b70 100644
--- a/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java
+++ b/lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java
@@ -62,7 +62,6 @@ import org.apache.lucene.util.IOUtils;
import org.apache.lucene.util.LuceneTestCase;
import org.apache.lucene.util.RamUsageTester;
import org.apache.lucene.util.TestUtil;
-import org.junit.Test;
public class TestLRUQueryCache extends LuceneTestCase {
@@ -1479,8 +1478,6 @@ public class TestLRUQueryCache extends LuceneTestCase {
}
}
- @Test
- @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028")
public void testDocValuesUpdatesDontBreakCache() throws IOException {
Directory dir = newDirectory();
IndexWriterConfig iwc = newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE);
@@ -1511,7 +1508,7 @@ public class TestLRUQueryCache extends LuceneTestCase {
w.addDocument(doc);
reader.close();
reader = DirectoryReader.open(w);
- searcher = newSearcher(reader);
+ searcher = new AssertingIndexSearcher(random(), reader); // no newSearcher(reader) - see comment above
searcher.setQueryCachingPolicy(QueryCachingPolicy.ALWAYS_CACHE);
searcher.setQueryCache(cache);
@@ -1520,7 +1517,7 @@ public class TestLRUQueryCache extends LuceneTestCase {
reader.close();
reader = DirectoryReader.open(w);
- searcher = newSearcher(reader);
+ searcher = new AssertingIndexSearcher(random(), reader); // no newSearcher(reader) - see comment above
searcher.setQueryCachingPolicy(QueryCachingPolicy.ALWAYS_CACHE);
searcher.setQueryCache(cache);
@@ -1531,7 +1528,7 @@ public class TestLRUQueryCache extends LuceneTestCase {
w.updateNumericDocValue(new Term("text", "text"), "field", 2l);
reader.close();
reader = DirectoryReader.open(w);
- searcher = newSearcher(reader);
+ searcher = new AssertingIndexSearcher(random(), reader); // no newSearcher(reader) - see comment above
searcher.setQueryCachingPolicy(QueryCachingPolicy.ALWAYS_CACHE);
searcher.setQueryCache(cache);
@@ -1546,4 +1543,65 @@ public class TestLRUQueryCache extends LuceneTestCase {
dir.close();
}
+
+ public void testBulkScorerLocking() throws Exception {
+
+ Directory dir = newDirectory();
+ IndexWriterConfig iwc = newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE);
+ IndexWriter w = new IndexWriter(dir, iwc);
+
+ final int numDocs = atLeast(10);
+ Document emptyDoc = new Document();
+ for (int d = 0; d < numDocs; ++d) {
+ for (int i = random().nextInt(5000); i >= 0; --i) {
+ w.addDocument(emptyDoc);
+ }
+ Document doc = new Document();
+ for (String value : Arrays.asList("foo", "bar", "baz")) {
+ if (random().nextBoolean()) {
+ doc.add(new StringField("field", value, Store.NO));
+ }
+ }
+ }
+ for (int i = TestUtil.nextInt(random(), 3000, 5000); i >= 0; --i) {
+ w.addDocument(emptyDoc);
+ }
+ if (random().nextBoolean()) {
+ w.forceMerge(1);
+ }
+
+ DirectoryReader reader = DirectoryReader.open(w);
+ DirectoryReader noCacheReader = new DummyDirectoryReader(reader);
+
+ LRUQueryCache cache = new LRUQueryCache(1, 100000, context -> true);
+ IndexSearcher searcher = new AssertingIndexSearcher(random(), reader);
+ searcher.setQueryCache(cache);
+ searcher.setQueryCachingPolicy(QueryCachingPolicy.ALWAYS_CACHE);
+
+ Query query = new ConstantScoreQuery(new BooleanQuery.Builder()
+ .add(new BoostQuery(new TermQuery(new Term("field", "foo")), 3), Occur.SHOULD)
+ .add(new BoostQuery(new TermQuery(new Term("field", "bar")), 3), Occur.SHOULD)
+ .add(new BoostQuery(new TermQuery(new Term("field", "baz")), 3), Occur.SHOULD)
+ .build());
+
+ searcher.search(query, 1);
+
+ IndexSearcher noCacheHelperSearcher = new AssertingIndexSearcher(random(), noCacheReader);
+ noCacheHelperSearcher.setQueryCache(cache);
+ noCacheHelperSearcher.setQueryCachingPolicy(QueryCachingPolicy.ALWAYS_CACHE);
+ noCacheHelperSearcher.search(query, 1);
+
+ Thread t = new Thread(() -> {
+ try {
+ noCacheReader.close();
+ w.close();
+ dir.close();
+ }
+ catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ });
+ t.start();
+ t.join();
+ }
}