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

[Fleet] Fix upgrade with a large number of stream backing indices #201272

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Nov 21, 2024

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

@nchaulet nchaulet added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) labels Nov 21, 2024
@nchaulet nchaulet self-assigned this Nov 21, 2024
@nchaulet nchaulet requested a review from a team as a code owner November 21, 2024 19:11
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

Comment on lines 1022 to 1023
const existingDs = await esClient.indices.get({
index: dataStreamName,
index: dataStream?.data_streams?.[0]?.indices?.at(-1)?.index_name ?? dataStreamName,
Copy link
Contributor

@jen-huang jen-huang Nov 21, 2024

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?

Copy link
Member Author

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.

@nchaulet nchaulet requested a review from jen-huang November 22, 2024 14:32
@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@nchaulet nchaulet requested a review from a team November 25, 2024 16:06
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

cc @nchaulet

Copy link
Contributor

@jen-huang jen-huang left a 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!

@nchaulet nchaulet merged commit 97318c9 into elastic:main Nov 26, 2024
26 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.17, 8.x

https://github.com/elastic/kibana/actions/runs/12032787776

@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.17
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Nov 26, 2024
…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]>
kibanamachine added a commit that referenced this pull request Nov 26, 2024
…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]>
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.17.0 v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants