Skip to content

Commit 91f9e80

Browse files
astubbsclaude
andcommitted
refactor(core) #225: put recovery on producerConfig alone, and move the factory, the derived id, redaction and the deprecation up the stack
This PR now stacks on #426 - producerConfig with the default constructor - rather than on #420. Everything #420 carries beyond that rung leaves this branch's tree here and returns above it, in #420 itself, which now stacks on this PR: ProducerFactory and its contract exception, TransactionalIdDerivation, ProducerConfigRedaction (and their tests), the deprecation of the instance option with its WARN and removal queue, the README ownership section and migration note, and the four migrated example apps, which go back to master's. What recovery is built on instead: the module's protected buildProducer(Map) seam from #426. replacementProducerWrap() builds another wrapper through it from the same configuration, the caller's transactional.id included, and hands the id to the ReplacementProducerSource so a failure to build can still name it. The build runs through UserFunctions.carefullyRun as before: the seam is overridable, and an Error from the constructor must surface as a build failure rather than escape the recovery pass. The policy's terminal set loses the factory-contract clause with the factory; Authorization, UnsupportedVersion and Error stay. Tests move with it. ProducerRecoveryTest hands ParallelEoSStreamProcessor a PCModule whose seam hands out the spied MockProducers the factory used to, records the id each build was handed, and asserts it is the caller's, unchanged, on every build; the log-line and failure-message assertions key on a per-instance id the test sets, as they keyed on the derived one. ProducerFencingRecoveryIT sets its own transactional.id so the rogue can fence under it, and drops the derived-shape assertion, which returns with the derivation. ProducerManagerRecoveryTest's factory-contract case leaves with the contract. The two option/module test classes are rung 1's. Records: the plan carries a dated re-cut note at its head rather than a rewrite of its units; the in-flight note lists the three-PR stack; the roadmap entry, the issue-response draft, the refactoring queue and the README template (a producer-recovery section only, no ownership section) say what this PR now says. Mechanically this commit is the inverse of what #420 becomes: reverting it on top of this branch is how #420 is re-based, so its history is kept too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VrpH51xNDodaajE4P2nhFg
1 parent 8e02d82 commit 91f9e80

35 files changed

Lines changed: 300 additions & 1137 deletions

File tree

README.adoc

Lines changed: 7 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -523,30 +523,30 @@ Where `${project.version}` is the version to be used:
523523
[source,java,indent=0]
524524
----
525525
Consumer<String, String> kafkaConsumer = getKafkaConsumer(); // <1>
526+
Producer<String, String> kafkaProducer = getKafkaProducer();
526527
527528
var options = ParallelConsumerOptions.<String, String>builder()
528529
.ordering(KEY) // <2>
529530
.maxConcurrency(1000) // <3>
530531
.consumer(kafkaConsumer)
531-
.producerConfig(getProducerConfig()) // <4>
532-
.producerFactory(getProducerFactory())
532+
.producer(kafkaProducer)
533533
.build();
534534
535535
ParallelStreamProcessor<String, String> eosStreamProcessor =
536536
ParallelStreamProcessor.createEosStreamProcessor(options);
537537
538-
eosStreamProcessor.subscribe(of(inputTopic)); // <5>
538+
eosStreamProcessor.subscribe(of(inputTopic)); // <4>
539539
540540
return eosStreamProcessor;
541541
----
542542

543543
<1> Setup your clients as per normal.
544+
A Producer is only required if using the `produce` flows: hand PC either a finished `Producer` instance, or the configuration to build one from as `producerConfig` - any `ProducerConfig` key, serializers included, and PC builds it with `new KafkaProducer<>(config)`, and builds it again if the broker invalidates it (see <<producer-recovery>>).
545+
In the transactional commit mode, set `transactional.id` in that configuration as you would when building the producer yourself.
544546
<2> Choose your ordering type, `KEY` in this case.
545547
This ensures maximum concurrency, while ensuring messages are processed and committed in `KEY` order, making sure no offset is committed unless all offsets before it in it's partition, are completed also.
546548
<3> The maximum number of concurrent processing operations to be performing at any given time.
547549
Also, because the library coordinates offsets, `enable.auto.commit` must be disabled in your consumer.
548-
<4> Producer configuration, only required if using the `produce` flows: PC builds the producer from it through the factory on the next line (the default is `new KafkaProducer<>(config)`; override it to wrap or instrument the producer, keeping every key it is given), and rebuilds it if the broker invalidates it - see <<producer-ownership>> and <<producer-recovery>>.
549-
Handing PC a finished `Producer` instance still works, deprecated.
550550
<5> Subscribe to your topics
551551

552552
NOTE: Because the library coordinates offsets, `enable.auto.commit` must be disabled.
@@ -635,7 +635,7 @@ There is no guarantee that the messages will be retried again in the same batch.
635635
----
636636
ParallelStreamProcessor.createEosStreamProcessor(ParallelConsumerOptions.<String, String>builder()
637637
.consumer(getKafkaConsumer())
638-
.producerConfig(getProducerConfig())
638+
.producer(getKafkaProducer())
639639
.maxConcurrency(100)
640640
.batchSize(5) // <1>
641641
.build());
@@ -794,18 +794,6 @@ The API is source-compatible, so beyond the import lines no source edit is neede
794794
Committed offset metadata is unchanged too, so an existing consumer group can be upgraded in place without resetting or migrating offsets.
795795
The library's shared internal utilities moved with the rest, into `bz.stub.parallelconsumer.internal.utils` -- they were always plumbing, they are not part of the published API, and they are now named as such.
796796

797-
==== Move from a producer instance to producer configuration
798-
799-
The `producer` option - a finished `Producer` instance - is deprecated in favour of `producerConfig` (plus, optionally, `producerFactory`), and its removal is queued for `0.7.0.0`.
800-
Nothing forces the move in this release, but only the configuration path recovers when the broker invalidates the producer; see <<producer-recovery>>.
801-
To move:
802-
803-
* Replace `.producer(kafkaProducer)` with `.producerConfig(producerProperties)`, the same map you built the producer from, serializers included.
804-
* Drop any `transactional.id` from that map. PC derives one per running instance, and a value you set is replaced with a WARN.
805-
* If your cluster grants TransactionalId ACLs, grant a prefixed one on `pc-<L>-<group.id>-` (`<L>` being the length of your `group.id`) - the literal id you used to grant on will no longer be used.
806-
* If operational tooling keyed on your `transactional.id` - dashboards, alerts, `kafka-transactions.sh` scripts - it now needs the derived prefix instead; the exact id is logged at start-up.
807-
* If you wrapped or instrumented the producer, do that in a `producerFactory` instead; it receives the resolved configuration and must build from it unchanged.
808-
809797
=== From 0.4 to 0.5
810798

811799
This version has a breaking change in the API - instead of passing in `ConsumerRecord` instances, it passes in a `PollContext` object which has extra information and utility methods.
@@ -1101,24 +1089,11 @@ It is covered by an existing test, but no negative control was observed for it,
11011089
The produce-lock timeout returning the record for retry started out the same way and no longer is: the eager-processing test holds the commit lock shut until a worker's acquisition is observed to time out, then watches that record's retry succeed, and removing the hold turns it red.
11021090
It is recorded `PROVED`.
11031091

1104-
[[producer-ownership]]
1105-
=== Producer ownership
1106-
1107-
Supply the producer as configuration - `producerConfig`, the map you would hand `new KafkaProducer<>(config)` - and PC builds the producer itself, through `producerFactory` (default: exactly that constructor; override it to wrap or instrument the producer, keeping every key it is given).
1108-
PC sets the `transactional.id` where it builds the producer: `pc-<L>-<group.id>-<uuid>`, where `<L>` is the decimal length of the consumer's `group.id`.
1109-
The prefix `pc-<L>-<group.id>-` is stable for the group, so one prefixed TransactionalId ACL authorises every id PC derives for it, and the length field keeps one group's prefix from being a prefix of another's (`app` versus `app-x`).
1110-
A `transactional.id` set in `producerConfig` does not take effect there, and PC says so at WARN naming the value it derived instead.
1111-
Configuration values are never rendered into logs: PC's own log lines and `toString()` show an allow-list of non-secret keys and redact the rest.
1112-
A factory must return a new producer on every call - PC may discard one and build another from the same configuration - and a factory that returns one it already returned, or drops the `transactional.id` it was given, is rejected at construction naming that contract.
1113-
1114-
Supplying a finished `Producer` instance still works for every flow it worked for, and is deprecated: PC cannot read a finished producer's configuration back out, so it can never build another from it.
1115-
PC logs one WARN at start-up naming the remedy.
1116-
The instance option's removal is queued for `0.7.0.0`.
1117-
11181092
[[producer-recovery]]
11191093
=== Producer recovery
11201094

