cloud_topics: gate tiered_cloud on full upgrade and add tiered_v1/tiered_v2 storage modes#30966
cloud_topics: gate tiered_cloud on full upgrade and add tiered_v1/tiered_v2 storage modes#30966Lazin wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new cluster feature gate to prevent a 26.2+ leader from replicating the new cloud-topics compaction set_min_allowed_local_threshold (MASH floor) command to pre-26.2 replicas during rolling upgrades, avoiding “unknown command key” failures on older nodes.
Changes:
- Adds the
ctp_min_allowed_local_thresholdcluster feature (gated on the 26.2 logical version) and wires it intolevel_zero_notifierto short-circuit replication until the cluster is fully upgraded. - Introduces
ctp_stm_api_errc::feature_disabledand threads it through relevant call sites (notifier, frontend, compaction sink) with appropriate retry/logging behavior. - Adds/updates unit and rptest coverage and Bazel dependencies to validate the feature gate behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/rptest/tests/cluster_features_test.py | Excludes the new feature from an upgrade-status test that doesn’t exercise feature-specific setups. |
| src/v/features/feature_table.h | Registers the new ctp_min_allowed_local_threshold feature and schema entry. |
| src/v/features/feature_table.cc | Adds string conversion for the new feature. |
| src/v/cloud_topics/reconciler/reconciliation_source.cc | Handles the new feature_disabled error code in a switch. |
| src/v/cloud_topics/level_zero/stm/ctp_stm_api.h | Adds ctp_stm_api_errc::feature_disabled and formatting. |
| src/v/cloud_topics/level_zero/notifier/tests/level_zero_notifier_test.cc | Adds a unit test validating notifier gating when the feature is inactive. |
| src/v/cloud_topics/level_zero/notifier/tests/BUILD | Adds Bazel dependency on //src/v/features for the new test. |
| src/v/cloud_topics/level_zero/notifier/level_zero_notifier.h | Extends notifier wiring to accept a feature_table pointer and adds a gate helper. |
| src/v/cloud_topics/level_zero/notifier/level_zero_notifier.cc | Implements feature gate checks and returns feature_disabled when inactive. |
| src/v/cloud_topics/level_zero/notifier/BUILD | Adds Bazel dependency on //src/v/features for notifier feature checks. |
| src/v/cloud_topics/level_one/maintenance/compaction/compaction_sink.cc | Downgrades log severity for expected feature_disabled during upgrades. |
| src/v/cloud_topics/frontend/frontend.cc | Treats feature_disabled like other transient errors in error mapping. |
| src/v/cloud_topics/app.cc | Wires controller feature table into the level-zero notifier construction. |
| case ctp_stm_api_errc::failure: | ||
| case ctp_stm_api_errc::feature_disabled: | ||
| co_return std::unexpected(errc::failure); |
There was a problem hiding this comment.
^^^ makes sense to me, but I'm not sure how reconciler treats failure vs timeout differently.
There was a problem hiding this comment.
the reconciler never replicates the command that can trigger this error
|
force push: rebase with dev |
oleiman
left a comment
There was a problem hiding this comment.
lgtm modulo open question about how the reconciler should treat the error
| case ctp_stm_api_errc::failure: | ||
| case ctp_stm_api_errc::feature_disabled: | ||
| co_return std::unexpected(errc::failure); |
There was a problem hiding this comment.
^^^ makes sense to me, but I'm not sure how reconciler treats failure vs timeout differently.
| shadow_link_sr_api_sync = 1ULL << 16U, | ||
| iceberg_extended_mode_config = 1ULL << 17U, | ||
| fetch_controller_snapshot_rpc = 1ULL << 18U, | ||
| ctp_min_allowed_local_threshold = 1ULL << 21U, |
There was a problem hiding this comment.
nitpick: that's a weird place to put it
| # Gates replication of the cloud-topics compaction min-allowed-local- | ||
| # threshold command; exercising it needs cloud-topics compaction setup | ||
| # orthogonal to the finalization behavior under test. | ||
| "ctp_min_allowed_local_threshold", |
There was a problem hiding this comment.
Need to either (1) introduce testing for this feature in the context of unfinalized upgrade or (2) explain here why it isn't needed.
|
How come that it's closed? |
|
Reopening |
Retry command for Build#86734please wait until all jobs are finished before running the slash command |
Retry command for Build#86759please wait until all jobs are finished before running the slash command |
| return redpanda_storage_mode_from_string(s); | ||
| } | ||
|
|
||
| const char* redpanda_storage_mode_user_name( |
There was a problem hiding this comment.
so am I to undestand that we are still exposing tiered_v1/tiered_v2 details to users when the configured mode doesn't match the cluster default? Was this determined to be acceptable w/r/t the product ask?
316abe4 to
45993c3
Compare
Switch the flag from explicit_only to always so it activates as soon as every node runs v26.2, with no admin action. The create and alter-config gates keep rejecting cloud-architecture tiered topics (and cloud -> tiered_cloud conversions) while the cluster is only partially upgraded, and open automatically once the upgrade completes. set_feature_active now tolerates always-policy features jumping straight from unavailable to active without an observable available state, and the real-upgrade finalization test expects the flag to auto-activate after finalize. Signed-off-by: Evgeny Lazin <4lazin@gmail.com>
The set_min_allowed_local_threshold stm command is new in v26.2, so the compaction sink must not replicate it while pre-v26.2 replicas may still be in the raft group. Gate the notifier on the tiered_cloud_topics feature flag: until it activates (the cluster is fully upgraded to v26.2) every notification attempt reports success without replicating. This is safe because no cloud-architecture tiered topic -- the only reader of local data below the floor -- can exist while the flag is inactive. Signed-off-by: Evgeny Lazin <4lazin@gmail.com>
0f44051 to
39ff8f6
Compare
Retry command for Build#86893please wait until all jobs are finished before running the slash command |
The tiered_cloud storage mode ships in v26.2 as the second implementation of tiered storage, so the user surface is reworked around the 'tiered' name instead of exposing the internal enum: - redpanda.storage.mode accepts unset/local/tiered/cloud; the 'tiered_cloud' spelling is no longer a valid value. Both tiered variants display as 'tiered' in DescribeConfigs. - The new default_redpanda_storage_mode_tiered_impl cluster config (tiered_v1 = classic Tiered Storage, tiered_v2 = Cloud Topics based) selects which implementation 'tiered' refers to. It defaults to tiered_v1 for all clusters, so an upgrade never changes what 'tiered' means and tiered_v2 is an explicit opt-in. - The new read-only redpanda.storage.mode.impl topic property reports the exact implementation of every topic's storage mode (unset/local/tiered_v1/tiered_v2/cloud), making the describe output unambiguous regardless of the reader's cluster config. On creation it selects the storage mode precisely, alone or together with a matching redpanda.storage.mode; mismatched pairs are rejected. The property is not alterable, although sets that match the current value are tolerated as no-ops so that describe-then-alter round trips (which replay every config) keep working. It is reported with the default config source: the value is derived from the storage mode rather than being an explicit override, so config backup/replay tooling must not treat it as user-set. Storage-mode transition errors print the impl names, so a rejected tiered_v1 to tiered_v2 transition does not read as 'tiered to tiered'. The tiered_v2 impl keeps the tiered_cloud_topics feature and cloud storage gates. Signed-off-by: Evgeny Lazin <4lazin@gmail.com>
The synced describe output shows 'tiered' for both tiered impls, so the mode string alone is ambiguous. Request the read-only redpanda.storage.mode.impl property alongside the mode and apply it first (it is exact and reported for every topic), reconstructing the source topic's storage mode on the target instead of re-interpreting 'tiered' through the target's own tiered-impl config - with different configs on the two clusters that could silently turn a cloud-architecture topic into a classic tiered one and start an archiver on a partition that has no archival stm. The mode string remains as a fallback for sources that predate the impl property. Mirror-topic creation and property updates bypass the kafka handlers, so this path enforces the guards itself: storage-mode transition rules (a bad sync becomes a rejected update instead of an applied forbidden transition) and the tiered_cloud_topics upgrade gate - no mirror topic is created with or moved to the tiered_v2 impl until the cluster is fully upgraded to v26.2; creation is skipped and retried once the feature activates. Signed-off-by: Evgeny Lazin <4lazin@gmail.com>
TieredCloudUpgradeTest checks the mixed-version behavior: creating a tiered_v2 topic or converting a cloud topic to it is rejected (or, via a v26.1 controller that drops the unknown impl property, degrades to a classic tiered topic) while the cluster is partially upgraded, and both operations work once every node runs v26.2. StorageModeAliasMatrixTest covers every mode/impl input combination under both tiered impls, including impl-only creation, mode/impl consistency rejections, read-only enforcement and the always-present describe entry. ShadowLinkStorageModeSyncTest syncs cloud and tiered topics across all four combinations of source/target tiered impls, including a mid-flight cloud to tiered_v2 conversion. TopicSpec spells tiered variants through the new storage_mode_config helper (mode=tiered plus the impl property, which keeps v26.1 brokers on a tiered mode instead of silently falling back to the cluster default), kafka-cli describe output round-trips through the new impl kwarg, and the describe-topics parser accepts the asterisk kcl appends to read-only properties. The storage-mode toggle and rnot flipping tests point the 'tiered' alias at tiered_v2 explicitly, since the alter path resolves it through the cluster config. Signed-off-by: Evgeny Lazin <4lazin@gmail.com>
This PR prepares the
tiered_cloudstorage mode (Cloud Topics based tieredstorage) for the 26.2 release: it makes the mode safe during rolling
upgrades and introduces the user-facing
tiered_v1/tiered_v2vocabularybuilt around a new read-only
redpanda.storage.mode.impltopic property.Upgrade gating. The
tiered_cloud_topicsfeature flag switches fromexplicit_onlytoalways, so it activates exactly when every broker runsv26.2. While the cluster is only partially upgraded, creating a tiered_v2
topic or converting a
cloudtopic to it is rejected; once the upgradecompletes both operations work with no admin action. The same flag gates
the L0 notifier: the post-compaction
set_min_allowed_local_thresholdstmcommand is new in 26.2 and older replicas cannot apply it, so until the
flag activates every notification attempt is a no-op that reports success.
This is safe because no tiered_v2 topic — the only reader of local data
below the floor — can exist while the flag is inactive, so the compaction
sink can commit normally.
Storage mode vocabulary.
redpanda.storage.modeacceptslocal,tiered,cloudand (legacy)unset; the internaltiered_cloudspelling and the implementation names are not valid mode values. The plain
tieredvalue resolves through the newdefault_redpanda_storage_mode_tiered_implcluster config (tiered_v1=classic Tiered Storage,
tiered_v2= Cloud Topics based), which defaultsto
tiered_v1for all clusters — an upgrade never changes whattieredmeans, and tiered_v2 as the meaning of
tieredis an explicit opt-in.redpanda.storage.mode.impl. A new companion topic property carries the
exact implementation of every topic's storage mode:
unambiguous vocabulary
unset|local|tiered_v1|tiered_v2|cloud; the modeitself displays both tiered variants as
tiered, so the displayed pairis never ambiguous regardless of any cluster's tiered-impl config.
together with a matching
redpanda.storage.mode(mismatched pairs arerejected).
through the
tieredalias resolved via the cluster config.Shadow linking syncs the
(mode, impl)pair and applies the impl valuefirst, reconstructing the source topic's exact storage mode on the target
instead of re-interpreting the ambiguous
tieredstring through thetarget's own cluster config — with different tiered impls on the two
clusters, a string-only sync could silently turn a Cloud Topics based
topic into a classic tiered one (and crash the target's archiver). The
link's apply path also enforces the storage-mode transition rules.
Ducktape coverage: a mixed-version upgrade test (creation/conversion
rejected mid-upgrade, allowed after full upgrade), a full matrix test of
both tiered impls over every mode/impl input combination (including
impl-only creation, consistency rejections and read-only enforcement),
and a shadow-link sync test over all four source/target tiered-impl
combinations including a mid-flight cloud → tiered_v2 conversion.
Backports Required
Release Notes
Features
default_redpanda_storage_mode_tiered_implcluster config(
tiered_v1|tiered_v2, defaulttiered_v1) selects whether thetieredvalue of theredpanda.storage.modetopic property refers toclassic Tiered Storage or Cloud Topics based tiered storage.
redpanda.storage.mode.impltopic property reports theexact implementation of every topic's storage mode
(
unset|local|tiered_v1|tiered_v2|cloud) and can be supplied on topiccreation — alone or together with a matching
redpanda.storage.mode—to select the storage mode precisely. Topics using the
tiered_v2implementation (or conversions from
cloud) can only be created oncethe whole cluster runs v26.2.