feat(source): v1 hardening — tx-Term contract, dependency isolation, test-gap closure - #203
Merged
Merged
Conversation
…test-gap closure P2-9 (decided + implemented + documented): settling Term directly on a transactional subscription is rejected with the new exported ErrTermInsideTransaction — a DLQ produce outside the open EOS session would break atomicity with the consumed offset. The supported poison path is inside Transactional.Begin: produce to the dead-letter topic via the handed Tx and return success, committing DLQ record and offset atomically. Documented in kafka.go, README divergence section, Begin doc; pinned by unit tests for both the rejection and the Begin DLQ pattern. P2-7: Hopper.exitErr drops its ignored ctx parameter. P2-8: the RedPanda integration leg moves to its own test-only nested module source/kafka/integration with its own go.mod, so testcontainers-go, redpanda, and kadm no longer appear in source/kafka's require block and downstream module graphs stop inheriting them. Workflow matrix and mage Integration target follow the new path; the -tags integration convention and Docker-skip guard are kept. P2-10/P2-11 comments landed with checkpoint 1 (requeue cost model; poll-error record discard). Test gaps closed: drain-on-cancel with mid-flight backend settle (Close+cancel race: exactly one settle, zero post-Close yields, no stranded goroutines); duplicate-settle idempotence on both adapters (memsource ledger + kafka marks); rebalance revoke-with-in-flight end-to-end on RedPanda (two members join/leave, 40 records delivered exactly once across the group); tx-Term pair above. Also fixes vet shadow warnings in checkpoint-1's drain tests.
The nested module's manifest was created after the checkpoint branch's initial commit and missed the git add, so CI's source/kafka/integration leg had no module definition. tidy run via GOFLAGS=-tags=integration so the build-tag-gated testcontainers/redpanda/kadm imports keep their requirements.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checkpoint 3/5 — P2 decisions + dependency isolation on the road to source/v1.0.0
Stacked after #199 (merged) and #200 (merged); rebased onto current main.
P2-9 — transactional inlet + Term/DLQ: decided, implemented, documented
Direct
Settle(Term)on aWithTransactionalsubscription is rejected with the new exportedErrTermInsideTransaction(a DLQ produce outside the open EOS session would break atomicity with the offset mark). The supported poison path is insideTransactional.Begin: produce the rejected record to the DLQ topic via the handedsource.Tx, return success — DLQ record + consumed offset commit atomically. Documented in package doc, README divergence section, and Begin's doc comment.P2-7 —
Hopper.exitErrdrops its ignored ctx parameter (unexported; no API change).P2-8 — integration deps isolated out of downstream module graphs
The RedPanda leg moves to its own test-only nested module
source/kafka/integration(own go.mod/go.sum).source/kafka/go.modnow requires only franz-go(+kmsg)+source; testcontainers-go/redpanda/kadm appear nowhere downstream. Workflow matrix + mage Integration target follow the new path;-tags integrationconvention and Docker-skip guard kept.Test gaps closed (E)
Also fixes govet/shadow warnings in checkpoint 1's drain tests.
Evidence (local, this branch)
Regressions vs baseline: none. API additions beyond approved list:
ErrTermInsideTransactiononly (required by the P2-9 decision).