From d06fc06dfa8b76c7994d735e0916448cdfa23651 Mon Sep 17 00:00:00 2001 From: Szymon Bialkowski Date: Wed, 13 Aug 2025 17:21:29 +0200 Subject: [PATCH 1/6] Expose existing DLS cache x-pack usage statistics --- .../DocumentSubsetBitsetCache.java | 13 ++++- .../DocumentSubsetBitsetCacheTests.java | 54 +++++++++++++++++-- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCache.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCache.java index 18c13860efd6a..7c7dec55e0bd5 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCache.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCache.java @@ -40,6 +40,8 @@ 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 static List> getSettings() { public Map 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 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 { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java index 0645ea8b43b16..5ae81262f9dc5 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java @@ -53,6 +53,7 @@ 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; @@ -64,6 +65,7 @@ import java.util.concurrent.atomic.AtomicReference; 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 +398,9 @@ public void testCacheUnderConcurrentAccess() throws Exception { 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 +519,53 @@ public void testEquivalentMatchAllDocsQuery() { 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 Map expectedStats = new LinkedHashMap<>(); + expectedStats.put("count", 0); + expectedStats.put("memory", "0b"); + expectedStats.put("memory_in_bytes", 0L); + expectedStats.put("hits", 0L); + expectedStats.put("misses", 0L); + expectedStats.put("evictions", 0L); + 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()); + + // second same lookup - hit + final BitSet bitSet1Again = cache.getBitSet(query1, leafContext); + assertThat(bitSet1Again, sameInstance(bitSet1)); + + expectedStats.put("hits", 1L); + expectedStats.put("misses", 1L); + expectedStats.put("count", 1); + 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 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()); + + // szymon: bug I believe, created issue: https://github.com/elastic/elasticsearch/issues/132842 + // assertBusy can be removed once the bug is fixed. + expectedStats.put("misses", 3L); + expectedStats.put("evictions", 1L); + assertBusy(() -> { assertThat(cache.usageStats(), equalTo(expectedStats)); }, 200, TimeUnit.MILLISECONDS); + }); + } + private void runTestOnIndex(CheckedBiConsumer body) throws Exception { runTestOnIndices(1, ctx -> { final TestIndexContext indexContext = ctx.get(0); @@ -638,5 +687,4 @@ private void runTestOnIndices(int numberIndices, CheckedConsumer Date: Thu, 14 Aug 2025 12:33:01 +0200 Subject: [PATCH 2/6] update comment --- .../authz/accesscontrol/DocumentSubsetBitsetCacheTests.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java index 5ae81262f9dc5..ef57a655d9c94 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java @@ -558,8 +558,9 @@ public void testHitsMissesAndEvictionsStats() throws Exception { final BitSet bitSet2 = cache.getBitSet(query2, leafContext); assertThat(bitSet2, notNullValue()); - // szymon: bug I believe, created issue: https://github.com/elastic/elasticsearch/issues/132842 - // assertBusy can be removed once the bug is fixed. + // szymon: eviction callback calls `get` on the cache, asynchronously, which updates the stats. + // so assertion is current state of the code, rather than the expected state. + // issue: https://github.com/elastic/elasticsearch/issues/132842 expectedStats.put("misses", 3L); expectedStats.put("evictions", 1L); assertBusy(() -> { assertThat(cache.usageStats(), equalTo(expectedStats)); }, 200, TimeUnit.MILLISECONDS); From 6a35dedb904f4bf592c747d3358c2fb01837c2ac Mon Sep 17 00:00:00 2001 From: Szymon Bialkowski Date: Thu, 14 Aug 2025 15:12:51 +0200 Subject: [PATCH 3/6] Update docs/changelog/132845.yaml --- docs/changelog/132845.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/132845.yaml diff --git a/docs/changelog/132845.yaml b/docs/changelog/132845.yaml new file mode 100644 index 0000000000000..ff15b91a9113a --- /dev/null +++ b/docs/changelog/132845.yaml @@ -0,0 +1,5 @@ +pr: 132845 +summary: Expose existing DLS cache x-pack usage statistics +area: Authorization +type: enhancement +issues: [] From 695e22db37e6d5e76683c78d2ff35b5757bf9ba7 Mon Sep 17 00:00:00 2001 From: Szymon Bialkowski Date: Fri, 15 Aug 2025 15:19:52 +0200 Subject: [PATCH 4/6] joe comments --- .../DocumentSubsetBitsetCacheTests.java | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java index ef57a655d9c94..aa059f80dd6bc 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java @@ -63,6 +63,7 @@ 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; @@ -527,13 +528,18 @@ public void testHitsMissesAndEvictionsStats() throws Exception { .build(); final DocumentSubsetBitsetCache cache = newCache(settings); - final Map expectedStats = new LinkedHashMap<>(); - expectedStats.put("count", 0); - expectedStats.put("memory", "0b"); - expectedStats.put("memory_in_bytes", 0L); - expectedStats.put("hits", 0L); - expectedStats.put("misses", 0L); - expectedStats.put("evictions", 0L); + final Supplier> emptyStatsSupplier = () -> { + final Map 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 expectedStats = emptyStatsSupplier.get(); assertThat(cache.usageStats(), equalTo(expectedStats)); runTestOnIndex((searchExecutionContext, leafContext) -> { @@ -541,16 +547,18 @@ public void testHitsMissesAndEvictionsStats() throws Exception { 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); - expectedStats.put("misses", 1L); - expectedStats.put("count", 1); - 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 query - miss, should evict the first one @@ -558,13 +566,21 @@ public void testHitsMissesAndEvictionsStats() throws Exception { final BitSet bitSet2 = cache.getBitSet(query2, leafContext); assertThat(bitSet2, notNullValue()); - // szymon: eviction callback calls `get` on the cache, asynchronously, which updates the stats. + // eviction callback calls `get` on the cache, asynchronously, which updates the stats. // so assertion is current state of the code, rather than the expected state. // issue: 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 finalStats = emptyStatsSupplier.get(); + // related to comment above: surprisingly last eviction doesn't increment hits nor misses, because it goes through onClose(), + // and short-circuits in the eviction callback before doing a `get`. + finalStats.put("hits", 1L); + finalStats.put("misses", 3L); + finalStats.put("evictions", 2L); + assertThat(cache.usageStats(), equalTo(finalStats)); } private void runTestOnIndex(CheckedBiConsumer body) throws Exception { From 074d280ac410e315584611c70ce383daaddecdd1 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 15 Aug 2025 15:06:34 -0400 Subject: [PATCH 5/6] Whitespace --- .../authz/accesscontrol/DocumentSubsetBitsetCacheTests.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java index aa059f80dd6bc..69534251ffa0f 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java @@ -551,13 +551,11 @@ public void testHitsMissesAndEvictionsStats() throws Exception { 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)); @@ -565,7 +563,6 @@ public void testHitsMissesAndEvictionsStats() throws Exception { final Query query2 = QueryBuilders.termQuery("field-2", "value-2").toQuery(searchExecutionContext); final BitSet bitSet2 = cache.getBitSet(query2, leafContext); assertThat(bitSet2, notNullValue()); - // eviction callback calls `get` on the cache, asynchronously, which updates the stats. // so assertion is current state of the code, rather than the expected state. // issue: https://github.com/elastic/elasticsearch/issues/132842 From c91bc87f10a02020f2bdd9ebd09524a3766f6649 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 15 Aug 2025 15:06:47 -0400 Subject: [PATCH 6/6] Reword some comments --- .../accesscontrol/DocumentSubsetBitsetCacheTests.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java index 69534251ffa0f..19c48c182b85b 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java @@ -563,17 +563,15 @@ public void testHitsMissesAndEvictionsStats() throws Exception { final Query query2 = QueryBuilders.termQuery("field-2", "value-2").toQuery(searchExecutionContext); final BitSet bitSet2 = cache.getBitSet(query2, leafContext); assertThat(bitSet2, notNullValue()); - // eviction callback calls `get` on the cache, asynchronously, which updates the stats. - // so assertion is current state of the code, rather than the expected state. - // issue: https://github.com/elastic/elasticsearch/issues/132842 + // 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 finalStats = emptyStatsSupplier.get(); - // related to comment above: surprisingly last eviction doesn't increment hits nor misses, because it goes through onClose(), - // and short-circuits in the eviction callback before doing a `get`. finalStats.put("hits", 1L); finalStats.put("misses", 3L); finalStats.put("evictions", 2L);