fix(cdc): acknowledge Kafka delivery before offset progress - #139
Draft
seonghobae wants to merge 17 commits into
Draft
fix(cdc): acknowledge Kafka delivery before offset progress#139seonghobae wants to merge 17 commits into
seonghobae wants to merge 17 commits into
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
This was referenced Aug 9, 2026
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
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.
Buyer-visible reliability gap
The protected
developCDC path previously allowed Debezium source progress to become decoupled from Kafka acknowledgement. The bounded slice now requires successful Kafka acknowledgement beforeRecordCommitter.markProcessed(...), retries one terminal publication failure without advancing the source record, and adds a finite application-level guard so a returned Kafka send future cannot stall a CDC batch indefinitely.This remains an independent, path-disjoint slice from protected
develop; it does not depend on or deepen the invalid durable-job stack and does not mutate separately leased repositories.Exact current identity
develop;622e5e6c3d534f230c390f10e3832efadfc01825;279a29eb9033c40e021750ee81af520fb76f96e8;7f7998c9b1f4301b6c073f5f6fc2c7be4e252a8c;Every check, review, approval, or scanner result from an older head or base is stale and does not transfer.
Delivery contract
For each destination-bearing Debezium event, the live batch path now:
kafkaPublishSuccessonly after acknowledgement;KafkaExceptionafter the second failed/timed-out attempt without marking the current record or batch finished.The producer also requires
acks=all, idempotence, at most five in-flight requests per connection, a 60-second defaultdelivery.timeout.ms, and a 30-second defaultmax.block.ms. The 65-second future-wait guard deliberately exceeds the default producer delivery timeout so normal Kafka terminal completion can surface first. It is a bound on waiting after a future is returned, not a claim about total end-to-end attempt duration.This remains an at-least-once/replay-tolerant boundary, not an end-to-end exactly-once claim.
TDD evidence
Initial acknowledgement boundary
Historical RED
f7ca5f149df959d03ff330ea0e374bc8fcb031e4introduced the acknowledgement/committer contract before production implementation. Later GREEN commits implemented acknowledged batch publication, retry/failure counters, interrupt propagation, producer durability controls, and truthful public lifecycle documentation. GitHub Code Quality's valid redundant-retry finding was refactored; its later claim that the unchecked@throws KafkaExceptiondocumentation was impossible was incorrect because the runtime path does throw that unchecked exception, and the bot resolved that thread.RED — non-terminating future
Commit
85e9f8a3369e39634b57295c24f52c9e89bf5917addedCdcKafkaPublishTimeoutTestbefore production changed. PR-triggered CI run31276261918, Ubuntu job93150176837, compiled the CDC production and test sources and ran the intended boundary. The CDC module ran 116 tests with exactly one failure and zero errors:timesOutHungKafkaAcknowledgementsWithoutAdvancingOffsetsexpected timedget(65000, MILLISECONDS)calls to fail closed, but production still used unboundedCompletableFuture.get(). Existing acknowledged-publication tests remained green. This is valid fail-first evidence rather than a fixture/import/setup defect.GREEN — finite future wait
Commit
c56dce4586ca330e7b199a2651010eb50283b5e5added the 65-second timed future wait and routesTimeoutExceptionthrough the existing retry/fail-closed path. PR-triggered CI run31276397082, Ubuntu job93150509726, completed the full Maven reactor successfully:CdcKafkaPublishTimeoutTestpassed 1/1,CdcKafkaPublishAcknowledgementTestpassed 8/8, and the CDC module passed 116 tests with zero failures/errors/skips.6e89ee2a4d6fe372f8e6a22309f72aee939f87a8,4848e7b822e4289a472c5065dba4aced23d16a90, and exact current head279a29eb9033c40e021750ee81af520fb76f96e8align authoritative doctoring, operations guidance, andCHANGELOG.mdwith the finite-wait contract. APA 7 primary references remain recorded indocs/doctoring/cdc-kafka-acknowledged-delivery.md.Exact-current-head workflow state
For exact current head
279a29eb9033c40e021750ee81af520fb76f96e8at the latest refresh, every exposed aggregate workflow completed successfully:31276544782: success;31276544774: success;31276544791: success;31276544800: success;31276544771: success;These protected-branch workflow definitions still predate the literal-head source-checkout controls carried by #121. The successful CI aggregate is not accepted as literal-head source proof: macOS job
93150913818checked out synthetic merge7f7998c9b1f4301b6c073f5f6fc2c7be4e252a8c(Merge 279a29e... into 622e5e6...) rather than literal head279a29e.... Therefore aggregate green does not satisfy the repository's exact-source acceptance rule.Current review reinspection shows zero unresolved review threads. Formal reviews are COMMENTED only; there is no qualifying independent non-author
APPROVEDreview anchored to exact current head279a29eb9033c40e021750ee81af520fb76f96e8.Follow-on lifecycle RCA
Issue #141 records a separate buyer-visible lifecycle gap found after this publication slice:
CdcService.stop()still clearsdebeziumEngineandengineTaskimmediately after requesting Debezium close, so stopped-state observability can precede asynchronous engine completion. The defect is present on this exact head and protecteddevelop. Its preferred bounded repair is feasible in source design but remainsdefer_until_triggerfor mutation until a trusted literal-head source-execution path exists; do not fold an unverified lifecycle repair into this already-reviewed acknowledgement slice or manufacture exact-head evidence by copying workflow controls here.Dependency and merge boundary
PR #121 carries repository-local literal-head CI/SBOM and source-identity controls but is not integrated into protected
develop; its scanner-control dependency remains owned by the separately leasedContextualWisdomLab/.githubloop. This PR must not locally copy or bypass those controls to manufacture acceptable evidence.Keep this PR Draft. Do not merge until the unchanged literal current head has complete required literal-head CI/security/SAST/SBOM evidence under integrated protected controls, zero actionable review threads, a qualifying independent non-author exact-head approval, and branch protection permits merge without bypass. Queued, pending, skipped-required, neutral-required, absent, cancelled, failed, stale-head, predecessor-head, or synthetic-merge-only evidence is not passing.