storage: don't enable non-strict retention by default #16077
Conversation
dotnwat
force-pushed
the
default-sm-enable
branch
from
c71d746
to
b837252
Compare
| # in 23.2 space management exists, but is disabled by default for | ||
| # upgraded clusters. | ||
| self._upgrade(wipe_cache, self.intermediate_version) | ||
| self._check_value_everywhere("space_management_enable", False) | ||
| self._check_value_everywhere("retention_local_strict", True) | ||
|
|
There was a problem hiding this comment.
What about clusters that started on 23.2, where retention_local_strict is False because that's the default? We're intentionally flipping that off -- is that right?
There was a problem hiding this comment.
Are you asking whether retention_local_string stays False after a 23.2 -> 23.3 upgrade in this case?
There was a problem hiding this comment.
i think the issue here is two fold. first is that this keeps strict retention enabled for new installations, which isn't what we want. maybe it's just one fold
There was a problem hiding this comment.
Kind of, I'm pointing out that the default in 23.2 is False, and it isn't quite captured by the commit message that this is an intentional flip in behavior in upgrade.
I chatted with Noah a bit, and it seems like this blanket setting of retention_local_strict=True isn't what we want, and we should perhaps revert the removal of the legacy default decorator. Otherwise, it's a bit convoluted to describe the upgrade behavior of this config with this change as is.
new 23.3 cluster (strict=True): I think the desired behavior is actually strict=False
23.2 -> 23.3 (strict=False -> strict=True): I don't think this is what we want either. It should leave things as strict=False moving forward
23.1 -> 23.2 -> 23.3 (strict=True -> strict=True -> strict=True): this is okay
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
dotnwat
force-pushed
the
default-sm-enable
branch
from
b837252
to
a35990f
Compare
|
/backport v23.3.x |
In #15996 a change was made that enabled space management and non-strict retention mode when upgrading (unlike v23.1 -> v23.2 upgrade in which the features were disabled by default). It also maintained the behavior that new clusters would have the features enabled (the same behavior as a new v23.2 cluster). Only part of this behavior is desired, so this PR reverts the behavior for strict retention setting.
A new 23.3 cluster will have the same behavior as a new 23.2 cluster
An upgrade sequence 23.1, 23.2, 23.2 will have
NOTE: this requires the same workaround that we deployed for v23.2 to make upgrade options sticky. That is,
.retention_local_strict_overrideneeds to be set in upgraded clusters for the option to be sticky.Backports Required
Release Notes
Improvements