You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "javanna (via GitHub)" <gi...@apache.org> on 2026/09/29 09:03:36 UTC
[PR] Release default LRUQueryCache as early as possible in tests [lucene]
javanna opened a new pull request, #16740:
URL: https://github.com/apache/lucene/pull/16740
`LuceneTestCaseParent` currently assigns a new LRUQueryCache instance to each of its test methods, as well as one to the code execution outside of tests (e.g. before class methods)
This is important to make sure that each index searcher used starts with its own instance of LRUQueryCache.
All of the different instances are currently piled up in a static list, that gets iterated on in the after class method, which closes each item. The static list is never cleared, hence it ends up holding a lot of references. Each cache could be closed straight-away once it's out of scope. As a consequence, you can have extreme scenarios where accumulating query caches in the static list causes OOM.
This commit removes the list, and attempts to release resources held by the different LRU query caches instances as early as possible, by introducing an after test method.
The after class method stays but it closes only the static LRU query cache, no longer a list.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Release default LRUQueryCache as early as possible in tests [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #16740:
URL: https://github.com/apache/lucene/pull/16740#discussion_r4159918547
##########
lucene/test-framework/src/java/org/apache/lucene/tests/util/LuceneTestCaseParent.java:
##########
@@ -165,6 +165,7 @@
import org.apache.lucene.util.automaton.RegExp;
import org.hamcrest.Matcher;
import org.hamcrest.MatcherAssert;
+import org.junit.After;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Before;
Review Comment:
Ok, these seem like an oversight when jupiter was introduced. I'll file a separate PR to clean it up.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Release default LRUQueryCache as early as possible in tests [lucene]
Posted by "javanna (via GitHub)" <gi...@apache.org>.
javanna commented on code in PR #16740:
URL: https://github.com/apache/lucene/pull/16740#discussion_r4156905566
##########
lucene/test-framework/src/java/org/apache/lucene/tests/util/LuceneTestCaseParent.java:
##########
@@ -165,6 +165,7 @@
import org.apache.lucene.util.automaton.RegExp;
import org.hamcrest.Matcher;
import org.hamcrest.MatcherAssert;
+import org.junit.After;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Before;
Review Comment:
oh, I did not realize that. everything compiles and tests run though. Happy to review your followup when you get to it. Thanks for looking!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Release default LRUQueryCache as early as possible in tests [lucene]
Posted by "sgup432 (via GitHub)" <gi...@apache.org>.
sgup432 commented on code in PR #16740:
URL: https://github.com/apache/lucene/pull/16740#discussion_r4138359763
##########
lucene/test-framework/src/java/org/apache/lucene/tests/util/LuceneTestCaseParent.java:
##########
@@ -2536,36 +2537,46 @@ public static IOContext newIOContext(Random random, IOContext oldContext) {
private static final QueryCache DEFAULT_QUERY_CACHE = IndexSearcher.getDefaultQueryCache();
private static final QueryCachingPolicy DEFAULT_CACHING_POLICY =
IndexSearcher.getDefaultQueryCachingPolicy();
- private static final List<LRUQueryCache> queryCacheList = new ArrayList<>();
+ private static LRUQueryCache classQueryCache;
- @Before
- public void overrideTestDefaultQueryCache() {
- // Make sure each test method has its own cache
- overrideDefaultQueryCache();
+ private static LRUQueryCache createAndOverrideDefaultQueryCache() {
+ LRUQueryCache lruQueryCache =
+ new LRUQueryCache(10000, 1 << 25, _ -> true, Float.POSITIVE_INFINITY);
+ IndexSearcher.setDefaultQueryCache(lruQueryCache);
+ IndexSearcher.setDefaultQueryCachingPolicy(MAYBE_CACHE_POLICY);
+ return lruQueryCache;
}
@BeforeClass
- public static void overrideDefaultQueryCache() {
+ public static void overrideClassDefaultQueryCache() {
// we need to reset the query cache in an @BeforeClass so that tests that
// instantiate an IndexSearcher in an @BeforeClass method use a fresh new cache
- LRUQueryCache queryCacheTemp =
- new LRUQueryCache(10000, 1 << 25, _ -> true, Float.POSITIVE_INFINITY);
- queryCacheList.add(queryCacheTemp);
- IndexSearcher.setDefaultQueryCache(queryCacheTemp);
- IndexSearcher.setDefaultQueryCachingPolicy(MAYBE_CACHE_POLICY);
+ classQueryCache = createAndOverrideDefaultQueryCache();
}
@AfterClass
- public static void resetDefaultQueryCache() {
+ public static void resetClassDefaultQueryCache() throws IOException {
IndexSearcher.setDefaultQueryCache(DEFAULT_QUERY_CACHE);
IndexSearcher.setDefaultQueryCachingPolicy(DEFAULT_CACHING_POLICY);
- for (int i = 0; i < queryCacheList.size(); i++) {
- try {
- queryCacheList.get(i).close();
- } catch (IOException e) {
- throw new RuntimeException(e);
- }
- }
+ IOUtils.close(classQueryCache);
+ classQueryCache = null;
+ }
+
+ private LRUQueryCache methodQueryCache;
+
+ @Before
+ public void overrideTestDefaultQueryCache() {
+ methodQueryCache = createAndOverrideDefaultQueryCache();
+ }
+
+ @After
+ public void resetTestDefaultQueryCache() throws IOException {
+ // Restore the class-level cache so TestRules and @AfterClass still see a
+ // test-scoped cache rather than the process-wide production default.
+ IndexSearcher.setDefaultQueryCache(classQueryCache);
+ IndexSearcher.setDefaultQueryCachingPolicy(MAYBE_CACHE_POLICY);
+ IOUtils.close(methodQueryCache);
Review Comment:
The LRUQueryCache.close() actually doesn't do any resource freeing but just closes the background scheduled cleaner(this was introduced as part of recent refactoring), in this case it is a no-op.
I think we should also call methodQueryCache.clear() before this close() call.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Release default LRUQueryCache as early as possible in tests [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #16740:
URL: https://github.com/apache/lucene/pull/16740#discussion_r4163912966
##########
lucene/test-framework/src/java/org/apache/lucene/tests/util/LuceneTestCaseParent.java:
##########
@@ -165,6 +165,7 @@
import org.apache.lucene.util.automaton.RegExp;
import org.hamcrest.Matcher;
import org.hamcrest.MatcherAssert;
+import org.junit.After;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Before;
Review Comment:
It's here if you want to take a peek #16763 .
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Release default LRUQueryCache as early as possible in tests [lucene]
Posted by "javanna (via GitHub)" <gi...@apache.org>.
javanna commented on code in PR #16740:
URL: https://github.com/apache/lucene/pull/16740#discussion_r4142092638
##########
lucene/test-framework/src/java/org/apache/lucene/tests/util/LuceneTestCaseParent.java:
##########
@@ -2536,36 +2537,46 @@ public static IOContext newIOContext(Random random, IOContext oldContext) {
private static final QueryCache DEFAULT_QUERY_CACHE = IndexSearcher.getDefaultQueryCache();
private static final QueryCachingPolicy DEFAULT_CACHING_POLICY =
IndexSearcher.getDefaultQueryCachingPolicy();
- private static final List<LRUQueryCache> queryCacheList = new ArrayList<>();
+ private static LRUQueryCache classQueryCache;
- @Before
- public void overrideTestDefaultQueryCache() {
- // Make sure each test method has its own cache
- overrideDefaultQueryCache();
+ private static LRUQueryCache createAndOverrideDefaultQueryCache() {
+ LRUQueryCache lruQueryCache =
+ new LRUQueryCache(10000, 1 << 25, _ -> true, Float.POSITIVE_INFINITY);
+ IndexSearcher.setDefaultQueryCache(lruQueryCache);
+ IndexSearcher.setDefaultQueryCachingPolicy(MAYBE_CACHE_POLICY);
+ return lruQueryCache;
}
@BeforeClass
- public static void overrideDefaultQueryCache() {
+ public static void overrideClassDefaultQueryCache() {
// we need to reset the query cache in an @BeforeClass so that tests that
// instantiate an IndexSearcher in an @BeforeClass method use a fresh new cache
- LRUQueryCache queryCacheTemp =
- new LRUQueryCache(10000, 1 << 25, _ -> true, Float.POSITIVE_INFINITY);
- queryCacheList.add(queryCacheTemp);
- IndexSearcher.setDefaultQueryCache(queryCacheTemp);
- IndexSearcher.setDefaultQueryCachingPolicy(MAYBE_CACHE_POLICY);
+ classQueryCache = createAndOverrideDefaultQueryCache();
}
@AfterClass
- public static void resetDefaultQueryCache() {
+ public static void resetClassDefaultQueryCache() throws IOException {
IndexSearcher.setDefaultQueryCache(DEFAULT_QUERY_CACHE);
IndexSearcher.setDefaultQueryCachingPolicy(DEFAULT_CACHING_POLICY);
- for (int i = 0; i < queryCacheList.size(); i++) {
- try {
- queryCacheList.get(i).close();
- } catch (IOException e) {
- throw new RuntimeException(e);
- }
- }
+ IOUtils.close(classQueryCache);
+ classQueryCache = null;
+ }
+
+ private LRUQueryCache methodQueryCache;
+
+ @Before
+ public void overrideTestDefaultQueryCache() {
+ methodQueryCache = createAndOverrideDefaultQueryCache();
+ }
+
+ @After
+ public void resetTestDefaultQueryCache() throws IOException {
+ // Restore the class-level cache so TestRules and @AfterClass still see a
+ // test-scoped cache rather than the process-wide production default.
+ IndexSearcher.setDefaultQueryCache(classQueryCache);
+ IndexSearcher.setDefaultQueryCachingPolicy(MAYBE_CACHE_POLICY);
+ IOUtils.close(methodQueryCache);
Review Comment:
That's a good point, thanks! What I was mainly after here was clearing the previous list, which was the root cause for the OOMs that I have seen. But clearing the cache before closing sounds like a good improvement.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Release default LRUQueryCache as early as possible in tests [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #16740:
URL: https://github.com/apache/lucene/pull/16740#discussion_r4155640793
##########
lucene/test-framework/src/java/org/apache/lucene/tests/util/LuceneTestCaseParent.java:
##########
@@ -165,6 +165,7 @@
import org.apache.lucene.util.automaton.RegExp;
import org.hamcrest.Matcher;
import org.hamcrest.MatcherAssert;
+import org.junit.After;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Before;
Review Comment:
Sorry, I missed it. Something is wrong here - these imports shouldn't be in this class since it's shared with jupiter base class. I'll take a look at this later and write a follow-up.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Release default LRUQueryCache as early as possible in tests [lucene]
Posted by "sgup432 (via GitHub)" <gi...@apache.org>.
sgup432 commented on code in PR #16740:
URL: https://github.com/apache/lucene/pull/16740#discussion_r4138359763
##########
lucene/test-framework/src/java/org/apache/lucene/tests/util/LuceneTestCaseParent.java:
##########
@@ -2536,36 +2537,46 @@ public static IOContext newIOContext(Random random, IOContext oldContext) {
private static final QueryCache DEFAULT_QUERY_CACHE = IndexSearcher.getDefaultQueryCache();
private static final QueryCachingPolicy DEFAULT_CACHING_POLICY =
IndexSearcher.getDefaultQueryCachingPolicy();
- private static final List<LRUQueryCache> queryCacheList = new ArrayList<>();
+ private static LRUQueryCache classQueryCache;
- @Before
- public void overrideTestDefaultQueryCache() {
- // Make sure each test method has its own cache
- overrideDefaultQueryCache();
+ private static LRUQueryCache createAndOverrideDefaultQueryCache() {
+ LRUQueryCache lruQueryCache =
+ new LRUQueryCache(10000, 1 << 25, _ -> true, Float.POSITIVE_INFINITY);
+ IndexSearcher.setDefaultQueryCache(lruQueryCache);
+ IndexSearcher.setDefaultQueryCachingPolicy(MAYBE_CACHE_POLICY);
+ return lruQueryCache;
}
@BeforeClass
- public static void overrideDefaultQueryCache() {
+ public static void overrideClassDefaultQueryCache() {
// we need to reset the query cache in an @BeforeClass so that tests that
// instantiate an IndexSearcher in an @BeforeClass method use a fresh new cache
- LRUQueryCache queryCacheTemp =
- new LRUQueryCache(10000, 1 << 25, _ -> true, Float.POSITIVE_INFINITY);
- queryCacheList.add(queryCacheTemp);
- IndexSearcher.setDefaultQueryCache(queryCacheTemp);
- IndexSearcher.setDefaultQueryCachingPolicy(MAYBE_CACHE_POLICY);
+ classQueryCache = createAndOverrideDefaultQueryCache();
}
@AfterClass
- public static void resetDefaultQueryCache() {
+ public static void resetClassDefaultQueryCache() throws IOException {
IndexSearcher.setDefaultQueryCache(DEFAULT_QUERY_CACHE);
IndexSearcher.setDefaultQueryCachingPolicy(DEFAULT_CACHING_POLICY);
- for (int i = 0; i < queryCacheList.size(); i++) {
- try {
- queryCacheList.get(i).close();
- } catch (IOException e) {
- throw new RuntimeException(e);
- }
- }
+ IOUtils.close(classQueryCache);
+ classQueryCache = null;
+ }
+
+ private LRUQueryCache methodQueryCache;
+
+ @Before
+ public void overrideTestDefaultQueryCache() {
+ methodQueryCache = createAndOverrideDefaultQueryCache();
+ }
+
+ @After
+ public void resetTestDefaultQueryCache() throws IOException {
+ // Restore the class-level cache so TestRules and @AfterClass still see a
+ // test-scoped cache rather than the process-wide production default.
+ IndexSearcher.setDefaultQueryCache(classQueryCache);
+ IndexSearcher.setDefaultQueryCachingPolicy(MAYBE_CACHE_POLICY);
+ IOUtils.close(methodQueryCache);
Review Comment:
The LRUQueryCache.close() actually doesn't do any resource freeing but just closes the background scheduled cleaner(this was introduced as part of recent refactoring), in this it is a no-op.
I think we should also call methodQueryCache.clear() before this close() call.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Release default LRUQueryCache as early as possible in tests [lucene]
Posted by "javanna (via GitHub)" <gi...@apache.org>.
javanna commented on PR #16740:
URL: https://github.com/apache/lucene/pull/16740#issuecomment-5914473576
thanks @jimczi !
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Release default LRUQueryCache as early as possible in tests [lucene]
Posted by "javanna (via GitHub)" <gi...@apache.org>.
javanna merged PR #16740:
URL: https://github.com/apache/lucene/pull/16740
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org