sgup432 commented on code in PR #15558:
URL: https://github.com/apache/lucene/pull/15558#discussion_r2695478747
##########
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:
Before this PR, yes we don't remove unique queries from `uniqueQueries`
because of the reason you mentioned. They can be either removed explicitly via
`clearQuery` or automatically through LRU policy(inside `evictIfNecessary`).
With this PR, we are not tracking unique queries anymore. Instead this cache
now a structure as `<<Query, Segment>, Value>` where key is a composite key and
maxSize represents total keys a cache can hold. So if a segment is closed, then
any keys containing such stale segments would be removed(along with associated
query).
--
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]
