RFC: ts-ct-migration: STM coexistence [PR 2]#30887
Conversation
|
This seemed like the simpler approach. The other option would be to add infrastructure to dynamically add or remove stms. I have a branch for that I can resurrect if it there's interest. |
| // no runtime STM install). Without this a passenger would pin the local log | ||
| // at offset::min() forever. | ||
| if (!_max_applied_epoch.has_value()) { | ||
| return model::offset::max(); |
There was a problem hiding this comment.
This is not the only problem that ctp_stm creates. The ctp_stm also implements its own log eviction (same as log_eviction_stm).
There was a problem hiding this comment.
Yeah, a subsequent PR goes over behaviors like that in both the TS and CT to prevent them from operating at the same time.
| // still holds archived data (a non-empty live manifest or a spillover | ||
| // archive) it is still served from tiered storage, so keep constraining via | ||
| // cloud_recoverable_offset() until the manifest is cleared. | ||
| collect_all = collect_all && !holds_archived_data(); |
There was a problem hiding this comment.
is_archival_enabled() could be set to false by the user
There was a problem hiding this comment.
Ah, that's a complication.
| // partition, and on restart/recovery a still-migrating partition is | ||
| // constructed cloud-mode -- in both cases the STM must be present. On a | ||
| // native cloud topic it is an inert passenger: retention and DeleteRecords | ||
| // go through the L1 path, so no local eviction requests are generated. |
There was a problem hiding this comment.
This is not correct, on a native cloud topic or TSv2 partition ctp_stm serves the same role as log_eviction_stm. It truncates the local log.
There is another big difference between the TSv1 and TSv2. The disk_log_impl instance is started with the background housekeeping loop disabled. TSv1 and local only partitions need this loop to funciton but ctp_stm does not rely on it.
There was a problem hiding this comment.
The comment means that log_eviction_stm specifically won't do any local eviction.
Lazin
left a comment
There was a problem hiding this comment.
I think the approach with multiple STMs running all the time is not simpler compared to handoff (STM A stops, STM B starts). We will have to guarantee that both STMs are not doing something that they can't do together.
|
I do have a branch with an approach to removing/adding stms, and I'd actually prefer that if there's an appetite for the additional mechanics. The problem of TS and CT background operations occurring potentially at the same time is still present, however. For the most part, those behaviors depend on the config rather than the presence or absence of an stm, and will still require adjustments to deal with mid-migration situation. |
ef59066 to
58fdc72
Compare
In order to dynamically switch a partition from TS to CT/TSv2, we need to be able to enable the ctp_stm on the fly. The simplest way to do that is to simply attach an empty ctp_stm to every partition. Unfortunately, with LRO unset, ctp_stm previously returned offset::min() for get_max_collectible_offset() preventing log trimming. This commit modifies that behavior to return offset::max() if _max_applied_epoch is empty making the presence of a ctp_stm harmless until advance_epoch() is called. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tions is_applicable_for: pre-install the (idle) ctp_stm on tiered-storage partitions when cloud topics are available, so a tiered->cloud migration needs no runtime STM install -- the STM is already present and inert until the partition becomes a cloud topic. STM membership is fixed at construction, so it has to be there beforehand. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…verload Split advance_reconciled_offset so the reconciled log offset can be supplied explicitly instead of always deriving it from the raft offset translator. The existing kafka-offset-only entry point now delegates, deriving the log offset as before. This lets a caller seed the reconciliation baseline of a TS-migrated partition directly to the migration boundary -- passing the precise raft offset of the last uploaded tiered-storage segment -- so the already reconciled region can be trimmed without re-deriving the offset. ctp_stm carries no migration state of its own; this is expressed purely as "reconciliation starts at offset N". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
During migration, we need the newly configured TSv2 or CT partition to continue to behave as an archival stm. Thus, we need the archival stm to continue to exist after the config switch. The simplest way to do this is to allow it to exist generally on CT/TSv2 topics. Drop the cloud_topic_enabled() == false guard from archival_metadata_stm_factory::is_applicable_for, so every cloud-topic partition carries an archival STM. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A cloud-mode partition whose storage mode has been flipped reports is_archival_enabled() == false, which would make max_removable_local_log_offset collect_all and stop constraining local-log truncation -- evicting tiered-storage data not yet uploaded. Keep constraining via cloud_recoverable_offset() while the partition still holds archived data (a non-empty live manifest or a spillover archive), introduced here as holds_archived_data(); once the manifest is emptied, trimming resumes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the cloud_topic_enabled() guard from log_eviction_stm_factory so the eviction STM is installed on cloud-topic partitions too. A partition migrating from tiered storage is still served from its local log + archival manifest and must keep supporting prefix truncation (DeleteRecords) while migrating; the trigger flips storage_mode to cloud without reconstructing the partition, and a still-migrating partition is reconstructed cloud-mode on restart/recovery, so the STM must already be present. On a native cloud topic it is an inert passenger. Subsequent integration tests will provide coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
58fdc72 to
dfd6695
Compare
Summary
In order to dynamically convert a TS partition to CT/TSv2, we need to be able to:
The simplest way to achieve the above is for all TS and CT/TSv2 partitions to already have all three. This PR adjusts is_applicable_for on these stms to make the above happen and adjusts as needed trimming behavior to prevent empty stms from causing problems and to ensure that the archival_metadata_stm's constraints still apply during migration.
The PR also includes a minor ctp_stm change to allow specifying a reconciled log offset directly.
What's in this PR
cloud_topics/stm: don't pin truncation on an idlectp_stm—get_max_collectible_offset()returnsoffset::max()while no epoch has beenapplied (
_max_applied_epochempty), so actp_stmthat has seen no cloud datanever pins local-log truncation. Once
advance_epoch()is called it falls backto
offset::min()to protect unreconciled data until the LRO advances.cloud_topics/stm: pre-install an idlectp_stmon tiered-storagepartitions — so a
tiered→cloudmigration needs no runtime STM install (STMmembership is fixed at construction). Also makes
apply_raft_snapshottreat anempty buffer as a no-op (the recovery fast-forward hands each STM an empty
snapshot to advance over).
cloud_topics/stm: explicit-log-offsetadvance_reconciled_offsetoverload — lets a caller supply the reconciled log offset directly instead of
deriving it from the raft offset translator; the existing kafka-offset-only
entry point now delegates.
archival: create the archival STM on cloud-topic partitions — drop thecloud_topic_enabled() == falseguard so a migrated partition keeps itsarchival STM after the storage-mode flip; an always-cloud topic carries an
inert, empty STM.
archival: clamp local-log trim while the manifest holds archived data — acloud-mode partition reports
is_archival_enabled() == false, which would letmax_removable_local_log_offsetcollect-all and evict not-yet-uploaded tiereddata; keep constraining via
cloud_recoverable_offset()whileholds_archived_data()(live manifest non-empty or spillover archivepresent).
cluster: install the eviction STM on cloud-topic partitions — so amigrating partition keeps supporting prefix truncation (DeleteRecords) and
retention while migrating; an inert passenger on a native cloud topic.
Testing
ctp_stm_state_test: an idleget_max_collectible_offset()returnsmax().ctp_stm_test: applying an empty raft snapshot is a no-op (neither throws norresets applied state).
archival_metadata_stm_test: the local-trim clamp keeps constraining while themanifest is non-empty.
🤖 Generated with Claude Code
Design Doc
PR 0 · PR 1 · PR 2 · PR 3 · PR 4 · PR 5 · PR 6 · PR 7 · PR 8