-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Dlm add auto rollover condition max age #94950
Dlm add auto rollover condition max age #94950
Conversation
@elasticmachine run elasticsearch-ci/part-1 |
@elasticmachine update branch |
Hi @gmarouli, I've created a changelog YAML for you. |
Pinging @elastic/es-data-management (Team:Data Management) |
server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverConfiguration.java
Show resolved
Hide resolved
@andreidan Apologies, I broke something, I will fix it. Update: Fixed |
Merging with main caused some issue, I am resolving them locally and I will update this branch asap. |
Updated new endpoints to use |
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 working on this Mary. This looks great.
Left some minor comments
server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverConfiguration.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverConfiguration.java
Show resolved
Hide resolved
modules/dlm/src/main/java/org/elasticsearch/dlm/DataLifecycleService.java
Outdated
Show resolved
Hide resolved
modules/dlm/src/main/java/org/elasticsearch/dlm/DataLifecycleService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/metadata/DataLifecycle.java
Outdated
Show resolved
Hide resolved
@andreidan thank you for the great feedback, you caught some things I missed but also gave some tips to make things fall into place! |
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 iterating on this Mary
I think this is nearly ready, left one last round of suggestions
server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverConfiguration.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverConfiguration.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverConfiguration.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/action/admin/indices/rollover/Condition.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverConfiguration.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/coordination/ElasticsearchNodeCommand.java
Outdated
Show resolved
Hide resolved
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 iterating on this Mary
🚀
Thanks for the review @andreidan !! 🚀 shipping! |
@elasticmachine update branch |
@elasticmachine update branch |
@elasticmachine update branch |
@elasticmachine update branch |
What are we trying to do:
In DLM the rollover is going to be configured cluster wide via a setting. For most conditions we feel confident that this is enough but we have our concerns about the value of
max_age
. We believe that it might be better to choose amax_age
that depends on the data retention the user has requested for a specific data stream, the smaller the retention time the more fine-grained the indices.How are we planning to do it
For this reason, we are introducing an option to configure the
max_age
withauto
.The default rollover configuration will be:
cluster.dlm.default.rollover: max_primary_shard_size=50gb,max_age=auto,max_docs=200000000,min_docs=1
When
max_age
isauto
we’ll use the following retention dependent heuristics to compute the value we’ll use for the rollover operation:Implementation Details
In order to design the proposed solution we made some assumptions:
RolloverRequest
should have the conditions fully resolved, this way the "automatic" logic can be determined by the DLM and possibly ILM in the future.To facilitate the above we introduce a wrapper of the
RolloverConditions
theRolloverConfiguration
. This class has a method that accepts the data retention as an argument and resolved the configuration toRolloverConditions
.When there is no
auto
condition, then theRolloverConfiguration
contains only an instance ofRolloverConditions
so it just returns that. If themax_age
is configured asauto
, thenmax_age
is kept as an automatic property and when we request theRolloverConditions
the already calculated conditions are enriched with the calculated value and returned to the caller. Furthermore:RolloverConfiguration
.max_age: auto
when the data retention is not available andmax_age: 7d [automatic]
when it is available.Part of: #93596