This is an automated email from the ASF dual-hosted git repository.
magibney pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new a7a4765ef6f SOLR-17597: Fix `CaffeineCache.put()` ramBytes decrement
(#2917)
a7a4765ef6f is described below
commit a7a4765ef6f72f4508caf2f6264dd3df134eba84
Author: Michael Gibney <[email protected]>
AuthorDate: Fri Dec 20 16:12:37 2024 -0500
SOLR-17597: Fix `CaffeineCache.put()` ramBytes decrement (#2917)
(cherry picked from commit a40826eae6d0dd3b60166844a3be048a6dfee40f)
---
.../java/org/apache/solr/search/CaffeineCache.java | 36 ++++++++++++----------
.../org/apache/solr/search/TestCaffeineCache.java | 4 +--
2 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/search/CaffeineCache.java
b/solr/core/src/java/org/apache/solr/search/CaffeineCache.java
index 1d921030ff7..091dd984fa5 100644
--- a/solr/core/src/java/org/apache/solr/search/CaffeineCache.java
+++ b/solr/core/src/java/org/apache/solr/search/CaffeineCache.java
@@ -232,7 +232,7 @@ public class CaffeineCache<K, V> extends SolrCacheBase
// We reserved the slot, so we do the work
V value = mappingFunction.apply(key);
future.complete(value); // This will update the weight and expiration
- recordRamBytes(key, null, value);
+ recordRamBytes(key, value);
inserts.increment();
return value;
} catch (Error | RuntimeException | IOException e) {
@@ -262,7 +262,7 @@ public class CaffeineCache<K, V> extends SolrCacheBase
if (value == null) {
return null;
}
- recordRamBytes(key, null, value);
+ recordRamBytes(key, value);
inserts.increment();
return value;
});
@@ -275,30 +275,34 @@ public class CaffeineCache<K, V> extends SolrCacheBase
public V put(K key, V val) {
inserts.increment();
V old = cache.asMap().put(key, val);
- recordRamBytes(key, old, val);
+ // ramBytes decrement for `old` happens via #onRemoval
+ if (val != old) {
+ // NOTE: this is conditional on `val != old` in order to work around a
+ // behavior in the Caffeine library: where there is reference equality
+ // between `val` and `old`, caffeine does _not_ invoke RemovalListener,
+ // so the entry is not decremented for the replaced value (hence we
+ // don't need to increment ram bytes for the entry either).
+ recordRamBytes(key, val);
+ }
return old;
}
/**
- * Update the estimate of used memory
+ * Update the estimate of used memory.
+ *
+ * <p>NOTE: old value (in the event of replacement) adjusts {@link
#ramBytes} via {@link
+ * #onRemoval(Object, Object, RemovalCause)}
*
* @param key the cache key
- * @param oldValue the old cached value to decrement estimate (can be null)
* @param newValue the new cached value to increment estimate
*/
- private void recordRamBytes(K key, V oldValue, V newValue) {
+ private void recordRamBytes(K key, V newValue) {
ramBytes.add(
RamUsageEstimator.sizeOfObject(newValue,
RamUsageEstimator.QUERY_DEFAULT_RAM_BYTES_USED));
- if (oldValue == null) {
- ramBytes.add(
- RamUsageEstimator.sizeOfObject(key,
RamUsageEstimator.QUERY_DEFAULT_RAM_BYTES_USED));
- ramBytes.add(RamUsageEstimator.LINKED_HASHTABLE_RAM_BYTES_PER_ENTRY);
- if (async) ramBytes.add(RAM_BYTES_PER_FUTURE);
- } else {
- ramBytes.add(
- -RamUsageEstimator.sizeOfObject(
- oldValue, RamUsageEstimator.QUERY_DEFAULT_RAM_BYTES_USED));
- }
+ ramBytes.add(
+ RamUsageEstimator.sizeOfObject(key,
RamUsageEstimator.QUERY_DEFAULT_RAM_BYTES_USED));
+ ramBytes.add(RamUsageEstimator.LINKED_HASHTABLE_RAM_BYTES_PER_ENTRY);
+ if (async) ramBytes.add(RAM_BYTES_PER_FUTURE);
}
@Override
diff --git a/solr/core/src/test/org/apache/solr/search/TestCaffeineCache.java
b/solr/core/src/test/org/apache/solr/search/TestCaffeineCache.java
index 87ff29d5e6a..b4c69600fbd 100644
--- a/solr/core/src/test/org/apache/solr/search/TestCaffeineCache.java
+++ b/solr/core/src/test/org/apache/solr/search/TestCaffeineCache.java
@@ -308,7 +308,7 @@ public class TestCaffeineCache extends SolrTestCase {
cache.put(0, "test");
long nonEmptySize = cache.ramBytesUsed();
- cache.put(0, "test");
+ cache.put(0, random().nextBoolean() ? "test" : "rest");
assertEquals(nonEmptySize, cache.ramBytesUsed());
cache.remove(0);
@@ -341,7 +341,7 @@ public class TestCaffeineCache extends SolrTestCase {
cache.put(0, "test");
long nonEmptySize = cache.ramBytesUsed();
- cache.put(0, "test");
+ cache.put(0, random().nextBoolean() ? "test" : "rest");
assertEquals(nonEmptySize, cache.ramBytesUsed());
cache.remove(0);
