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