From 2c5c07d1df7e5cb74cdd2457ef4c70788f14041c Mon Sep 17 00:00:00 2001
From: Adrien Grand <jpountz@gmail.com>
Date: Tue, 26 Mar 2019 14:23:10 +0100
Subject: [PATCH] Enhancements to IndicesQueryCache. (#39099) (#40446)

This commit adds the following:
 - made `IndicesQueryCache.stats2` a synchronized map. All writes to it are
   already protected by the lock of the Lucene cache, but the final read from
   an assertion in `IndicesQueryCache#close()` was not so this change should
   avoid any potential visibility issues.
 - human-readable `toString`s to make debugging easier.

Relates #37117
---
 .../indices/IndicesQueryCache.java            | 26 +++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java b/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java
index 814288dbe588f..dc054f8b51d3e 100644
--- a/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java
+++ b/server/src/main/java/org/elasticsearch/indices/IndicesQueryCache.java
@@ -42,6 +42,7 @@
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.IdentityHashMap;
 import java.util.Map;
@@ -71,7 +72,7 @@ public class IndicesQueryCache implements QueryCache, Closeable {
     // This is a hack for the fact that the close listener for the
     // ShardCoreKeyMap will be called before onDocIdSetEviction
     // See onDocIdSetEviction for more info
-    private final Map<Object, StatsAndCount> stats2 = new IdentityHashMap<>();
+    private final Map<Object, StatsAndCount> stats2 = Collections.synchronizedMap(new IdentityHashMap<>());
 
     public IndicesQueryCache(Settings settings) {
         final ByteSizeValue size = INDICES_CACHE_QUERY_SIZE_SETTING.get(settings);
@@ -189,20 +190,35 @@ public void close() {
         assert shardKeyMap.size() == 0 : shardKeyMap.size();
         assert shardStats.isEmpty() : shardStats.keySet();
         assert stats2.isEmpty() : stats2;
+
+        // This cache stores two things: filters, and doc id sets. At this time
+        // we only know that there are no more doc id sets, but we still track
+        // recently used queries, which we want to reclaim.
         cache.clear();
     }
 
     private static class Stats implements Cloneable {
 
+        final ShardId shardId;
         volatile long ramBytesUsed;
         volatile long hitCount;
         volatile long missCount;
         volatile long cacheCount;
         volatile long cacheSize;
 
+        Stats(ShardId shardId) {
+            this.shardId = shardId;
+        }
+
         QueryCacheStats toQueryCacheStats() {
             return new QueryCacheStats(ramBytesUsed, hitCount, missCount, cacheCount, cacheSize);
         }
+
+        @Override
+        public String toString() {
+            return "{shardId=" + shardId + ", ramBytedUsed=" + ramBytesUsed + ", hitCount=" + hitCount + ", missCount=" + missCount +
+                    ", cacheCount=" + cacheCount + ", cacheSize=" + cacheSize + "}";
+        }
     }
 
     private static class StatsAndCount {
@@ -213,6 +229,11 @@ private static class StatsAndCount {
             this.stats = stats;
             this.count = 0;
         }
+
+        @Override
+        public String toString() {
+            return "{stats=" + stats + " ,count=" + count + "}";
+        }
     }
 
     private boolean empty(Stats stats) {
@@ -249,7 +270,7 @@ private Stats getOrCreateStats(Object coreKey) {
             final ShardId shardId = shardKeyMap.getShardId(coreKey);
             Stats stats = shardStats.get(shardId);
             if (stats == null) {
-                stats = new Stats();
+                stats = new Stats(shardId);
                 shardStats.put(shardId, stats);
             }
             return stats;
@@ -265,6 +286,7 @@ protected void onClear() {
                 stats.cacheSize = 0;
                 stats.ramBytesUsed = 0;
             }
+            stats2.clear();
             sharedRamBytesUsed = 0;
         }