[Index Management] Optimize ES response for indices list#123629
[Index Management] Optimize ES response for indices list#123629yuliacech merged 1 commit intoelastic:mainfrom
Conversation
yuliacech
added
bug
|
Pinging @elastic/platform-deployment-management (Team:Deployment Management) |
|
@yuliacech Does this impact the behavior of opening the flyout for an index? I created #106041 to propose similar behavior to what you've implemented, but in that issue I note that we'd need to load the mappings and other information for an index opened in the flyout. |
alisonelizabeth
left a comment
alisonelizabeth
left a comment
There was a problem hiding this comment.
Thanks for working on this @yuliacech! Changes LGTM. Verified table and flyout still render as expected.
I didn't do an in-depth look at the current test coverage - are there any additional tests we should add to ensure there are no regressions, or do we already have adequate coverage for the table/flyout?
@yuliacech Does this impact the behavior of opening the flyout for an index? I created #106041 to propose similar behavior to what you've implemented, but in that issue I note that we'd need to load the mappings and other information for an index opened in the flyout.
It looks like there are separate requests made when a user clicks on the following tabs in the flyout: "Settings", "Mappings", "Stats", and "Edit settings" so this change should not impact the flyout.
I did notice the "Docs Deleted" and "Primary Storage Size" details are missing on the "Summary" tab in the flyout. From what I can tell, this is a pre-existing bug. I don't see either property in the response. I tried deleting a doc from an index and it was still missing. I can open up a separate issue once you confirm this is not a regression. Thanks!
💚 Build Succeeded
Metrics [docs]
To update your PR or re-run it, just comment with: |
|
Thank you for the review, @alisonelizabeth! And great catch with the flyout values, I opened this issue to track that. It seems like it's not a regression but an existing bug on main. |
yuliacech
added
the
auto-backport
|
The following labels were identified as gaps in your version labels and will be added automatically:
If any of these should not be on your pull request, please manually remove them. |
💔 All backports failed
How to fixRe-run the backport manually: Questions ?Please refer to the Backport tool documentation |
…rom ES (elastic#123629) (cherry picked from commit f5e66d6) # Conflicts: # x-pack/plugins/index_management/server/lib/fetch_indices.ts
…rom ES (elastic#123629) (cherry picked from commit f5e66d6) # Conflicts: # x-pack/plugins/index_management/server/lib/fetch_indices.ts
| const { body: indices } = await client.asCurrentUser.indices.get({ | ||
| index: indexNamesString, | ||
| expand_wildcards: ['hidden', 'all'], | ||
| // only get specified properties in the response |
There was a problem hiding this comment.
@yuliacech Could you add some more details to this comment to make a note that we want to avoid bloating this response, and that if we include mappings we risk exceeding the 500MB (or whatever it is) limit on Node payloads? The goal we want to achieve is to ensure someone making changes to this code doesn't accidentally reintroduce this bug.
6 participants
Fixes #106041
Summary
When loading all indices from ES for Index Management page, the request
GET *?expand_wildcards=hidden,allincludes all indices of the cluster with their settings and mappings. The response to this request can surpass the limit of 536MB and that would crash Kibana server.This PR updates the request used in Index Management to
GET *?expand_wildcards=hidden,all&filter_path=*.aliases,*.settings.index.hidden,*.data_streamthat would only return specific index properties from the ES.This change reduces the risk of Index Management requests to reach the limit of 536MB.
How to test
Check if Index Management is loading and there are no regressions in the indices table.