Kibana: Do not overwhelm Elasticsearch with UiSettings requests

[restrry]

@gingerwizard analysed APM data and noticed the high number of _security/user/has_priveleges requests to Elasticsearch. Some of them as caused by the Platform code reading uiSettings See _doc/config:

This problem is caused by the current implementation of the UiSettings client. Every get('setting_name') invocation performs a request to Elasticsearch service. It becomes a bottleneck since plugins might call it several times in a row. We can see that for given APM data, vis_type_timeseries plugin calls UiSettingsClient.get method thrice:
https://github.com/elastic/kibana/blob/73fbadb85149d97cddd75da4fb1ab69190c3f1bc/src/plugins/vis_type_timeseries/server/lib/vis_data/helpers/get_es_query_uisettings.js#L24-L28
data plugin calls UiSettingsClient.get method twice:
https://github.com/elastic/kibana/blob/73fbadb85149d97cddd75da4fb1ab69190c3f1bc/src/plugins/data/server/search/es_search/get_default_search_params.ts#L33-L34

I can see a couple of options with different pros & cons:

• change API to allow to retrieve multiple settings at once. client.get(['key:a', 'key:b']) but this approach isn't well scalable because it requires a manual audit.
• implement a caching mechanism in the UiSettings client. Even though UiSettings client lifetime is limited by an incoming request lifetime and UiSettings client controls all write operations, it might be that cache isn't invalidated if uiSettings write request is handled by another Kibana instance.

A side note: Every page load performs three requests: 2 when fetching /bootstrap.js and one in ui_render_mixin. I expect we address this problem in https://github.com/elastic/kibana/issues/54375#issuecomment-703624873

[elasticmachine]

Pinging @elastic/kibana-platform (Team:Platform)

[pgayvallet]

change API to allow to retrieve multiple settings at once

Seems reasonable to at least expose this method and document that it should be favored when retrieving multiple settings. This should imho be done regardless of our decision for the next option.

implement a caching mechanism in the UiSettings client. Even though UiSettings client lifetime is limited by an incoming request lifetime and UiSettings client controls all write operations, it might be that cache isn't invalidated if uiSettings write request is handled by another Kibana instance.

Yea, that seems kinda dangerous to me for that exact reason. OTOH I'm unsure of the need to have 'real time' / 'fresh' access to UISettings. Would it be acceptable to have a per-instance cache with a very short lifespan (like, 60 / 120 seconds or so)? If it does, it could probably be a significant perf improvement.

[legrego]

implement a caching mechanism in the UiSettings client. Even though UiSettings client lifetime is limited by an incoming request lifetime and UiSettings client controls all write operations, it might be that cache isn't invalidated if uiSettings write request is handled by another Kibana instance.

Yea, that seems kinda dangerous to me for that exact reason. OTOH I'm unsure of the need to have 'real time' / 'fresh' access to UISettings. Would it be acceptable to have a per-instance cache with a very short lifespan (like, 60 / 120 seconds or so)? If it does, it could probably be a significant perf improvement.

A per-request cache doesn't feel all that dangerous to me, at least for a vast majority of requests. Having a cache timeout makes sense for the longer running requests, like those that we'd see with Fleet.

[joshdover]

A per-request cache doesn't feel all that dangerous to me, at least for a vast majority of requests. Having a cache timeout makes sense for the longer running requests, like those that we'd see with Fleet.

+1 to this. Most requests are short-lived and I suspect there would be very few problems created by caching the config document from the first access during a request. I also suspect this could get us the quickest performance wins.

[pgayvallet]

Most requests are short-lived and I suspect there would be very few problems created by caching the config document from the first access during a request.

I'm good with that. Note that it will require some IUiSettingsClient internal changes, as it is currently bound to a SavedObjectClient, not directly to a request:

https://github.com/elastic/kibana/blob/bc8a1dac99af3ea16bbcf5f2825d0f90724fa3b3/src/core/server/core_route_handler_context.ts#L92-L94

(uiSettingsClient can also be created from outside of the scope of a request, using UiSettingsServiceStart.asScopedToClient from a plugin's start lifecycle)

I think we will need to add a new UiSettingsServiceStart.asScoped(request) in addition to the current UiSettingsServiceStart.asScopedToClient to to be able to dissociate a 'transient' uiSettings client created for a request's lifespan from a 'persistent' client that may be created to be used from within server-side services.

Follow-up question: we don't plan to optimize / cache the config document for 'persistent' ui settings client then?