fix(cdc): prevent dead-letter records from re-entering replica apply - #197
fix(cdc): prevent dead-letter records from re-entering replica apply#197seonghobae wants to merge 3 commits into
Conversation
|
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:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review handoff for #181: explicit pre-applier DLT terminal quarantine addresses poison-loop prevention. Remaining #181 governance items stay open: retention/deletion, ACL and authorized redrive, encryption, topic/environment separation, payload/header bounds, and idempotent/fenced replay. This PR should not claim the whole DLT lifecycle contract. |
Purpose
Advance #181 with a second path-disjoint reliability/data-governance slice from protected
develop@622e5e6c3d534f230c390f10e3832efadfc01825. The protected replica listener subscribes by default toxtrmetl-cdc\..*, while exhausted failures are published to<source-topic>.DLT. That regular expression also matches generated.DLTtopics, so dead-letter records are currently delivered back into the live replica listener instead of being explicitly terminal at the consumer boundary.Under today's default
replica.tables=processed_data,ProcessedDataReplicaApplierderives the final topic segmentDLT, finds no matching allowed table, and returns without JDBC work. That means the default tree does not currently prove an actual recursive DLT chain. However, terminal dead-letter behavior depends implicitly on an unrelated downstream table allow-list and applier implementation, and a broad/custom configuration or future applier change can break that accidental safety property. This slice makes the terminal quarantine boundary explicit before any applier is invoked.This Draft is independent of #192's Kafka error-handler/header files and does not choose DLT retention, ACLs, encryption, operator redrive, or tenant policy.
Exact current identity
develop@622e5e6c3d534f230c390f10e3832efadfc01825;fix/cdc-dlt-poison-loop-622e5e6;56a39482e0d023dd276940157677ca24235833cf;c88bf401f819a9cc7c9735746c4b55ae517b764f;b140a449ffb64c4ec36d6f430ba93bc462e650bd;Every check/review from a predecessor source head is historical and does not transfer.
RCA
.DLTderivatives.TDD evidence
RED —
56a39482e0d023dd276940157677ca24235833cfThe fail-first commit changes only
CdcReplicaConsumerTest. It sends a realConsumerRecordonxtrmetl-cdc.public.processed_data.DLTto the production consumer and requires zero interactions with both replica appliers.CI
31348479543, macOS job93334765167, checked out synthetic merge35867415cc4edc244ad22f63f64ef66db1af2f05(Merge 56a3948... into 622e5e6...). The ETL module passed; CDC ran 107 tests with exactly the new DLT test failing and zero errors/skips. Mockito reported the one real interaction atCdcReplicaConsumer.onMessage:processedDataReplicaApplier.apply(...). The RED therefore reached the intended live-routing boundary, not setup/import/fixture code.The RED also falsified the stronger initial hypothesis that the protected default necessarily creates
.DLT.DLTchains: source inspection shows the current processed-data applier treatsDLTas the table suffix and returns when that table is not allow-listed. The accepted remediation is therefore defense-in-depth terminal routing, not a claim that default production is already recursively republishing.GREEN —
c88bf401f819a9cc7c9735746c4b55ae517b764faf9b71d334f7cf265893b5286e83c67a79435048names the shared terminal suffix asReplicaTopics.DEAD_LETTER_SUFFIX;c88bf401f819a9cc7c9735746c4b55ae517b764fadds an explicit first routing guard: a.DLTrecord returns before either applier is invoked;Current-head CI
31348644772completed successfully on macOS, Ubuntu and Windows. macOS job93335234861checked out synthetic mergeb140a449ffb64c4ec36d6f430ba93bc462e650bd(Merge c88bf401... into 622e5e6...), ranCdcReplicaConsumerTest4/4 green, CDC 107/107 green, ETL 275/275 green, gateway tests green and completed the full reactor withBUILD SUCCESS.The guard remains safe even if an operator supplies a broad custom topic pattern. Merely narrowing the default regex was rejected because it would leave custom patterns able to reintroduce dead-letter records.
Current exact-head workflow evidence
For exact source head
c88bf401f819a9cc7c9735746c4b55ae517b764f:31348644772: aggregate success on macOS/Ubuntu/Windows, but source execution is synthetic-merge-only;31348644789: success;31348644788: success;31348644769: success;31348644759: aggregate success, not accepted as complete security evidence;Security Scan completeness RCA
Hard Trivy job
93335221082checked out the same synthetic mergeb140a449..., not literal source headc88bf401.... It also printed:before reporting zero CRITICAL/HIGH/MEDIUM findings. Issue #196 owns this scanner-completeness defect. A zero-findings SARIF cannot prove absence of vulnerable BOM-managed Maven dependencies when the scanner itself says dependency resolution is incomplete. This Security Scan is therefore
incomplete / non-passingfor mightyETL acceptance despite the aggregate green conclusion.The same CI run independently reproduces the inherited JaCoCo quality-control defect: report/check each say
Analyzed bundle 'etl-service' with 0 classesand then report coverage checks met. #162/#164 owns that repair; this current-head CI is not valid non-vacuous 100% owned-production coverage evidence.These evidence gaps are independent: fixing DLT routing does not fix source identity, Maven scanner completeness, Jackson baseline, or coverage non-vacuity. Do not copy shared fixes into this three-file feature slice or reinterpret aggregate green as accepted evidence.
Scope
This slice does not change dead-letter payload/header diagnostic retention (#192), broker retention/ACL/encryption, replay/redrive authority, schema compatibility, poison-message operator workflow, or tenant policy. #181 remains the canonical broader DLT lifecycle/data-governance issue.
Merge boundary
Keep Draft. Merge only after the unchanged exact source head has accepted literal-source deterministic/security evidence, complete same-revision dependency/vulnerability evidence with no #196 incompleteness, non-vacuous owned-production coverage under #162/#164 or equivalent accepted repair, zero valid unresolved findings, current DLT governance/traceability where applicable, and qualifying independent non-author approval where governance requires it. Pending, failed, incomplete-scanner, predecessor-head, status-only, skipped-required, or synthetic-merge-only evidence is not passing.