Enable soft-deletes by default for 7.0+ indices #38929
Conversation
dnhatn
added
>enhancement
v7.0.0
:Distributed Indexing/Distributed
|
Pinging @elastic/es-distributed |
jasontedor
left a comment
jasontedor
left a comment
There was a problem hiding this comment.
LGTM. Left a nit. Thanks for picking this up.
server/src/test/java/org/elasticsearch/index/IndexSettingsTests.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/index/IndexSettingsTests.java
Outdated
Show resolved
Hide resolved
ywelsch
left a comment
ywelsch
left a comment
There was a problem hiding this comment.
IndexSettings defines softDeleteEnabled using softDeleteEnabled = version.onOrAfter(Version.V_6_5_0) && scopedSettings.get(INDEX_SOFT_DELETES_SETTING);. Can you move that version check to the same place here?
Can you also rewrite the soft-deletes check in TransportPutFollowAction.createFollowerIndex to something like
if (IndexSettings.INDEX_SOFT_DELETES_SETTING.get(leaderIndexMetaData.getSettings()) == false) {
and check all usages of the soft-deletes setting (this seems to be a bit of a mess, there's another one in AutoFollowCoordinator as well). Thank you.
| public static final Setting<Boolean> INDEX_SOFT_DELETES_SETTING = | ||
| Setting.boolSetting("index.soft_deletes.enabled", true, Property.IndexScope, Property.Final); | ||
| public static final Setting<Boolean> INDEX_SOFT_DELETES_SETTING = Setting.boolSetting("index.soft_deletes.enabled", | ||
| settings -> Boolean.toString(IndexMetaData.SETTING_INDEX_VERSION_CREATED.get(settings).onOrAfter(Version.V_7_0_0)), |
There was a problem hiding this comment.
I wonder if all unit tests are setting this correctly. If the setting is absent, we could perhaps assume that soft-deletes are enabled, instead of disabled.
There was a problem hiding this comment.
if we create IndexMetadata without this setting I think it will barf. so I think we are good.
|
@ywelsch Thanks for looking.
This requires a follow-up because we don't pass the Settings instance while parsing the value. However, I will add a Setting Validator to ensure that only 6.5+ indices have this setting true.
and check all usages of the soft-deletes setting (this seems to be a bit of a mess, there's another one in AutoFollowCoordinator as well) Sorry, this was on my check-list when I was working on the PR, but somehow I missed it.
I will use a Setting Validator to ensure that the Can you please have another look? |
|
@jasontedor I am trying to add a Setting Validator to the soft-deletes setting. The soft-deletes validation depends on the index_created_version, but a Validator requires the setting and its dependencies have the same type. Do you have any suggestion to lift this restriction? |
| public static final Setting<Boolean> INDEX_SOFT_DELETES_SETTING = | ||
| Setting.boolSetting("index.soft_deletes.enabled", true, Property.IndexScope, Property.Final); | ||
| public static final Setting<Boolean> INDEX_SOFT_DELETES_SETTING = Setting.boolSetting("index.soft_deletes.enabled", | ||
| settings -> Boolean.toString(IndexMetaData.SETTING_INDEX_VERSION_CREATED.get(settings).onOrAfter(Version.V_7_0_0)), |
There was a problem hiding this comment.
if we create IndexMetadata without this setting I think it will barf. so I think we are good.
|
@jasontedor @ywelsch @s1monw Thanks for reviewing! |
Today when users upgrade to 7.0, existing indices will automatically switch to soft-deletes without an opt-out option. With this change, we only enable soft-deletes by default for new indices. Relates #36141
Today when users upgrade to 7.0, existing indices will automatically switch to soft-deletes without an opt-out option. With this change, we only enable soft-deletes by default for new indices. Relates #36141
6 participants
Today when users upgrade to 7.0, existing indices will automatically switch to soft-deletes without an opt-out option. With this change, we only enable soft-deletes by default for new indices.
Relates #36141