Enforce data_frozen for partial searchable snapshot _tier_preference #71155
Conversation
We already set `data_frozen` for partial searchable snapshots when they are mounted (elastic#70786), and automatically convert values other than the frozen role automatically for these snapshots when the metadata is loaded (elastic#71014). This commit makes the `_tier_preference` setting validate that the setting is *only* `data_frozen` when set on a partial searchable snapshot index.
dakrone
added
:Distributed Coordination/Snapshot/Restore
elasticmachine
added
the
Team:Distributed (Obsolete)
|
Pinging @elastic/es-distributed (Team:Distributed) |
|
@elasticmachine update branch |
henningandersen
left a comment
henningandersen
left a comment
There was a problem hiding this comment.
I would like to strengthen it to require _tier_preference=data_frozen for frozen indices (aka partial searchable snapshots).
| String[] split = value.split(","); | ||
| if (Arrays.stream(split).anyMatch(DATA_FROZEN::equals)) { | ||
| throw new IllegalArgumentException("[" + DATA_FROZEN + "] tier can only be used for partial searchable snapshots"); | ||
| if (Strings.hasText(value)) { |
There was a problem hiding this comment.
I think we need to either make _tier_preference validate that partial searchable snapshots always have the value data_frozen (i.e., must be set) or else make that the default when unset.
Ideally, we should remove support for include/exclude/require now that we have the chance too? Can be a separate PR if that is easier.
There was a problem hiding this comment.
Ideally, we should remove support for include/exclude/require now that we have the chance too? Can be a separate PR if that is easier.
Are you talking about include/exclude/require for _tier, or all include/exclude/require parameters? If you mean only for tier, I think I'd prefer to work out how we want to treat them for regular indices first (since we talked about removing _tier in favor of _tier_preference for all indices) before special casing them for only frozen indices
There was a problem hiding this comment.
I was only referring to include/exclude/require for _tier. We can tackle it outside this PR. But it feels odd that the only legal exclude value is data_frozen for frozen indices, since that is the last thing you want. When you modify _tier_preference validation to require data_frozen, the validation have to be different for include/exclude/require vs _tier_preference anyway, which is why I was suggesting to tackle this now. But perfectly fine to tackle later too.
There was a problem hiding this comment.
I think we need to either make _tier_preference validate that partial searchable snapshots always have the value data_frozen (i.e., must be set) or else make that the default when unset.
I pushed 9bab290 for this
There was a problem hiding this comment.
I wonder if there is still a loophole of setting _tier_preference to either "" (empty) or " " (white-space), since then the default value will not apply and we end up with a frozen shard with no _tier_preference? I think we need to just check for null here to fix that, at least for frozen.
There was a problem hiding this comment.
Yep, I changed this to be a null comparison
…lastic#71155) We already set `data_frozen` for partial searchable snapshots when they are mounted (elastic#70786), and automatically convert values other than the frozen role automatically for these snapshots when the metadata is loaded (elastic#71014). This commit makes the `_tier_preference` setting validate that the setting is *only* `data_frozen` when set on a partial searchable snapshot index.
…rence (#71155) (#71342) * Enforce data_frozen for partial searchable snapshot _tier_preference (#71155) We already set `data_frozen` for partial searchable snapshots when they are mounted (#70786), and automatically convert values other than the frozen role automatically for these snapshots when the metadata is loaded (#71014). This commit makes the `_tier_preference` setting validate that the setting is *only* `data_frozen` when set on a partial searchable snapshot index. * Handle mixed version clusters with pre-7.13.0 frozen SBIs * Fix checkstyyyyyyyyle * Pass index version in settings for test * Unconditionally return only data_frozen for setting Co-authored-by: Elastic Machine <[email protected]>
5 participants
We already set
data_frozenfor partial searchable snapshots when they are mounted (#70786), andautomatically convert values other than the frozen role automatically for these snapshots when the
metadata is loaded (#71014). This commit makes the
_tier_preferencesetting validate that thesetting is only
data_frozenwhen set on a partial searchable snapshot index.