Skip to content

Commit da32918

Browse files
committed
Document the unanswered-commit case in transactional persistence
The transactional mode's one failure that is not a rejection: an unanswered commit. If `commitTransaction` times out client-side, the broker may have committed anyway. Nothing local resolves it - retrying blocks for up to `max.block.ms` inside the poll cycle, and `abortTransaction` throws rather than aborting while a commit is unresolved, a throw `commitBatch` already discards. Nothing is lost either way: the input offset rides the same transaction, so snapshot and offset commit together or not at all. Under the default `ignorePersistErrors = false` the failure tears the module down and the next producer's `initTransactions` settles the transaction broker-side. Under `ignorePersistErrors = true` it is swallowed and the producer stays unusable for the rest of the assignment, so the partition stops persisting AND stops advancing offsets until the next rebalance - frozen and self-consistent, not corrupted. GroupCommitSpec gains a case pinning that the commit is never retried in place. No production code changes. The contract and a measurement of it are pinned as ext(K16) in the verification research corpus; that harness is not checked in, since it pins a kafka-clients contract rather than kafka-flow behavior.
1 parent 6f76364 commit da32918

2 files changed

Lines changed: 48 additions & 7 deletions

File tree

docs/persistence.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,16 @@ You do not catch the rejection yourself; it is handled for you:
6363
Either way the rejected write does not land and no offset is committed for it, so the new owner
6464
replays the affected events.
6565

66+
One **Kafka** transactional-mode failure is not a rejection but an *unanswered* commit: if
67+
`commitTransaction` times out client-side, the broker may have committed anyway. The write is reported
68+
as failed and nothing local can resolve it — retrying blocks for up to `max.block.ms` inside the poll
69+
cycle, and aborting is inert while a commit is unresolved. Nothing is lost: the input offset rides the
70+
same transaction, so snapshot and offset commit together or not at all, and the failure tears the flow
71+
down so the rebuilt module's `initTransactions` settles it. Note that `ignorePersistErrors = true`
72+
swallows this too and leaves the producer unusable for the rest of the assignment, so the partition
73+
stops persisting *and* stops advancing offsets until the next rebalance — frozen and self-consistent,
74+
not corrupted.
75+
6676
### Transactional snapshot writes (Kafka)
6777

6878
**EXPERIMENTAL** — use at your own risk: the mechanism is design-verified but not yet proven in

persistence-kafka/src/test/scala/com/evolutiongaming/kafka/flow/kafkapersistence/GroupCommitSpec.scala

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import com.evolutiongaming.skafka.consumer.ConsumerGroupMetadata
1212
import com.evolutiongaming.skafka.producer.{Producer, ProducerRecord, RecordMetadata}
1313
import com.evolutiongaming.skafka.{Offset, OffsetAndMetadata, Partition, ToBytes, Topic, TopicPartition}
1414
import munit.FunSuite
15+
import org.apache.kafka.common.errors.TimeoutException
1516

1617
import scala.concurrent.duration.*
1718

@@ -235,6 +236,31 @@ class GroupCommitSpec extends FunSuite {
235236
}
236237
test.unsafeRunSync()
237238
}
239+
240+
// a client-side commit timeout is ambiguous (the broker may have committed) and surfaces as itself; retrying in
241+
// place is the tempting "fix" this guards against, since each attempt blocks up to max.block.ms inside a poll
242+
// cycle. See docs/persistence.md. The abort's inertness needs a real client - this fake's abort succeeds.
243+
test("an ambiguously timed-out commit surfaces as itself, unretried") {
244+
val test = for {
245+
events <- Ref.of[IO, Vector[Event]](Vector.empty)
246+
attempts <- Ref.of[IO, Int](0)
247+
boom = new TimeoutException("commit timed out")
248+
commit = attempts.update(_ + 1) *> IO.raiseError[Unit](boom)
249+
tx <- buildTransactional(
250+
recordingProducer(events, commitTransactionOf = commit.some),
251+
ConsumerGroupMetadata.Empty.some,
252+
maxWritesPerTransaction = 256,
253+
)
254+
result <- tx.writeDatabase.persist(kafkaKey("key1"), "state-1").attempt
255+
tries <- attempts.get
256+
} yield {
257+
// told it failed, though the broker may have committed - only a rebuilt producer resolves that
258+
assertEquals(result.left.toOption, boom.some)
259+
// exactly once: never retried in place
260+
assertEquals(tries, 1)
261+
}
262+
test.unsafeRunSync()
263+
}
238264
}
239265

240266
object GroupCommitSpec {
@@ -251,19 +277,24 @@ object GroupCommitSpec {
251277
private val CommitBoom = new RuntimeException("commit boom")
252278

253279
/** A `Producer` that records the transactional calls into `events` and delegates everything else to a no-op producer
254-
* (which also fabricates the `RecordMetadata` for `send`). `failCommit` makes `commitTransaction` raise.
280+
* (which also fabricates the `RecordMetadata` for `send`). `failCommit` makes `commitTransaction` raise;
281+
* `commitTransactionOf` replaces the whole `commitTransaction` behavior (the override must record `Event.Commit`
282+
* itself when it succeeds).
255283
*/
256284
def recordingProducer(
257285
events: Ref[IO, Vector[Event]],
258-
failCommit: Boolean = false,
259-
onBeginTransaction: IO[Unit] = IO.unit,
286+
failCommit: Boolean = false,
287+
onBeginTransaction: IO[Unit] = IO.unit,
288+
commitTransactionOf: Option[IO[Unit]] = None,
260289
): Producer[IO] = {
261290
val base = Producer.empty[IO]
262291
new Producer[IO] {
263-
def initTransactions: IO[Unit] = base.initTransactions
264-
def beginTransaction: IO[Unit] = events.update(_ :+ Event.Begin) *> onBeginTransaction
265-
def commitTransaction: IO[Unit] = if (failCommit) IO.raiseError(CommitBoom) else events.update(_ :+ Event.Commit)
266-
def abortTransaction: IO[Unit] = events.update(_ :+ Event.Abort)
292+
def initTransactions: IO[Unit] = base.initTransactions
293+
def beginTransaction: IO[Unit] = events.update(_ :+ Event.Begin) *> onBeginTransaction
294+
def commitTransaction: IO[Unit] = commitTransactionOf.getOrElse {
295+
if (failCommit) IO.raiseError(CommitBoom) else events.update(_ :+ Event.Commit)
296+
}
297+
def abortTransaction: IO[Unit] = events.update(_ :+ Event.Abort)
267298

268299
def sendOffsetsToTransaction(
269300
offsets: NonEmptyMap[TopicPartition, OffsetAndMetadata],

0 commit comments

Comments
 (0)