cluster/rm_stm: preserve open transaction producers in local snapshots - #30003
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes an rm_stm local-snapshot correctness bug where transactional producers with an open transaction but no finished requests yet could be dropped from the snapshot, causing restart-time log replay to synthesize incomplete transaction state and permanently stall LSO.
Changes:
- Preserve producers with
has_transaction_in_progress()inrm_stm::do_take_local_snapshot. - Add a regression test that snapshots after tx fences but before data batches, then simulates restart and verifies commit/abort work.
- Add a test-fixture helper to replay-apply record batches.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/v/cluster/rm_stm.cc | Includes open-transaction producers in local snapshots to prevent losing tx metadata across restarts. |
| src/v/cluster/tests/rm_stm_tests.cc | Adds restart/regression test for open-tx producer preservation and post-restart commit/abort behavior. |
| src/v/cluster/tests/rm_stm_test_fixture.h | Exposes a helper for applying batches during simulated replay in tests. |
| apply_snapshot(snapshot.header, std::move(snapshot.data)).get(); | ||
| auto rdr = _raft->log() | ||
| ->make_reader( | ||
| storage::local_log_reader_config( | ||
| model::next_offset(snap_offset), model::offset::max())) | ||
| .get(); | ||
| auto batches = model::consume_reader_to_memory( | ||
| std::move(rdr), model::no_timeout) | ||
| .get(); | ||
| for (const auto& batch : batches) { | ||
| replay_apply(batch).get(); | ||
| } |
There was a problem hiding this comment.
wondering if we can have a restart() helper in the stm and rm_stm fixtures.
this seems a bit hacky.
There was a problem hiding this comment.
Good point, will switch to using _stm->{start, stop}() in a restart helper for the fixture as discussed.
Retry command for Build#82511please wait until all jobs are finished before running the slash command |
Retry command for Build#82515please wait until all jobs are finished before running the slash command |
|
test failures are unrelated, trying to fetch tagged release of |
do_take_local_snapshot filters producers by finished_requests, which
drops transactional producers that have begun (fence batch applied) but
not yet replicated data batches. On restart, the snapshot is loaded and
log replay starts from the snapshot offset skipping the fence batch.
Data batches replayed without the fence synthesize transaction state
with tx_seq{-1} and timeout=nullopt. This makes the transaction
impossible to commit (tx_seq mismatch), abort (tx_seq seen as from the
future), or auto-expire (timeout=max), permanently stalling LSO on the
partition.
Include producers with in-progress transactions in local snapshots
regardless of finished_requests so their tx_seq, timeout, and
coordinator partition survive the snapshot roundtrip.
|
/backport v26.1.x |
|
/backport v25.3.x |
|
/backport v25.2.x |
|
Failed to create a backport PR to v25.3.x branch. I tried: |
|
Failed to create a backport PR to v25.2.x branch. I tried: |
Fixes a bug in
rm_stm::do_take_local_snapshotwhere transactional producers with open transactions but no data batches are silently dropped from local snapshots. On restart, the fence batch (which carriestx_seq,timeout, andcoordinator_partition) is behind the snapshot offset and not replayed. Data batches replayed after the snapshot synthesize transaction state withtx_seq{-1}andtimeout=nullopt, making the transaction impossible to commit, abort, or auto-expire, permanently stalling LSO on the partition.Steps to reproduce
finished_requestsempty, so it's dropped from the snapshot.snapshot_offset + 1(fence is not replayed). The data batches will be replayed andapply_datawill synthesizestransaction_statewithtx_seq{-1}andtimeout=nullopt.commit_txwithtx_seq=0(or whatever the actual seq was) and therm_stmfor the partition will reject it(-1 != 0). It could also send aabort_txwithtx_seq=0and therm_stmfor the partition will reject it as well(-1 < 0interpreted as "from the future")ms_since_last_update() > timeout_ms(),timeout_ms()returnsmax()due tonulloptso it'll never expire.All of this will result in the LSO for the partition becoming permanently stuck.
Backports Required
Release Notes