Cache completion stats between refreshes

[DaveCTurner]

Computing the stats for completion fields may involve a significant amount of
work since it walks every field of every segment looking for completion fields.
Innocuous-looking APIs like GET _stats or GET _cluster/stats do this for
every shard in the cluster. This repeated work is unnecessary since these stats
do not change between refreshes; in many indices they remain constant for a
long time.

This commit introduces a cache for these stats which is invalidated on a
refresh, allowing most stats calls to bypass the work needed to compute them on
most shards.

Closes #51915

[elasticmachine]

Pinging @elastic/es-distributed (:Distributed/Engine)

[jimczi]

I added a way to load the completion suggester off-heap but never had time to check the performance after the change so we're still loading the entire FST on-heap in ES. Stats shouldn't be costly to retrieve, that should be considered as a bug in the completion field. If we cannot always load the completion suggester off-heap we should probably add a fast path to get the memory consumption without really loading the FST. That's debatable though because getting the stats today loads every completion field in memory which is equivalent to running a query on every completion fields. Once the FST is loaded, the stats call should be fast so I also wonder if the cache is helpful in this case ?

[DaveCTurner]

Once the FST is loaded, the stats call should be fast so I also wonder if the cache is helpful in this case ?

I frequently see org.apache.lucene.index.FilterLeafReader.terms() in the hot threads outputs from cases like this. I am interpreting this to mean that we mostly spend time simply looking up fields to find out if they're completion fields or not, and normally they're not.

For instance:

• https://gist.github.com/johnskopis/bb3012a42f2b56304cc31d2861b6bfe0
• https://discuss.elastic.co/t/bulk-indexing-causes-management-threadpool-queue-to-skyrocket/217852/5
• https://discuss.elastic.co/t/monitor-tasks-piling-up-in-master-node-6-8-3/212547

The fact that I do not often see hot threads actually loading the FSTs does suggest that they are indeed shared between calls.

[DaveCTurner]

@elasticmachine please run elasticsearch-ci/docs

[dnhatn]

@DaveCTurner Thank you for working on this. I left a comment on the caching implementation.

As Jim said, I think we can solve the issue by loading terms of the completion fields only instead of all fields from FieldInfos. We can retrieve the list of the completion fields from the MapperService. WDYT?

List<String> completionFields = StreamSupport.stream(mapperService.fieldTypes().spliterator(), false)
    .filter(field -> field instanceof CompletionFieldMapper.CompletionFieldType)
    .map(MappedFieldType::name).collect(Collectors.toList());

[dnhatn]

Instead of invalidating the entire current cache, we can mark the current cache as outdated (i.e., need to refresh), then we can reuse the stats of some LeafReader that haven't changed between refreshes.

[DaveCTurner]

If we re-use a LeafReader across a refresh, does it keep its suggester loaded? If so, do we not already avoid most of the work of recomputing stats?

Note that we need to break the stats down by field, because the user can select the fields in the API. If I understand correctly I think re-using stats on a per-segment basis too would require tracking everything on a per-segment-per-field basis which seems unnecessary.

[henningandersen]

Great find, looks good.

I wonder if you considered checking the mappings of the index for whether it can have any completion terms? I think the caching you have is good anyway, but checking this could speed up the first stats call after a full cluster restart and maybe we could utilize it to not cache for every shard. Not really suggesting to do this in this PR and might not be worth doing it, but would like to hear your opinion on that.

[DaveCTurner]

@dnhatn, re:

We can retrieve the list of the completion fields from the MapperService.

True (and maybe that's the real answer here) but today neither Engine has access to a MapperService and that feels right to me. We could plumb it in from the IndexShard of course (yay more dependencies).

[DaveCTurner]

@elasticmachine update branch

[dnhatn]

LGTM, thanks David.

[henningandersen]

LGTM.