DocumentSubsetBitsetCache eviction increments `misses` statistic

[szybia]

Elasticsearch Version

9.2

Installed Plugins

No response

Java Version

bundled

OS Version

24.6.0 Darwin Kernel Version 24.6.0

Problem Description

going to attach code once i merge it

Steps to Reproduce

going to attach code once i merge it

Logs (if relevant)

No response

[elasticsearchmachine]

Pinging @elastic/es-data-management (Team:Data Management)

[joegallo]

This isn't a problem with the general Cache, it's specifically a problem with the cache evictions RemovalListener in the DocumentSubsetBitsetCache. Notice that we call bitsetCache.get(...) near the bottom of this block:

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

Lines 166 to 186 in a995a12

	private void onCacheEviction(RemovalNotification<BitsetCacheKey, BitSet> notification) {
	final BitsetCacheKey bitsetKey = notification.getKey();
	final IndexReader.CacheKey indexKey = bitsetKey.index;
	if (keysByIndex.getOrDefault(indexKey, Set.of()).contains(bitsetKey) == false) {
	// If the bitsetKey isn't in the lookup map, then there's nothing to synchronize
	return;
	}
	// We push this to a background thread, so that it reduces the risk of blocking searches, but also so that the lock management is
	// simpler - this callback is likely to take place on a thread that is actively adding something to the cache, and is therefore
	// holding the read ("update") side of the lock. It is not possible to upgrade a read lock to a write lock ("eviction"), but we
	// need to acquire that lock here.
	cleanupExecutor.submit(() -> {
	try (ReleasableLock ignored = cacheEvictionLock.acquire()) {
	// it's possible for the key to be back in the cache if it was immediately repopulated after it was evicted, so check
	if (bitsetCache.get(bitsetKey) == null) {
	// key is no longer in the cache, make sure it is no longer in the lookup map either.
	Optional.ofNullable(keysByIndex.get(indexKey)).ifPresent(set -> set.remove(bitsetKey));
	}
	}
	});
	}

That runs in a background thread, which also explains why you have to catch it in an assertBusy, and why if you try it both ways you see that almost always the number of misses is 2, but sometimes it's 3, but if you wait long enough (with the assertBusy) then it'll always end up being 3.

I don't think there's anything for us to do about this math at this point -- part of the work I'm doing for DLS anyway is refactoring and rewriting the CloseListener and RemovalListener here to avoid the locking that they're currently doing, and it's very likely that by the time I'm done we'll end up happy with the hits/misses/evictions counts anyway without having to do extra work.

As far as I'm concerned, I think you can drop a lampshade comment on it in the test that you're adding to capture that the test is covering the behavior that currently exists, and not the behavior that we would prefer to have.

[elasticsearchmachine]

Pinging @elastic/es-security (Team:Security)