11211095
Where PC built the producer, a producer the broker reports invalid is replaced and the work its aborted transaction discarded is processed again, so processing continues instead of the instance stopping.
1096+
The replacement is built from the same configuration, `transactional.id` included, and initialising it is what fences the producer it replaces; a finished `Producer` instance carries no configuration to rebuild from, so on that path these conditions keep their earlier behaviour.
11221097
Recovery is a transactional-mode mechanism: it runs from the transactional commit loop, so in the consumer-commit modes a PC-built producer is not recovered and these conditions surface as the produce failures they were before.
11231098
To make the replay possible, transactional mode keeps every completed record until the commit that carries it succeeds - on both producer paths - so memory held per partition grows with `commitInterval`; the default interval keeps it to a fraction of a second of throughput.
11241099
The conditions PC treats this way are `ProducerFencedException`, `InvalidProducerEpochException`, `InvalidPidMappingException`, `OutOfOrderSequenceException` (and so `UnknownProducerIdException`) and `CommitFailedException`, on both the commit path and the produce path, unwrapped from the `ExecutionException` a send future raises and the `KafkaException` the client wraps a stored error in.

docs/data/roadmap.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,10 +190,10 @@ entries:
190190
stage_detail: >-
191191
Implemented on astubbs#410 to the plan's Definition of Done: PC builds its own transactional producer
192192
from configuration, detects the broker-reported conditions on both paths, aborts, replays the
193-
completed-but-uncommitted ledger, and rebuilds under the same derived id; proven at the unit level
193+
completed-but-uncommitted ledger, and rebuilds under the same transactional.id; proven at the unit level
194194
(fenced-during-commit was red before the change) and on a real coordinator by ProducerFencingRecoveryIT,
195195
with the recovery guarantee registered as a transactional claim. Not yet run in anger, so not
196-
"implemented"; the producer-instance path keeps its old behaviour and is deprecated.
196+
"implemented"; the producer-instance path keeps its old behaviour.
197197
why_now: >-
198198
Fencing is an ordinary consequence of a rebalance under transactions, and a broker can expire an
199199
idle producer id outright; either way PC dies on the commit path and spins on the produce path.
@@ -202,7 +202,7 @@ entries:
202202
done_when: >-
203203
A producer the broker has invalidated is replaced and processing continues, with no offset from
204204
the aborted transaction committed and its work processed again. Applies where PC builds the
205-
producer; supplying a Producer instance is deprecated and keeps today's behaviour.
205+
producer; supplying a Producer instance keeps today's behaviour.
206206
tracking: astubbs#225
207207
related: astubbs#411
208208
pull_request: astubbs#410

docs/inflight/core-recoverable-producer-fencing.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,19 @@
44
<!-- inflight-impact: crash -->
55

66

7+
## In flight as a stack of three PRs (2026-09-03)
8+
9+
<!-- post-merge: checked-begin -->
10+
- astubbs/parallel-consumer#426 - `producerConfig`: PC builds its producer from configuration with the
11+
default constructor, the caller's `transactional.id` included. The only rung recovery needs, because
12+
re-initialising a replacement under the same id is what fences the producer it replaces.
13+
- astubbs/parallel-consumer#410 - recovery: detection, the aborted-transaction ledger and replay, the
14+
replacement with backoff, observability, the broker IT. Stacked on astubbs#426.
15+
- astubbs/parallel-consumer#420 - the rest of producer ownership: a derived prefix-free
16+
`transactional.id`, a `ProducerFactory` with an enforced contract, configuration redaction, deprecating
17+
the instance option, migrating the examples. Stacked on astubbs#410.
18+
<!-- post-merge: checked-end -->
19+
720
<!-- post-merge: checked -->
821
The feature was implemented by astubbs/parallel-consumer#410 against the plan
922
`docs/plans/2026-09-02-001-feat-recoverable-producer-fencing-plan.md`, which owns the requirements, the

