Skip to content

Add max_single_primary_size as a condition for the ILM rollover action #68917

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

Merged
merged 11 commits into from
Feb 18, 2021

Conversation

joegallo
Copy link
Contributor

@joegallo joegallo commented Feb 11, 2021

Related to #63026, followup to #67842

Threads the new max_single_primary_size rollover trigger from #67842 into ILM's RolloverAction.

@joegallo joegallo added >enhancement :Data Management/ILM+SLM Index and Snapshot lifecycle management v8.0.0 Team:Data Management Meta label for data/management team WIP labels Feb 11, 2021
@joegallo joegallo force-pushed the ilm-rollover-max-single-shard-size branch 2 times, most recently from f05b75a to 398d7c1 Compare February 12, 2021 00:33
@joegallo joegallo force-pushed the ilm-rollover-max-single-shard-size branch from 398d7c1 to 66b94d3 Compare February 16, 2021 23:21
@joegallo joegallo removed the WIP label Feb 17, 2021
@joegallo joegallo marked this pull request as ready for review February 17, 2021 00:20
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

@joegallo
Copy link
Contributor Author

IMHO reviewing commit by commit would be best, but that's just me.

Copy link
Contributor

@andreidan andreidan 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 working on this Joe. This generally looks great! Nicely done.

I left a few rather minor suggestions and questions.

Comment on lines 75 to +76
this.maxSize = maxSize;
this.maxSinglePrimarySize = maxSinglePrimarySize;
Copy link
Contributor

@andreidan andreidan Feb 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make sense to validate that maxSinglePrimarySize is lte to maxSize?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO no -- we don't sanity check the other things (e.g. maxDocs 1 paired with maxSize 10,000GB is just as valid as maxDocs 10,000 with maxSize 1 byte -- at least AFAICT), so I think we should be similarly lenient with maxSinglePrimarySize.

@joegallo joegallo requested a review from andreidan February 17, 2021 21:55
@joegallo joegallo added v7.12.0 and removed v7.12.0 labels Feb 17, 2021
Copy link
Contributor

@andreidan andreidan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks Joe

@joegallo
Copy link
Contributor Author

@elastic/es-ui there's a new option for ILM Rollovers (in 7.13 and 8.0.0). On this PR it was added as max_single_primary_size but we renamed it to max_primary_shard_size on #69239.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/ILM+SLM Index and Snapshot lifecycle management >enhancement Team:Data Management Meta label for data/management team Team:Deployment Management Meta label for Management Experience - Deployment Management team v7.13.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants