jpountz commented on a change in pull request #916: LUCENE-8213: Asynchronous
Caching in LRUQueryCache
URL: https://github.com/apache/lucene-solr/pull/916#discussion_r332857726
##########
File path: lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java
##########
@@ -244,6 +272,225 @@ public void testLRUEviction() throws Exception {
dir.close();
}
+ public void testLRUConcurrentLoadAndEviction() throws Exception {
+ Directory dir = newDirectory();
+ final RandomIndexWriter w = new RandomIndexWriter(random(), dir);
+
+ Document doc = new Document();
+ StringField f = new StringField("color", "blue", Store.NO);
+ doc.add(f);
+ w.addDocument(doc);
+ f.setStringValue("red");
+ w.addDocument(doc);
+ f.setStringValue("green");
+ w.addDocument(doc);
+ final DirectoryReader reader = w.getReader();
+ ExecutorService service = new ThreadPoolExecutor(4, 4, 0L,
TimeUnit.MILLISECONDS,
+ new LinkedBlockingQueue<Runnable>(),
+ new NamedThreadFactory("TestLRUQueryCache"));
+
+ IndexSearcher searcher = new IndexSearcher(reader, service);
+
+ final CountDownLatch[] latch = {new CountDownLatch(1)};
+
+ final LRUQueryCache queryCache = new LRUQueryCache(2, 100000, context ->
true) {
+ @Override
+ protected void onDocIdSetCache(Object readerCoreKey, long ramBytesUsed) {
+ super.onDocIdSetCache(readerCoreKey, ramBytesUsed);
+ latch[0].countDown();
+ }
+ };
+
+ final Query blue = new TermQuery(new Term("color", "blue"));
+ final Query red = new TermQuery(new Term("color", "red"));
+ final Query green = new TermQuery(new Term("color", "green"));
+
+ assertEquals(Collections.emptyList(), queryCache.cachedQueries());
+
+ searcher.setQueryCache(queryCache);
+ // the filter is not cached on any segment: no changes
+ searcher.setQueryCachingPolicy(NEVER_CACHE);
+ searcher.search(new ConstantScoreQuery(green), 1);
+ assertEquals(Collections.emptyList(), queryCache.cachedQueries());
+
+ searcher.setQueryCachingPolicy(ALWAYS_CACHE);
+
+ // First read should miss
+ searcher.search(new ConstantScoreQuery(red), 1);
+
+ if (!(queryCache.cachedQueries().equals(Collections.emptyList()))) {
+ searcher.search(new ConstantScoreQuery(red), 1);
+ } else {
+ // Let the cache load be completed
+ latch[0].await(200, TimeUnit.MILLISECONDS);
Review comment:
I'd probably call `await()` without a timeout. The first reason is that CI
environments are sometimes very busy and even some simple operations can take
very long. The second is that if we get a timeout, we'll have no idea whether
the query took too long to cache or whether it was something else. On the other
hand, if we wait indefinitely, then the test runner will hit a timeout, and
when it does so it emits a thread dump in order to understand what the threads
were doing at the time of the timeout. So we'd have more information to
understand what happened. I'd suggest removing all timeouts in the below
await() calls.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
