[CORE-7998] PoC: Tiered Storage Sanctioning #23895
Conversation
BenPope
force-pushed
the
core-7998-tiered-storage-sanctioning
branch
from
bf16de9 to
64c71b8
Compare
src/v/cluster/topics_frontend.cc
Outdated
| // TODO (ben): probably best to partition the requests and only | ||
| // fail the topics that have SI enabled |
There was a problem hiding this comment.
would probably throw a log message in there as well about why
There was a problem hiding this comment.
We already get something like:
WARN 2024-10-30 21:41:54,557 [shard 0:main] kafka - config_utils.h:287 - Failed to alter topic properties of topic(s) {{kafka/full}} error_code observed: cluster::errc::feature_disabled
There was a problem hiding this comment.
+1 I think I would err on the side of being more verbose and add an extra log line. Specifically, I think this would be a good place to call out why the feature is disabled (they don't have a valid enterprise license) and how they can get out of the error (get a valid license).
Signed-off-by: Ben Pope <ben@redpanda.com>
Signed-off-by: Ben Pope <ben@redpanda.com>
Signed-off-by: Ben Pope <ben@redpanda.com>
Fixes CORE-7999 Signed-off-by: Ben Pope <ben@redpanda.com>
Fixes CORE-8000 Signed-off-by: Ben Pope <ben@redpanda.com>
BenPope
force-pushed
the
core-7998-tiered-storage-sanctioning
branch
from
64c71b8 to
39cacb2
Compare
|
Changes in force-push
|
| maybe_remove_flag(mode, update_props.remote_read, si_mode::drop_fetch); | ||
| maybe_remove_flag(mode, update_props.remote_write, si_mode::drop_archival); | ||
|
|
||
| if (mode != si_mode::disabled) { |
There was a problem hiding this comment.
We still want to allow them to alter non-enterprise topic configs (e.g. retention.ms) for their already-tiered storage-enabled topics.
| if (mode != si_mode::disabled) { | |
| if (mode != si_mode::disabled || mode == current_mode) { |
I think we should also drop 237-242 and have a similar check for remote_delete as well, where we check if it becomes enabled && it was not enabled.
There's a subtle but important behaviour difference here based on whether the tiered storage topic properties are explicitly specified or not. The case where this matters that I am thinking about is when customers manage their topic properties with terraform. As far as know, the terraform providers issue AlterTopicConfigs updates with all topic configs specified. Let's say they have an existing topic with TS enabled, their license expired yesterday, and they want to update retention.ms. With the existing code, they wouldn't be able to do that because terraform's AlterTopicsRequest would specify the enabled tiered storage topic properties explicitly as enabled and this check would fail - even though that is the existing state of the topic property and they are just trying to change a non-TS topic property. So I think we want to drop 237-242 and change all checks to be of the style becomes_enabled(X) && X != old_X.
Implement sanctions for tiered storage topic properties.
Backports Required
Release Notes