docs/inflight/issue-response-225.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,8 @@ the shape to copy, but it is not the "abort, then rejoin" this issue described.
2323
**"Rejoin" was not the hard part; the producer was.** Recovery needs a *new* producer, and PC cannot
2424
build one: `ParallelConsumerOptions` holds a finished `Producer` instance, and a `KafkaProducer`'s
2525
configuration cannot be read back out of it. So the change is an ownership change — PC takes producer
26-
configuration and builds the producer through an overridable factory, Kafka Streams' `KafkaClientSupplier`
27-
shape — not an exception swap. The producer-instance option stays, deprecated, without recovery, with
28-
its removal queued for the major after the one that deprecates it.
26+
configuration and builds the producer itself, the shape of Kafka Streams' `KafkaClientSupplier` — not an
27+
exception swap. The producer-instance option stays, without recovery.
2928

3029
**"Whether rejoin is expressible in PC's lifecycle" — the part this issue said to investigate first —
3130
is answered, and no state-machine addition is needed.** The produce/commit lock pair already gives the

docs/inflight/issue-response-411.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,10 @@ Two honest caveats:
4949

5050
- **It will not reach your configuration unchanged.** Recovery requires Parallel Consumer to build the
5151
producer itself, from configuration, so it can build a replacement. Supplying a `Producer` instance —
52-
the only option today, and what you were doing — stays supported and deprecated, but cannot recover,
52+
the only option today, and what you were doing — stays supported, but cannot recover,
5353
because a `KafkaProducer`'s configuration cannot be read back out of it.
5454
- **The loop itself is a separate defect**, tracked in
55-
`docs/inflight/bug-411-wrapped-send-failure-spins-forever.md`. Whether to close it on the deprecated
55+
`docs/inflight/bug-411-wrapped-send-failure-spins-forever.md`. Whether to close it on the instance
5656
path as well — so the condition at least terminates rather than spinning — is an open decision.
5757

5858
Thanks for the original report and for the fix attempt. The diagnosis in it was right; it was the

docs/plans/2026-09-02-001-feat-recoverable-producer-fencing-plan.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ execution: code
1111

1212
# Recoverable Producer Fencing - Plan
1313

