Browse Source

Expose existing DLS cache x-pack usage statistics (#132845)

- add already-tracked DLS cache stats into `/xpack/usage?filter_path=security.roles.dls`
Szymon Bialkowski 2 months ago
parent
commit
ca5d778a7c

+ 5 - 0
docs/changelog/132845.yaml

@@ -0,0 +1,5 @@
+pr: 132845
+summary: Expose existing DLS cache x-pack usage statistics
+area: Authorization
+type: enhancement
+issues: []

+ 12 - 1
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCache.java

@@ -40,6 +40,8 @@ import org.elasticsearch.threadpool.ThreadPool;
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -320,7 +322,16 @@ public final class DocumentSubsetBitsetCache implements IndexReader.ClosedListen
 
     public Map<String, Object> usageStats() {
         final ByteSizeValue ram = ByteSizeValue.ofBytes(ramBytesUsed());
-        return Map.of("count", entryCount(), "memory", ram.toString(), "memory_in_bytes", ram.getBytes());
+        final Cache.Stats cacheStats = bitsetCache.stats();
+
+        final Map<String, Object> stats = new LinkedHashMap<>();
+        stats.put("count", entryCount());
+        stats.put("memory", ram.toString());
+        stats.put("memory_in_bytes", ram.getBytes());
+        stats.put("hits", cacheStats.getHits());
+        stats.put("misses", cacheStats.getMisses());
+        stats.put("evictions", cacheStats.getEvictions());
+        return Collections.unmodifiableMap(stats);
     }
 
     private static final class BitsetCacheKey {

+ 63 - 3
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java

@@ -53,6 +53,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.IdentityHashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -62,8 +63,10 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
 
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
@@ -396,9 +399,9 @@ public class DocumentSubsetBitsetCacheTests extends ESTestCase {
                 cache.verifyInternalConsistency();
 
                 // Due to cache evictions, we must get more bitsets than fields
-                assertThat(uniqueBitSets.size(), Matchers.greaterThan(FIELD_COUNT));
+                assertThat(uniqueBitSets.size(), greaterThan(FIELD_COUNT));
                 // Due to cache evictions, we must have seen more bitsets than the cache currently holds
-                assertThat(uniqueBitSets.size(), Matchers.greaterThan(cache.entryCount()));
+                assertThat(uniqueBitSets.size(), greaterThan(cache.entryCount()));
                 // Even under concurrent pressure, the cache should hit the expected size
                 assertThat(cache.entryCount(), is(maxCacheCount));
                 assertThat(cache.ramBytesUsed(), is(maxCacheBytes));
@@ -517,6 +520,64 @@ public class DocumentSubsetBitsetCacheTests extends ESTestCase {
         assertFalse(DocumentSubsetBitsetCache.isEffectiveMatchAllDocsQuery(new TermQuery(new Term("term"))));
     }
 
+    public void testHitsMissesAndEvictionsStats() throws Exception {
+        // cache that will evict all-but-one element, to test evictions
+        final long maxCacheBytes = EXPECTED_BYTES_PER_BIT_SET + (EXPECTED_BYTES_PER_BIT_SET / 2);
+        final Settings settings = Settings.builder()
+            .put(DocumentSubsetBitsetCache.CACHE_SIZE_SETTING.getKey(), maxCacheBytes + "b")
+            .build();
+        final DocumentSubsetBitsetCache cache = newCache(settings);
+
+        final Supplier<Map<String, Object>> emptyStatsSupplier = () -> {
+            final Map<String, Object> stats = new LinkedHashMap<>();
+            stats.put("count", 0);
+            stats.put("memory", "0b");
+            stats.put("memory_in_bytes", 0L);
+            stats.put("hits", 0L);
+            stats.put("misses", 0L);
+            stats.put("evictions", 0L);
+            return stats;
+        };
+
+        final Map<String, Object> expectedStats = emptyStatsSupplier.get();
+        assertThat(cache.usageStats(), equalTo(expectedStats));
+
+        runTestOnIndex((searchExecutionContext, leafContext) -> {
+            // first lookup - miss
+            final Query query1 = QueryBuilders.termQuery("field-1", "value-1").toQuery(searchExecutionContext);
+            final BitSet bitSet1 = cache.getBitSet(query1, leafContext);
+            assertThat(bitSet1, notNullValue());
+            expectedStats.put("count", 1);
+            expectedStats.put("misses", 1L);
+            expectedStats.put("memory", EXPECTED_BYTES_PER_BIT_SET + "b");
+            expectedStats.put("memory_in_bytes", EXPECTED_BYTES_PER_BIT_SET);
+            assertThat(cache.usageStats(), equalTo(expectedStats));
+
+            // second same lookup - hit
+            final BitSet bitSet1Again = cache.getBitSet(query1, leafContext);
+            assertThat(bitSet1Again, sameInstance(bitSet1));
+            expectedStats.put("hits", 1L);
+            assertThat(cache.usageStats(), equalTo(expectedStats));
+
+            // second query - miss, should evict the first one
+            final Query query2 = QueryBuilders.termQuery("field-2", "value-2").toQuery(searchExecutionContext);
+            final BitSet bitSet2 = cache.getBitSet(query2, leafContext);
+            assertThat(bitSet2, notNullValue());
+            // surprisingly, the eviction callback can call `get` on the cache (asynchronously) which causes another miss (or hit)
+            // so this assertion is about the current state of the code, rather than the expected or desired state.
+            // see https://github.com/elastic/elasticsearch/issues/132842
+            expectedStats.put("misses", 3L);
+            expectedStats.put("evictions", 1L);
+            assertBusy(() -> { assertThat(cache.usageStats(), equalTo(expectedStats)); }, 200, TimeUnit.MILLISECONDS);
+        });
+
+        final Map<String, Object> finalStats = emptyStatsSupplier.get();
+        finalStats.put("hits", 1L);
+        finalStats.put("misses", 3L);
+        finalStats.put("evictions", 2L);
+        assertThat(cache.usageStats(), equalTo(finalStats));
+    }
+
     private void runTestOnIndex(CheckedBiConsumer<SearchExecutionContext, LeafReaderContext, Exception> body) throws Exception {
         runTestOnIndices(1, ctx -> {
             final TestIndexContext indexContext = ctx.get(0);
@@ -638,5 +699,4 @@ public class DocumentSubsetBitsetCacheTests extends ESTestCase {
     private DocumentSubsetBitsetCache newCache(Settings settings) {
         return new DocumentSubsetBitsetCache(settings, singleThreadExecutor);
     }
-
 }