-
Notifications
You must be signed in to change notification settings - Fork 593
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
cluster/config_manager: ensure cleanup of old aliases #15815
cluster/config_manager: ensure cleanup of old aliases #15815
Conversation
fix: if storing the proper name, the cleanup would be skipped
afea16c
to
354cfd8
Compare
/dt |
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
# NOTE due to https://github.com/redpanda-data/redpanda/issues/13432 , | ||
# test_values can't be -1 (a valid value nonetheless to signal infinite value) |
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.
Is this the right spot for this comment?
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.
Right it makes more sense before log_retention_ms. Will do in a followup
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.
SGTM
/backport v23.3.x |
Config aliases:
Part of the fix for #15263 requires ensuring that a cluster property is saved only once in the config_manager::_raw_values map.
When storing a property, the name is converted to the main property name and any aliases are removed from the map, in case the node inherits the snapshot from an old version.
the fix make sure that the cleanup part is always performed, before this it would be skipped the incoming key was already the main name
Backports Required
Release Notes