BaurzhanSakhariev commented on code in PR #15558:
URL: https://github.com/apache/lucene/pull/15558#discussion_r2693966867
##########
lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java:
##########
@@ -253,182 +304,63 @@ protected void onQueryEviction(Query query, long
ramBytesUsed) {
* @see #onDocIdSetEviction
* @lucene.experimental
*/
- protected void onDocIdSetCache(Object readerCoreKey, long ramBytesUsed) {
- assert writeLock.isHeldByCurrentThread();
- cacheSize += 1;
- cacheCount += 1;
- this.ramBytesUsed += ramBytesUsed;
- }
+ protected void onDocIdSetCache(Object readerCoreKey, long ramBytesUsed) {}
/**
* Expert: callback when one or more {@link DocIdSet}s are removed from this
cache.
*
* @see #onDocIdSetCache
* @lucene.experimental
*/
- protected void onDocIdSetEviction(Object readerCoreKey, int numEntries, long
sumRamBytesUsed) {
- assert writeLock.isHeldByCurrentThread();
- this.ramBytesUsed -= sumRamBytesUsed;
- cacheSize -= numEntries;
- }
+ protected void onDocIdSetEviction(Object readerCoreKey, int numEntries, long
sumRamBytesUsed) {}
/**
* Expert: callback when the cache is completely cleared.
*
* @lucene.experimental
*/
- protected void onClear() {
- assert writeLock.isHeldByCurrentThread();
- ramBytesUsed = 0;
- cacheSize = 0;
- }
-
- /** Whether evictions are required. */
- boolean requiresEviction() {
- assert writeLock.isHeldByCurrentThread();
- final int size = mostRecentlyUsedQueries.size();
- if (size == 0) {
- return false;
- } else {
- return size > maxSize || ramBytesUsed() > maxRamBytesUsed;
- }
- }
+ protected void onClear() {}
- CacheAndCount get(Query key, IndexReader.CacheHelper cacheHelper) {
+ public CacheAndCount get(Query key, IndexReader.CacheHelper cacheHelper) {
assert key instanceof BoostQuery == false;
assert key instanceof ConstantScoreQuery == false;
final IndexReader.CacheKey readerKey = cacheHelper.getKey();
- final LeafCache leafCache = cache.get(readerKey);
- if (leafCache == null) {
- onMiss(readerKey, key);
- return null;
- }
- // this get call moves the query to the most-recently-used position
- final QueryMetadata record = uniqueQueries.get(key);
- if (record == null || record.query == null) {
- onMiss(readerKey, key);
- return null;
- }
- final CacheAndCount cached = leafCache.get(record.query);
- if (cached == null) {
- onMiss(readerKey, record.query);
- } else {
- onHit(readerKey, record.query);
- }
- return cached;
+ QueryCacheKey queryCacheKey = new QueryCacheKey(readerKey, key);
+ int partitionNumber = getPartitionNumber(queryCacheKey);
+ return this.lruQueryCachePartition[partitionNumber].get(queryCacheKey,
key, cacheHelper);
}
public void putIfAbsent(Query query, CacheAndCount cached,
IndexReader.CacheHelper cacheHelper) {
assert query instanceof BoostQuery == false;
assert query instanceof ConstantScoreQuery == false;
- // under a lock to make sure that mostRecentlyUsedQueries and cache remain
sync'ed
- writeLock.lock();
- try {
- QueryMetadata record =
- uniqueQueries.computeIfAbsent(
- query,
- q -> {
- long queryRamBytesUsed = getRamBytesUsed(q);
- onQueryCache(q, queryRamBytesUsed);
- return new QueryMetadata(q, queryRamBytesUsed);
- });
- query = record.query;
- final IndexReader.CacheKey key = cacheHelper.getKey();
- LeafCache leafCache = cache.get(key);
- if (leafCache == null) {
- leafCache = new LeafCache(key);
- final LeafCache previous = cache.put(key, leafCache);
- ramBytesUsed += HASHTABLE_RAM_BYTES_PER_ENTRY;
- assert previous == null;
- // we just created a new leaf cache, need to register a close listener
- cacheHelper.addClosedListener(this::clearCoreCacheKey);
- }
- leafCache.putIfAbsent(query, cached);
- evictIfNecessary();
- } finally {
- writeLock.unlock();
- }
- }
-
- private void evictIfNecessary() {
- assert writeLock.isHeldByCurrentThread();
- // under a lock to make sure that mostRecentlyUsedQueries and cache keep
sync'ed
- if (requiresEviction()) {
- Iterator<Map.Entry<Query, QueryMetadata>> iterator =
uniqueQueries.entrySet().iterator();
- do {
- final Map.Entry<Query, QueryMetadata> entry = iterator.next();
- final int size = uniqueQueries.size();
- iterator.remove();
- if (size == uniqueQueries.size()) {
- // size did not decrease, because the hash of the query changed
since it has been
- // put into the cache
- throw new ConcurrentModificationException(
- "Removal from the cache failed! This "
- + "is probably due to a query which has been modified after
having been put into "
- + " the cache or a badly implemented clone(). Query class: ["
- + entry.getKey().getClass()
- + "], query: ["
- + entry.getKey()
- + "]");
- }
- onEviction(entry.getKey(), entry.getValue().queryRamBytesUsed);
- } while (iterator.hasNext() && requiresEviction());
- }
+ final IndexReader.CacheKey key = cacheHelper.getKey();
+ QueryCacheKey queryCacheKey = new QueryCacheKey(key, query);
+ int partitionNumber = getPartitionNumber(queryCacheKey);
+ this.lruQueryCachePartition[partitionNumber].putIfAbsent(
+ queryCacheKey, query, cacheHelper, cached);
}
/** Remove all cache entries for the given core cache key. */
- public void clearCoreCacheKey(Object coreKey) {
- writeLock.lock();
- try {
- final LeafCache leafCache = cache.remove(coreKey);
- if (leafCache != null) {
- ramBytesUsed -= HASHTABLE_RAM_BYTES_PER_ENTRY;
- final int numEntries = leafCache.cache.size();
- if (numEntries > 0) {
- onDocIdSetEviction(coreKey, numEntries, leafCache.ramBytesUsed);
- } else {
- assert numEntries == 0;
- assert leafCache.ramBytesUsed == 0;
- }
- }
- } finally {
- writeLock.unlock();
+ public void clearCoreCacheKey(IndexReader.CacheKey coreKey) {
+ this.registeredClosedListeners.remove(coreKey);
+ synchronized (keysToClean) {
+ keysToClean.add(coreKey);
Review Comment:
Sorry for chiming in here, just wanted to ask as I have been investigating
OOM due to cache:
Do I get it right, that in the revision before this PR (`uniqueQueries`)
and with this PR (multiple `LRUQueryCachePartition`-s),
`clearCoreCacheKey` doesn't remove unique queries, ie doesn't remove
`uniqueQueries` or `LRUQueryCachePartition.uniqueCacheKeys`?
I know that we can't remove `uniqueQuery` on `coreCacheKey` clearance
because it can be used in other `LeafCache`(main)/other partitions(this PR).
However, it should be possible to remove if it's the last one across all
leafCaches/all partitions? Probably "last one" can be checked by adding query
occurrences to the `QueryMetadata` and using `clearQuery` when
`clearCoreCacheKey` decrements occurrences to 0.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
