Change GeoIpCache and EnrichCache to LongAdder (#132922) (#133251)

From the javadocs:

> [`LongAdder`] is usually preferable to `AtomicLong` when multiple threads update a common sum that is used for purposes such as collecting statistics, not for fine-grained synchronization control.

Since we largely write stats and read very infrequently, let's avoid any lock-contention possibilities by switching to the more appropriate data type.

Author: szybia

docs/changelog/132922.yaml

@@ -0,0 +1,5 @@
+pr: 132922
+summary: Change GeoIpCache and EnrichCache to LongAdder
+area: Ingest Node
+type: bug
+issues: []

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpCache.java

@@ -16,7 +16,7 @@
 import org.elasticsearch.ingest.geoip.stats.CacheStats;
 
 import java.nio.file.Path;
-import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.LongAdder;
 import java.util.function.Function;
 import java.util.function.LongSupplier;
 
@@ -43,8 +43,8 @@ public String toString() {
 
     private final LongSupplier relativeNanoTimeProvider;
     private final Cache<CacheKey, Object> cache;
-    private final AtomicLong hitsTimeInNanos = new AtomicLong(0);
-    private final AtomicLong missesTimeInNanos = new AtomicLong(0);
+    private final LongAdder hitsTimeInNanos = new LongAdder();
+    private final LongAdder missesTimeInNanos = new LongAdder();
 
     // package private for testing
     GeoIpCache(long maxSize, LongSupplier relativeNanoTimeProvider) {
@@ -79,9 +79,9 @@ <RESPONSE> RESPONSE putIfAbsent(String ip, String databasePath, Function<String,
             // store the result or no-result in the cache
             cache.put(cacheKey, response);
             long databaseRequestAndCachePutTime = relativeNanoTimeProvider.getAsLong() - retrieveStart;
-            missesTimeInNanos.addAndGet(cacheRequestTime + databaseRequestAndCachePutTime);
+            missesTimeInNanos.add(cacheRequestTime + databaseRequestAndCachePutTime);
         } else {
-            hitsTimeInNanos.addAndGet(cacheRequestTime);
+            hitsTimeInNanos.add(cacheRequestTime);
         }
 
         if (response == NO_RESULT) {
@@ -125,8 +125,8 @@ public CacheStats getCacheStats() {
             stats.getHits(),
             stats.getMisses(),
             stats.getEvictions(),
-            TimeValue.nsecToMSec(hitsTimeInNanos.get()),
-            TimeValue.nsecToMSec(missesTimeInNanos.get())
+            TimeValue.nsecToMSec(hitsTimeInNanos.sum()),
+            TimeValue.nsecToMSec(missesTimeInNanos.sum())
         );
     }
 

x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichCache.java

@@ -22,7 +22,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.LongAdder;
 import java.util.function.Consumer;
 import java.util.function.LongSupplier;
 import java.util.function.ToLongBiFunction;
@@ -46,9 +46,9 @@ public final class EnrichCache {
 
     private final Cache<CacheKey, CacheValue> cache;
     private final LongSupplier relativeNanoTimeProvider;
-    private final AtomicLong hitsTimeInNanos = new AtomicLong(0);
-    private final AtomicLong missesTimeInNanos = new AtomicLong(0);
-    private final AtomicLong sizeInBytes = new AtomicLong(0);
+    private final LongAdder hitsTimeInNanos = new LongAdder();
+    private final LongAdder missesTimeInNanos = new LongAdder();
+    private final LongAdder sizeInBytes = new LongAdder();
 
     EnrichCache(long maxSize) {
         this(maxSize, System::nanoTime);
@@ -71,7 +71,7 @@ public final class EnrichCache {
 
     private Cache<CacheKey, CacheValue> createCache(long maxWeight, ToLongBiFunction<CacheKey, CacheValue> weigher) {
         var builder = CacheBuilder.<CacheKey, CacheValue>builder().setMaximumWeight(maxWeight).removalListener(notification -> {
-            sizeInBytes.getAndAdd(-1 * notification.getValue().sizeInBytes);
+            sizeInBytes.add(-1 * notification.getValue().sizeInBytes);
         });
         if (weigher != null) {
             builder.weigher(weigher);
@@ -102,7 +102,7 @@ public void computeIfAbsent(
         List<Map<?, ?>> response = get(cacheKey);
         long cacheRequestTime = relativeNanoTimeProvider.getAsLong() - cacheStart;
         if (response != null) {
-            hitsTimeInNanos.addAndGet(cacheRequestTime);
+            hitsTimeInNanos.add(cacheRequestTime);
             listener.onResponse(response);
         } else {
             final long retrieveStart = relativeNanoTimeProvider.getAsLong();
@@ -111,7 +111,7 @@ public void computeIfAbsent(
                 put(cacheKey, cacheValue);
                 List<Map<?, ?>> copy = deepCopy(cacheValue.hits, false);
                 long databaseQueryAndCachePutTime = relativeNanoTimeProvider.getAsLong() - retrieveStart;
-                missesTimeInNanos.addAndGet(cacheRequestTime + databaseQueryAndCachePutTime);
+                missesTimeInNanos.add(cacheRequestTime + databaseQueryAndCachePutTime);
                 listener.onResponse(copy);
             }, listener::onFailure));
         }
@@ -130,7 +130,7 @@ public void computeIfAbsent(
     // non-private for unit testing only
     void put(CacheKey cacheKey, CacheValue cacheValue) {
         cache.put(cacheKey, cacheValue);
-        sizeInBytes.addAndGet(cacheValue.sizeInBytes);
+        sizeInBytes.add(cacheValue.sizeInBytes);
     }
 
     public EnrichStatsAction.Response.CacheStats getStats(String localNodeId) {
@@ -141,9 +141,9 @@ public EnrichStatsAction.Response.CacheStats getStats(String localNodeId) {
             cacheStats.getHits(),
             cacheStats.getMisses(),
             cacheStats.getEvictions(),
-            TimeValue.nsecToMSec(hitsTimeInNanos.get()),
-            TimeValue.nsecToMSec(missesTimeInNanos.get()),
-            sizeInBytes.get()
+            TimeValue.nsecToMSec(hitsTimeInNanos.sum()),
+            TimeValue.nsecToMSec(missesTimeInNanos.sum()),
+            sizeInBytes.sum()
         );
     }