14+
> **Re-cut into a stack, 2026-09-03.** The producer-ownership half of this plan ships in stages rather than in one PR: `producerConfig` with the default constructor is the base rung (astubbs/parallel-consumer#426); recovery (astubbs/parallel-consumer#410) stacks on that rung alone and rebuilds under the `transactional.id` the caller sets; the derived prefix-free id, the `ProducerFactory` and its contract, configuration redaction, and the deprecation of the instance option with the example migration are astubbs/parallel-consumer#420, stacked above recovery. Where the text below says the factory or the derived id is what recovery rebuilds through, read: the same configuration, id included - the mechanisms and decisions are unchanged, only the order they land in.
15+
1416
## Goal Capsule
1517

1618
**Objective.** Give Parallel Consumer a transactional producer it owns, so that when the broker tells it the producer is no longer usable, PC closes that producer, builds a replacement, and carries on — instead of spinning forever on the produce path or shutting the instance down on the commit path. Tracked as astubbs#225.
@@ -331,6 +333,7 @@ U1 → U2 → U3 → U5 → U6 → U7; U4 has no dependencies and may be built a
331333
- **Requirements:** R1, R2, R3, R7, R16, R17, R19, R21; KTD1, KTD9.
332334
- **Dependencies:** none.
333335
- **Files:** `parallel-consumer-core/src/main/java/bz/stub/parallelconsumer/ParallelConsumerOptions.java`, new `parallel-consumer-core/src/main/java/bz/stub/parallelconsumer/ProducerFactory.java`, new `parallel-consumer-core/src/main/java/bz/stub/parallelconsumer/internal/ProducerConfigRedaction.java`, `docs/refactoring.md` (the entry exists; only re-check the release it names), tests in new `parallel-consumer-core/src/test/java/bz/stub/parallelconsumer/ParallelConsumerOptionsProducerConfigTest.java` and new `parallel-consumer-core/src/test/java/bz/stub/parallelconsumer/internal/ProducerConfigRedactionTest.java`.
336+
<!-- file-refs: N/A - the factory, derivation and redaction files are astubbs#420's, stacked above this PR; see the re-cut note at the head of this plan -->
334337
- **Approach:**
335338
1. Add `producerConfig` (`Map<String, Object>`) and `producerFactory` (`ProducerFactory<K, V>`, `@Builder.Default` to a `KafkaProducer` constructor) fields; `isProducerSupplied()` becomes "instance or configuration present".
336339
2. `validate()`: both `producer` and `producerConfig` set fails with a message naming `Fields.producer` and `Fields.producerConfig` (R17); transactional mode with neither fails as today; `producer` set alone logs the single R19 WARN naming `producerConfig` plus `producerFactory`, the absence of recovery, and the removal release.
@@ -353,6 +356,7 @@ U1 → U2 → U3 → U5 → U6 → U7; U4 has no dependencies and may be built a
353356
- **Requirements:** R2, R4, R5, R6; KTD2, KTD8.
354357
- **Dependencies:** U1.
355358
- **Files:** `parallel-consumer-core/src/main/java/bz/stub/parallelconsumer/internal/PCModule.java`, `parallel-consumer-core/src/main/java/bz/stub/parallelconsumer/internal/ProducerWrapper.java`, new `parallel-consumer-core/src/main/java/bz/stub/parallelconsumer/internal/TransactionalIdDerivation.java`, `parallel-consumer-core/src/test/java/bz/stub/parallelconsumer/internal/PCModuleTestEnv.java` (constructor change follow-through), tests in new `parallel-consumer-core/src/test/java/bz/stub/parallelconsumer/internal/TransactionalIdDerivationTest.java` and new `parallel-consumer-core/src/test/java/bz/stub/parallelconsumer/internal/PcBuiltProducerTest.java`.
359+
<!-- file-refs: N/A - the factory, derivation and redaction files are astubbs#420's, stacked above this PR; see the re-cut note at the head of this plan -->
356360
- **Approach:**
357361
1. `TransactionalIdDerivation`: `prefixFor(groupId)` returns `pc-<L>-<groupId>-`; `derive(groupId, instanceUuid)` appends the UUID; `resolve(producerConfig, commitMode, groupId, uuid)` returns a copy of the config with the id set (transactional mode) or removed (otherwise), emitting the R5 WARN when the caller had set one.
358362
2. `PCModule`: one `UUID` per module instance; `producerWrap()` builds from the instance when present, otherwise resolves the config and calls the factory; new `replacementProducerWrap()` returns `Optional<Supplier<ProducerWrapper<K, V>>>`, present only on the configuration path, each call resolving the same config and calling the factory again.

docs/refactoring.md

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,6 @@ refactors below, which are non-breaking and can land at any point in any line.
118118
(`public void setCommitInterval`, `private final Duration defaultMessageRetryDelay`,
119119
`isUsingTransactionalProducer`) **and retire the temporary Kafka-compat work-around flag**
120120
(`ignoreReflectiveAccessExceptionsForAutoCommitDisabledCheck`) - `ParallelConsumerOptions.java`.
121-
- **Remove the `producer` instance option**, once the configuration-plus-factory path deprecating it
122-
has landed - `ParallelConsumerOptions.java`. Queued rather than left open-ended because the two
123-
paths are not equivalent: only the PC-built one can rebuild a producer the broker has invalidated,
124-
so keeping both means every future change to producer handling is written twice, once on a path
125-
that recovers and once on a path that cannot. Deprecated by
126-
[`docs/plans/2026-09-02-001-feat-recoverable-producer-fencing-plan.md`](plans/2026-09-02-001-feat-recoverable-producer-fencing-plan.md)
127-
(astubbs#225); the deprecation javadoc names the major AFTER the release that ships the deprecation, so the option is not removed in the same version that deprecates it.
128121
- **Remove the JStream API** (deprecate first) - design ref
129122
`origin/refactor/deprecate-jstream` @8a8f6508.
130123
- **Rename the enum to the standard pattern** (public enum rename) -

0 commit comments

Comments
 (0)