Skip to content

fix(cdc): prevent dead-letter records from re-entering replica apply - #197

Draft
seonghobae wants to merge 3 commits into
developfrom
fix/cdc-dlt-poison-loop-622e5e6
Draft

fix(cdc): prevent dead-letter records from re-entering replica apply#197
seonghobae wants to merge 3 commits into
developfrom
fix/cdc-dlt-poison-loop-622e5e6

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Purpose

Advance #181 with a second path-disjoint reliability/data-governance slice from protected develop@622e5e6c3d534f230c390f10e3832efadfc01825. The protected replica listener subscribes by default to xtrmetl-cdc\..*, while exhausted failures are published to <source-topic>.DLT. That regular expression also matches generated .DLT topics, 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, ProcessedDataReplicaApplier derives the final topic segment DLT, 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

  • protected/live base: develop@622e5e6c3d534f230c390f10e3832efadfc01825;
  • branch: fix/cdc-dlt-poison-loop-622e5e6;
  • fail-first source head: 56a39482e0d023dd276940157677ca24235833cf;
  • exact current GREEN source head: c88bf401f819a9cc7c9735746c4b55ae517b764f;
  • current synthetic merge preview: b140a449ffb64c4ec36d6f430ba93bc462e650bd;
  • GitHub mergeability: true;
  • Draft: true.

Every check/review from a predecessor source head is historical and does not transfer.

RCA

  • Immediate cause: the default replica topic pattern matches both live CDC topics and their .DLT derivatives.
  • Technical root cause: terminal dead-letter quarantine and live replication input are not separated at the consumer boundary; current safety is an incidental consequence of downstream table-selection behavior.
  • Control failure: existing consumer tests prove ordinary data/schema routing but never prove a DLT record is terminal before applier invocation.
  • Product impact: dead-letter records consume live-listener work and their terminal semantics can silently change when custom topic/table policy or applier behavior changes. An explicit guard keeps poison-message quarantine independent of those mutable details.

TDD evidence

RED — 56a39482e0d023dd276940157677ca24235833cf

The fail-first commit changes only CdcReplicaConsumerTest. It sends a real ConsumerRecord on xtrmetl-cdc.public.processed_data.DLT to the production consumer and requires zero interactions with both replica appliers.

CI 31348479543, macOS job 93334765167, checked out synthetic merge 35867415cc4edc244ad22f63f64ef66db1af2f05 (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 at CdcReplicaConsumer.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.DLT chains: source inspection shows the current processed-data applier treats DLT as 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 — c88bf401f819a9cc7c9735746c4b55ae517b764f

  • af9b71d334f7cf265893b5286e83c67a79435048 names the shared terminal suffix as ReplicaTopics.DEAD_LETTER_SUFFIX;
  • c88bf401f819a9cc7c9735746c4b55ae517b764f adds an explicit first routing guard: a .DLT record returns before either applier is invoked;
  • ordinary schema-change, data-topic and null-topic routing remains unchanged;
  • the consumer's class, constructor and message-routing contract now have beginner-readable Javadoc explaining the terminal quarantine boundary.

Current-head CI 31348644772 completed successfully on macOS, Ubuntu and Windows. macOS job 93335234861 checked out synthetic merge b140a449ffb64c4ec36d6f430ba93bc462e650bd (Merge c88bf401... into 622e5e6...), ran CdcReplicaConsumerTest 4/4 green, CDC 107/107 green, ETL 275/275 green, gateway tests green and completed the full reactor with BUILD 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:

  • CI 31348644772: aggregate success on macOS/Ubuntu/Windows, but source execution is synthetic-merge-only;
  • Dependency Review 31348644789: success;
  • CycloneDX SBOM 31348644788: success;
  • SAST Semgrep 31348644769: success;
  • Security Scan 31348644759: aggregate success, not accepted as complete security evidence;
  • the conditional self-hosted CI job is skipped and not counted as positive evidence.

Security Scan completeness RCA

Hard Trivy job 93335221082 checked out the same synthetic merge b140a449..., not literal source head c88bf401.... It also printed:

Dependency version cannot be determined. Child dependencies will not be found.

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-passing for 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 classes and 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.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dd97b1be-a480-42f6-a731-d304132ac0a2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: medium Normal-priority or P2 work status: draft Draft pull request type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant