-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Fleet] Fix upgrade with a large number of stream backing indices #201272
[Fleet] Fix upgrade with a large number of stream backing indices #201272
Conversation
Pinging @elastic/fleet (Team:Fleet) |
const existingDs = await esClient.indices.get({ | ||
index: dataStreamName, | ||
index: dataStream?.data_streams?.[0]?.indices?.at(-1)?.index_name ?? dataStreamName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the difference between using get data stream vs get indices?
could we pass in features
and include_defaults
query params to the get indices API instead if the goal is the minify the response?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to do a call to get indices to get the current settings but only for the current write index, not all the stream backing indices.
I found that we already call get datastream before in the code, so with a little refactor I was able to remove that code let me know what do you think of it.
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]
History
cc @nchaulet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes!
Starting backport for target branches: 8.17, 8.x |
…astic#201272) (cherry picked from commit 97318c9)
…astic#201272) (cherry picked from commit 97318c9)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…ces (#201272) (#201800) # Backport This will backport the following commits from `main` to `8.17`: - [[Fleet] Fix upgrade with a large number of stream backing indices (#201272)](#201272) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Nicolas Chaulet","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-26T14:32:13Z","message":"[Fleet] Fix upgrade with a large number of stream backing indices (#201272)","sha":"97318c9f537bd6e9fecb35e40250aa14df325b81","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:Fleet","v9.0.0","backport:prev-minor","v8.17.0"],"title":"[Fleet] Fix upgrade with a large number of stream backing indices","number":201272,"url":"https://github.com/elastic/kibana/pull/201272","mergeCommit":{"message":"[Fleet] Fix upgrade with a large number of stream backing indices (#201272)","sha":"97318c9f537bd6e9fecb35e40250aa14df325b81"}},"sourceBranch":"main","suggestedTargetBranches":["8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/201272","number":201272,"mergeCommit":{"message":"[Fleet] Fix upgrade with a large number of stream backing indices (#201272)","sha":"97318c9f537bd6e9fecb35e40250aa14df325b81"}},{"branch":"8.17","label":"v8.17.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Nicolas Chaulet <[email protected]>
…es (#201272) (#201801) # Backport This will backport the following commits from `main` to `8.x`: - [[Fleet] Fix upgrade with a large number of stream backing indices (#201272)](#201272) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Nicolas Chaulet","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-26T14:32:13Z","message":"[Fleet] Fix upgrade with a large number of stream backing indices (#201272)","sha":"97318c9f537bd6e9fecb35e40250aa14df325b81","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:Fleet","v9.0.0","backport:prev-minor","v8.17.0"],"title":"[Fleet] Fix upgrade with a large number of stream backing indices","number":201272,"url":"https://github.com/elastic/kibana/pull/201272","mergeCommit":{"message":"[Fleet] Fix upgrade with a large number of stream backing indices (#201272)","sha":"97318c9f537bd6e9fecb35e40250aa14df325b81"}},"sourceBranch":"main","suggestedTargetBranches":["8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/201272","number":201272,"mergeCommit":{"message":"[Fleet] Fix upgrade with a large number of stream backing indices (#201272)","sha":"97318c9f537bd6e9fecb35e40250aa14df325b81"}},{"branch":"8.17","label":"v8.17.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Nicolas Chaulet <[email protected]>
Summary
It seems that upgrading a integration stream with a lot of backing indices could sometime fail because the response is too large.
We should avoid retrieving all backing indices (settings, and mappings) and only return the current one for the write index.
That PR does that.
Test
I added a unit test to verify we only call
GET /<indices>
with the current write index and not the datastream name