Recover transactional snapshots completely, with a bounded read - #16
Recover transactional snapshots completely, with a bounded read#16tobiajo wants to merge 1 commit into
Conversation
The combined implementation moved to tj/recovery-read-complete-and-bounded (#16); the register's status row and the temporary review record now reference it instead of the closed precursor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ufJ4z7CT68rDwfCaLiMeF
69d6832 to
8b5337e
Compare
| * [[KafkaPartitionPersistence.RecoveryReadStalledError]] instead of hanging. Keep it below the driving consumer's | ||
| * `max.poll.interval.ms` and above the open-transaction wait; both bounds are warned at module acquisition. The | ||
| * former is checked against this `consumerConfig`'s `maxPollInterval` as a stand-in - the module does not receive | ||
| * the driving consumer's config. Defaults to [[KafkaPartitionPersistence.defaultStallTimeout]]. |
There was a problem hiding this comment.
Inconsistent with maxWritesPerTransaction where default arg not mentioned. Also leading constant capital or not?
50f094c to
f819159
Compare
| brokers — which prevents the class). | ||
| Against one of those the deadline is the only client-side bound there is. | ||
|
|
||
| The two bounds that make the deadline meaningful are checked and warned at module acquisition — it |
There was a problem hiding this comment.
"The two bounds that make the deadline meaningful are checked and warned at module acquisition ..."
Not sure if interesting in design doc.
| `max.poll.interval.ms` evicts the member silently while process-level health checks stay green. | ||
|
|
||
| A no-progress deadline (`recoveryStallTimeout`, default 2 min, measured from the last position | ||
| advance) turns that hang into a `RecoveryReadStalledError`, after ~5 s stall log lines have already |
There was a problem hiding this comment.
We usually don't name internals outside the exception "Implementation" which acts as a map
| gone, so recovery becomes an offset-reset or restore decision for an operator; at or above it names | ||
| an open transaction that outlived the deadline. That last case usually self-heals — the broker | ||
| aborts the transaction within its `transaction.timeout.ms` plus the abort scan — but Kafka has a | ||
| known limitation where it does not: a *hanging* transaction, whose pin no timeout ever resolves |
There was a problem hiding this comment.
I think we should lead with the known limitation, that is the interesting that could cause silent hang? That we diagnose the other is only incidental?
| batch typically commits in well under a second. | ||
| Transaction-coordinator state is bounded by the partition count. | ||
|
|
||
| ### Recovery read: bounded by the high watermark |
There was a problem hiding this comment.
Maybe this section before stable IDs, as this is what is load bearing for guaranteed correctness even when prefix changes. Otherwise it might confuse.
| `ReadSnapshotsSpec` the read itself: a target taken from the read consumer's own `endOffsets` stops | ||
| at the LSO and fails the suite; a parked read fails at the deadline with the matching diagnosis | ||
| (outliving transaction, truncation, or undetermined when the re-read fails), a progressing read | ||
| outlives it, an unarmed read keeps waiting; the stall-log throttle and the wait warn are pinned |
There was a problem hiding this comment.
I don't think stall logging (pre deadline) is interesting enough for the design docs.
| discipline — but nothing ever aborts a crashed owner's unfinished transaction, so every post-crash recovery | ||
| waits out the full timeout the stable id resolves at init (see Recovery read), and coordinator | ||
| state accumulates per assignment until `transactional.id.expiration.ms`. | ||
| - **The reader's own LSO as the bound (no high-watermark capture)**: complete only while every |
There was a problem hiding this comment.
I don't think this classify under rejected alternatives, it was never considered intentionally.
| - **The reader's own LSO as the bound (no high-watermark capture)**: complete only while every | ||
| transaction on the topic belongs to the partition's lineage; the captured target holds | ||
| unconditionally and turns the residual cases into a loud wait (see Recovery read). | ||
| - **Recomputing the target on a stall** (lowering it to a re-read log end): silently completes an |
There was a problem hiding this comment.
Similar here as R387 comment?
| - **Producer-epoch order as the fence**: epoch order can diverge from ownership order, spuriously | ||
| fencing the true owner — the stable id is kept for the takeover-abort, never as the fence (see | ||
| Stable transactional.id). | ||
| - **Unique per-assignment `transactional.id`s** (`"<prefix>-<partition>-<uuid8>"`): no naming |
There was a problem hiding this comment.
I don't think naming discipline is why interesting but the theoretical previously states:
"epoch order can diverge from ownership order (whichever owner inits latest wins), spuriously fencing the true owner"
Not sure if should be captured in fully here, but the tradeoffs between that and possibility to abort, was what the decision boiled down too.
Even before this PR, it was assumed (incorrectly) that we would wait out transactions — then we chosed the other which is also sane — it was a valid tradeoff.
| staged, it commits nothing (a no-op). | ||
| - **Generation currency** — the `Consumer` wrapper holds `groupMetadata` in a `Ref`, refreshed after every | ||
| poll. | ||
| - **Log evidence** — an unfinished transaction resolves in one of two ways (aborted by the takeover's |
There was a problem hiding this comment.
Not sure how interesting, sure it is good that we log on the unexpected waiting out (when no prefix change at least) that "recovery read warns when its target sits above the last-stable-offset".
But the init logging is not much of evidence at all.
Not sure here.
| completes while silently missing committed snapshots above the pin. With a second handover inside the | ||
| window the next owner recovers stale state yet resumes from the newer committed input offset — the | ||
| corruption shape of [#732](https://github.com/evolution-gaming/kafka-flow/issues/732) with no fence | ||
| violated ([kafka-flow#850](https://github.com/evolution-gaming/kafka-flow/issues/850)). |
There was a problem hiding this comment.
Let's not mention issue evolution-gaming#850 and evolution-gaming#849 — instead "pretend" we designed for this from start. The transactional Kafka is not properly released yet.
|
|
||
| ### Recovery read: bounded by the high watermark | ||
|
|
||
| The takeover-abort covers exactly one lineage — the partition's own id. An open transaction from |
There was a problem hiding this comment.
Could be worth include reference to Kafka Streams doing the same LSO wait. I think there was some KAFKA issue for the same.
| - **Recovery fails loudly rather than hangs** — a recovery read that makes no progress for | ||
| `recoveryStallTimeout` (default 2 min) fails with `RecoveryReadStalledError` instead of hanging the | ||
| rebalance until the member is silently evicted at `max.poll.interval.ms`. The error names the | ||
| diagnosed cause: the snapshot log was truncated under the read (an unclean leader election lost |
There was a problem hiding this comment.
Refer to known KAFKA issue or KIP that can be helpful practically? Like leading for how to resolve manually or when expected to be fixed so can self-heal.
The persistence.md is for library users while the design doc for library aurhours.
| - Changing `transactionalIdPrefix` needs care: if an old-prefix instance hard-crashes during the | ||
| rollout, its unfinished transaction belongs to an id no new instance will ever init, so recovery | ||
| waits until the broker times it out — up to ~70 s at the defaults (`transaction.timeout.ms` plus the | ||
| broker's abort scan), never a wrong read. Prefer a full stop over a rolling prefix change to avoid the wait |
There was a problem hiding this comment.
"Prefer a full stop" unclear if any benefit with graceful shutdown? There is no correctness risk regardless.
| ([kafka-flow#850](https://github.com/evolution-gaming/kafka-flow/issues/850)). | ||
| - A stale owner's late `initTransactions` can fence the current owner's producer: that owner's flow | ||
| fails once and recovers (rebalance and replay); no wrong write can land. | ||
| - Changing `transactionalIdPrefix` needs care: if an old-prefix instance hard-crashes during the |
There was a problem hiding this comment.
Not necessarily a hard-crash, could also be a forced pod delete or similar. Maybe hard-crash includes that? Maybe fencing also cause hard-crash? Maybe just nitpicking unnecessary here and wrong — but good if exact.
| // outcome for a partition that is being given away anyway | ||
| // the release itself succeeds: the rejected write surfaces as a logged-and-swallowed cache entry | ||
| // release error ("scache: failed to release cache entry: ..."). Under the shared stable id B's init | ||
| // has already epoch-fenced A, so the swallowed error is ProducerFencedException; the generation |
There was a problem hiding this comment.
Correct with ProducerFencedException?
There was a problem hiding this comment.
Good catch — no. The CI log of this suite shows the swallowed error is InvalidProducerEpochException ("Producer attempted to produce with an old epoch"): still B's init epoch-fencing A, but surfacing as the produce-path rejection rather than ProducerFencedException. Comment corrected to the observed error.
Generated by Claude Code
e3eff27 to
6bcbc67
Compare
The recovery read stopped at the read consumer's own end offset - under read_committed that is the last-stable-offset, which an open transaction pins below records committed after it. A recovery inside that window silently missed committed snapshots while input offsets were not held back the same way, so a second handover could recover stale state yet resume from the newer committed offset. The read now targets the high watermark, captured up front through a short-lived read_uncommitted consumer, and waits out any open transaction below it - the shape Kafka Streams' exactly-once restore settled on (KAFKA-10167). A stable per-partition transactional.id makes the wait rare: a takeover's initTransactions aborts a crashed predecessor's unfinished transaction sub-second. Fencing of stale writers stays with the consumer generation, so completeness rests on the read bound, never on naming. That wait is normally self-resolving, but a hanging transaction (KIP-664; prevented by KIP-890 brokers) or a log truncated below the captured target made the read hang forever on the poll thread inside the rebalance callback, until max.poll.interval.ms evicted the member silently. A no-progress deadline (default 2 minutes, measured from the last position advance) fails the read with RecoveryReadStalledError instead, diagnosed against a re-read log end - truncation versus a transaction that outlived the deadline - freeing the thread for a restarted recovery. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ufJ4z7CT68rDwfCaLiMeF
4fdee3e to
3c378b2
Compare
The combined implementation moved to tj/recovery-read-complete-and-bounded (#16); the register's status row and the temporary review record now reference it instead of the closed precursor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ufJ4z7CT68rDwfCaLiMeF
1 participant
Transactional snapshot recovery had two gaps, surfaced by the research linked below; both are closed here, with the mechanism documented in the included design doc.
The silent miss (#850). Under
read_committedthe read stopped at the last-stable-offset, so an open transaction pinned it below newer committed snapshots: recovery returned incomplete state, and a second handover resumed from the newer committed offset over the stale state — the corruption shape of #732, with no fence violated. The read now targets the high watermark (captured up front through a short-livedread_uncommittedconsumer) and waits out open transactions below it — the shape Kafka Streams' exactly-once restore settled on (KAFKA-10167). A stable per-partitiontransactional.idkeeps the wait sub-second (a takeover'sinitTransactionsaborts a crashed predecessor's transaction); fencing deliberately stays with the consumer generation, so completeness rests on the read bound, never on naming.The silent hang (#849). Truncation (the log end regressing below the captured target) or a hanging transaction (KIP-664 detects and aborts these; KIP-890 brokers prevent them) could keep the read from its target forever — on the poll thread, so
max.poll.interval.msevicted the member silently. A no-progress deadline (default 3 minutes, reset by every position advance) fails the read withRecoveryReadStalledErrorinstead, diagnosed against a re-read log end (truncation versus a transaction that outlived the deadline — opposite operator responses), and the restarted recovery captures a fresh target.Follow-up commit. A post-merge adversarial review found the ephemeral recovery consumers inheriting the caller's
groupIdandautoCommit(on by default in skafka): a committed offset from a prior read pre-empts theearliestreset and silently truncates the next recovery. Pre-existing since the first snapshot-recovery read; both fields are now forced off at the shared chokepoint, and the module spec seeds the hazardous shape and asserts both consumers come out cleared.Review aid. The recovery read under stall on one page — the poll loop's advancing/waiting turns, the four scenarios, the verbatim log lines; kept outside the PR:
Backing research. Worked out in the research corpus (#835) — requirements register, TLA+ models of the read bound and the stall deadline, the remedy decision, the follow-up as finding F-12. The checked-in material stands alone and does not reference it.