Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detecting hidden indices takes nontrivial time #77974

Closed
DaveCTurner opened this issue Sep 17, 2021 · 3 comments · Fixed by #78612
Closed

Detecting hidden indices takes nontrivial time #77974

DaveCTurner opened this issue Sep 17, 2021 · 3 comments · Fixed by #78612
Assignees
Labels
:Core/Infra/Core Core issues without another label >enhancement Team:Core/Infra Meta label for core/infra team

Comments

@DaveCTurner
Copy link
Contributor

Metadata$Builder#build is called on every node in every cluster state update that changes the cluster metadata. In a busy cluster with many shards, this means it's on a fairly hot path. Profiling cluster state updates in such a cluster indicates that this method spends about 23% of its time calling Setting#get:

image

The only such call is this one:

final boolean visible = IndexMetadata.INDEX_HIDDEN_SETTING.get(indexMetadata.getSettings()) == false;

Frequently-accessed settings in IndexMetadata are generally made into fields to avoid having to do the lookup work on each access. We can make a substantial time saving in this method by moving the index.hidden setting to a field.

@DaveCTurner DaveCTurner added >enhancement :Core/Infra/Core Core issues without another label labels Sep 17, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Sep 17, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@grcevski
Copy link
Contributor

Hi @DaveCTurner, is there a way to share an expanded image (or the profile itself) for the last section of the flame graph, just what's under Setting.get? I'm curious if we can simply improve the performance of Setting.get?

@DaveCTurner
Copy link
Contributor Author

Sure, here it is, although beware that there's only 27 samples in this method so there will be some quantisation error.

image

Some of that is computing the keyset for the index settings (happens once per instance, slightly puzzled why we didn't already compute it in IndexMetadata$Builder.build()) and the rest is looking up a string in a map (including hashing the setting name).

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue Oct 4, 2021
We look up the value for `index.hidden` for every index every time we
build a new `Metadata`, which involves map lookups and string parsing
and so on and takes nontrivial time. This commit moves the value to a
field so the lookups are only needed if the index metadata changes.

Closes elastic#77974
@original-brownbear original-brownbear self-assigned this Oct 4, 2021
DaveCTurner added a commit that referenced this issue Oct 4, 2021
We look up the value for `index.hidden` for every index every time we
build a new `Metadata`, which involves map lookups and string parsing
and so on and takes nontrivial time. This commit moves the value to a
field so the lookups are only needed if the index metadata changes.

Closes #77974
DaveCTurner added a commit that referenced this issue Oct 4, 2021
We look up the value for `index.hidden` for every index every time we
build a new `Metadata`, which involves map lookups and string parsing
and so on and takes nontrivial time. This commit moves the value to a
field so the lookups are only needed if the index metadata changes.

Closes #77974
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >enhancement Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants