-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Deprecate indices without soft-deletes #50502
Conversation
Pinging @elastic/es-distributed (:Distributed/Distributed) |
qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/RecoveryIT.java
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.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.
LGTM
@@ -434,6 +437,11 @@ static Settings aggregateIndexSettings(ClusterState currentState, CreateIndexClu | |||
* that will be used to create this index. | |||
*/ | |||
MetaDataCreateIndexService.checkShardLimit(indexSettings, currentState); | |||
if (indexSettings.getAsBoolean(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), true) == false) { |
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.
Do we want to completely disallow setting this setting in 8.x? If so, should we always emit a warning, even if it's explicitly set to true?
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.
Users might be confused about the deprecation when creating indices for CCR in a mixed cluster between 7.x and 6.8 because they need to explicitly set the soft_deletes setting.
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.
If mixed clusters with 6.8 is a worry, you can just check here that all nodes are 7.0+.
With the approach here, we will still have to support the setting for the full duration of 8.x (i.e. allow people to explicitly set soft-deletes true).
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.
++. I will make this change in a follow-up.
@ywelsch Thanks for reviewing. |
Soft-deletes will be enabled for all indices in 8.0. Hence, we should deprecate new indices without soft-deletes in 7.x.
Soft-deletes will be enabled for all indices in 8.0. Hence, we should deprecate new indices without soft-deletes in 7.x.
Relates: #4341, elastic/elasticsearch#50502 This commit marks the enabled setting on soft delete index settings as obsolete, as setting enabled to false is deprecated.
Relates: #4341, elastic/elasticsearch#50502 This commit marks the enabled setting on soft delete index settings as obsolete, as setting enabled to false is deprecated.
Relates: #4341, elastic/elasticsearch#50502 This commit marks the enabled setting on soft delete index settings as obsolete, as setting enabled to false is deprecated.
Relates: #4341, elastic/elasticsearch#50502 This commit marks the enabled setting on soft delete index settings as obsolete, as setting enabled to false is deprecated. Co-authored-by: Russ Cam <[email protected]>
Relates: #4341, elastic/elasticsearch#50502 This commit marks the enabled setting on soft delete index settings as obsolete, as setting enabled to false is deprecated. (cherry-picked from commit d86e258)
Soft-deletes will be enabled for all indices in 8.0. Hence, we should deprecate new indices without soft-deletes in 7.x.