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/16 10:10:41 UTC
[1/2] lucene-solr:master: LUCENE-8254: LRUQueryCache can leak locks
Repository: lucene-solr
Updated Branches:
refs/heads/branch_7x c662b7d26 -> 8eb6ed463
refs/heads/master 3028f3e9e -> 19fa91dbf
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/19fa91db
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/19fa91db
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/19fa91db
Branch: refs/heads/master
Commit: 19fa91dbfbca990df460a9e709b7f83c27bc27cd
Parents: 3028f3e
Author: Alan Woodward <ro...@apache.org>
Authored: Mon Apr 16 10:57:12 2018 +0100
Committer: Alan Woodward <ro...@apache.org>
Committed: Mon Apr 16 10:57:12 2018 +0100
----------------------------------------------------------------------
lucene/CHANGES.txt | 4 ++
.../org/apache/lucene/search/LRUQueryCache.java | 22 +++----
.../apache/lucene/search/TestLRUQueryCache.java | 63 +++++++++++++++++++-
3 files changed, 77 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/19fa91db/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index e559099..1b790e4 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -162,6 +162,10 @@ Bug Fixes
index file names for updated doc values fields (Simon Willnauer,
Michael McCandless, Nhat Nguyen)
+* LUCENE-8254: LRUQueryCache could cause IndexReader to hang on close, when
+ shared with another reader with no CacheHelper (Alan Woodward, Simon Willnauer,
+ Adrien Grand)
+
Other
* LUCENE-8228: removed obsolete IndexDeletionPolicy clone() requirements from
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/19fa91db/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 9391afd..27480e0 100644
--- a/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java
+++ b/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java
@@ -727,16 +727,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);
@@ -807,16 +808,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/19fa91db/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 74066ca..f4240e1 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,7 +1478,6 @@ public class TestLRUQueryCache extends LuceneTestCase {
}
}
- @Test
public void testDocValuesUpdatesDontBreakCache() throws IOException {
Directory dir = newDirectory();
IndexWriterConfig iwc = newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE);
@@ -1545,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();
+ }
}
[2/2] lucene-solr:branch_7x: LUCENE-8254: LRUQueryCache can leak locks
Posted by ro...@apache.org.
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/8eb6ed46
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/8eb6ed46
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/8eb6ed46
Branch: refs/heads/branch_7x
Commit: 8eb6ed4638e4e27e4dc22ac9d8f44b36238ff3ac
Parents: c662b7d
Author: Alan Woodward <ro...@apache.org>
Authored: Mon Apr 16 10:57:12 2018 +0100
Committer: Alan Woodward <ro...@apache.org>
Committed: Mon Apr 16 10:57:49 2018 +0100
----------------------------------------------------------------------
lucene/CHANGES.txt | 4 ++
.../org/apache/lucene/search/LRUQueryCache.java | 22 +++----
.../apache/lucene/search/TestLRUQueryCache.java | 63 +++++++++++++++++++-
3 files changed, 77 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8eb6ed46/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 4ac77ed..4bba094 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -69,6 +69,10 @@ Bug Fixes
index file names for updated doc values fields (Simon Willnauer,
Michael McCandless, Nhat Nguyen)
+* LUCENE-8254: LRUQueryCache could cause IndexReader to hang on close, when
+ shared with another reader with no CacheHelper (Alan Woodward, Simon Willnauer,
+ Adrien Grand)
+
Other
* LUCENE-8228: removed obsolete IndexDeletionPolicy clone() requirements from
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8eb6ed46/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 fcd84d0..b20e7cf 100644
--- a/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java
+++ b/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java
@@ -725,16 +725,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);
@@ -805,16 +806,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/8eb6ed46/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 25e5953..874d525 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,7 +1478,6 @@ public class TestLRUQueryCache extends LuceneTestCase {
}
}
- @Test
public void testDocValuesUpdatesDontBreakCache() throws IOException {
Directory dir = newDirectory();
IndexWriterConfig iwc = newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE);
@@ -1545,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();
+ }
}