Remove DocumentSubsetBitsetCache locking

[joegallo]

For heavy users of Document Level Security (DLS), and where the entire DLS bitset cache cannot be held in memory at once, we can experience a high degree of cache churn. Due to the locking that keeps the DocumentSubsetBitsetCache's bitsetCache and keysByIndex in sync, we end up seeing an extreme level of lock contention when entries are evicted from the cache (as they are frequently if the cache is churning).

The purpose of the keysByIndex data structure is to allow us to proactively evict entries from the cache in the event that their associated segment becomes inaccessible (e.g. because of a segment merge, or if an index is closed or deleted).

By removing the locking around the updates to the bitsetCache and keysByIndex structures, we get significantly improved throughput, but it becomes possible (though the chances are quite small) that we will no longer have an entry in the keysByIndex structure for an index that is open and for which there is an entry in the bitsetCache. As a consequence, if that segment later becomes inaccessible, we will not proactively remove the entry from the cache. This is not a true memory leak, however, as the maximum size and TTL policies of the cache still apply, and the entry will be removed from the cache eventually.

I've created a small benchmark that indexes ~10 million documents in 8 indices and then runs a selection of searches and aggregations against the indices via 63 user accounts associated with different DLS role queries. If all the data were in the cache at the same time, it would be approximately 64mb, but the cache is limited to 48mb during the benchmark run.

On main without these changes, I see the following results from the benchmark:

|                                                Mean Throughput |           dls-search |    76.79        |  ops/s |
|                                              Median Throughput |           dls-search |    76.84        |  ops/s |
|                                        50th percentile latency |           dls-search |  1604.59        |     ms |
|                                        90th percentile latency |           dls-search |  2624.26        |     ms |
|                                        99th percentile latency |           dls-search |  3547.23        |     ms |

And with these changes (approximately 15x better throughput and latency):

|                                                Mean Throughput |           dls-search |  1169.86        |  ops/s |
|                                              Median Throughput |           dls-search |  1170.12        |  ops/s |
|                                        50th percentile latency |           dls-search |    93.6846      |     ms |
|                                        90th percentile latency |           dls-search |   167.648       |     ms |
|                                        99th percentile latency |           dls-search |   269.456       |     ms |

Because sufficiently poor performance can be characterized as a bug, I'm labeling this PR as a >bug and I intend to backport it to all the relevant branches.

Note: because I've removed the bitsetCache.get call from the onCacheEviction method, this PR happens to fix #132842.

[elasticsearchmachine]

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

[elasticsearchmachine]

Hi @joegallo, I've created a changelog YAML for you.

[elasticsearchmachine]

Hi @joegallo, I've updated the changelog YAML for you.

[tvernum]

LGTM, thanks for all the work on this.

It's amazing when a substantial investment in time leads to such an improvement simply by removing code.

[szybia]

lgtm!

[elasticsearchmachine]

💔 Backport failed

Status	Branch	Result
❌	9.1	Commit could not be cherrypicked due to conflicts
❌	9.0	Commit could not be cherrypicked due to conflicts
❌	8.18	Commit could not be cherrypicked due to conflicts
❌	8.19	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 133681

[joegallo]

#133707 will be autobackported to 8.18 and 8.19, so between that and #133705 we should be all set on the backports.