Repository navigation
[Improve][Connector-V2] Move Debezium ownership from connector-cdc-base to each CDC connector - #11639
DanielLeens wants to merge 12 commits into
Conversation
What Problem This PR Solves
I. Code Change Review1.1 Core Logic AnalysisExact change descriptionThe core packaging changes are in:
Before / after snippetsBefore, <dependency>
<groupId>io.debezium</groupId>
<artifactId>debezium-api</artifactId>
<version>${debezium.version}</version>
</dependency>
<dependency>
<groupId>io.debezium</groupId>
<artifactId>debezium-embedded</artifactId>
<version>${debezium.version}</version>
</dependency>After, the base jar only compiles against Debezium: <dependency>
<groupId>io.debezium</groupId>
<artifactId>debezium-api</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>io.debezium</groupId>
<artifactId>debezium-embedded</artifactId>
<scope>provided</scope>
</dependency>and each connector module now declares the runtime Debezium artifacts it packages itself. Key findings
Deep logic analysisThe relevant packaging/runtime path is: For openGauss specifically: Local verification results used for this review:
1.2 Compatibility ImpactJudgment: Partially incompatible Impacts:
Migration note:
1.3 Performance / Side-Effect Analysis
1.4 Error Handling and LoggingI did not find a new source-side blocker in the current head. II. Code Quality Assessment2.1 Coding StandardsThe comments are strong throughout the packaging changes, especially around the base-jar classloader constraint and the openGauss/PostgreSQL coupling. 2.2 Test Coverage and Test StabilityThe new adapter tests cover the most important guardrails:
Test stability conclusion: Stable
2.3 Documentation UpdatesThe incompatible-change note was updated in both:
That matches the user-visible packaging change for third-party connector builders. III. Architecture Rationality3.1 Elegance of the SolutionThis is a long-term packaging fix rather than a workaround. It addresses the shared-classloader root cause directly. 3.2 MaintainabilityThe ownership model is clearer after this change because each connector now declares the Debezium runtime it ships, and the adapter tests give a concrete drift alarm. 3.3 ExtensibilityThe new SPI gives the project a clean seam for future runtime wiring or version-specific logic if that becomes necessary later. 3.4 Historical Version CompatibilityFor in-tree connectors, compatibility remains effectively unchanged at runtime. The breaking surface is limited to external connectors that previously relied on the base jar's transitive Debezium packaging. IV. Issue SummaryNo formal issues found in the current head. V. Merge RecommendationConclusion: Can be merged
Overall assessment: Remaining gates:
|
SEZ9
left a comment
SEZ9
left a comment
There was a problem hiding this comment.
Thanks for working on this. I re-reviewed the latest head afb0fada4b16 from scratch.
What this PR fixes
- User pain: The
debezium.versionproperty added in #10799 is cosmetic —connector-cdc-baseships 294io.debeziumcore/embedded classes at compile scope and, becauseAbstractPluginDiscovery#filterPluginJarforce-loads that jar into every CDC plugin's class loader, it pins a single project-wide Debezium version no connector can override. - Fix approach: Flip
debezium-api/debezium-embedded/zstd-jnitoprovidedinconnector-cdc-base/pom.xml, make each connector (e.g.connector-cdc-mongodb) declare the Debezium runtime it ships plus aDebeziumAdapterpinning its version, and addPatchedDebeziumClassesTestto keep the hand-maintained shade exclusion list (the five patchedio.debeziumclasses) in sync so duplicates can't silently reappear. - One-line summary: A packaging-only refactor that makes each connector's jar the sole source of Debezium classes while
connector-cdc-basestays a Debezium-free shared jar, with docs (EN/ZH incompatible-changes) honestly capturing the third-party breaking change and the remaining SPI limitation — the parts I want to verify are the shade filters inconnector-cdc/pom.xmlandconnector-cdc-opengauss/pom.xmlreferenced by the new test, which are not visible in this diff excerpt.
Review conclusion
Conclusion: LGTM, can merge
Nice work — thanks again for the contribution. Happy to discuss any of the above.
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
What Problem Does This PR Solve?
- User pain point:
connector-cdc-baseships the Debezium runtime (debezium-api,debezium-embedded,zstd-jni) at compile scope, and becauseAbstractPluginDiscovery#filterPluginJar/selectPluginJarforce-loadconnector-cdc-base-*.jarinto the class loader of every CDC connector plugin whose name contains "cdc", that single shared jar pins one project-wide Debezium version. No individual connector (MySQL, Postgres, Oracle, SQL Server, MongoDB, openGauss, Vitess) can move independently. - Fix approach: flip
debezium-api/debezium-embedded/zstd-jnitoprovidedscope inconnector-cdc-base/pom.xml, centralize the version+exclusion management in theconnector-cdcparent'sdependencyManagement(interpolated per-module against each connector's owndebezium.versionproperty), have every Debezium-based connector declare its own runtime Debezium dependencies, add a lightweightDebeziumAdapterSPI + per-connector adapter and guardrail tests, and document the packaging change indocs/en/docs/zh. - One-sentence summary: this is a packaging-only refactor that moves Debezium runtime ownership from the shared
connector-cdc-basejar into each CDC connector's own jar, with no functional/runtime behavior change for in-tree connectors today.
1. Code Change Review
1.1 Core Logic Analysis
Coverage scope: I fully traced the complete diff (31 files) plus the surrounding, unmodified source needed to validate its premises: AbstractPluginDiscovery (plugin discovery/classloader merge), DebeziumAdapter/DebeziumAdapterFactory (pre-existing SPI from #10799, unmodified here), the pre-PR and post-PR pom.xml of all 8 connector-cdc-* modules (including the untouched connector-cdc-tidb), seatunnel-connectors-v2/pom.xml, seatunnel-format-compatible-debezium-json/pom.xml, seatunnel-dist/pom.xml and assembly-seatunnel-bin.xml, and every connector-cdc-*-e2e/pom.xml. This is full-depth coverage, not a sample.
Change description: connector-cdc-base/pom.xml drops its dependencyManagement pins for debezium-api/debezium-embedded/zstd-jni and adds <scope>provided</scope> to its own <dependencies> entries for them. connector-cdc/pom.xml (the parent) gains the same dependencyManagement entries (version + exclusions, unchanged content, just relocated) and a maven-shade-plugin <configuration><filters combine.children="append"> block that excludes the five patched io.debezium.* classes from debezium-core. Seven Debezium-based connectors (mysql, postgres, oracle, sqlserver, mongodb, opengauss, vitess) each get a direct (compile-scope) debezium-api/debezium-embedded/zstd-jni dependency, a <connector>DebeziumAdapter implementing the pre-existing DebeziumAdapter SPI, a META-INF/services registration, and an adapter test. connector-cdc-tidb (Debezium-free, uses tikv-client-java) is correctly left untouched.
Runtime path verification — I checked whether the normal execution path is affected:
DebeziumAdapterFactory.getAdapter(...)has zero production call sites in this diff or indev— it is only invoked from the new test classes. TheDebeziumAdapterinterface Javadoc itself says "concrete wiring... will be added in follow-up PRs." So the entire adapter SPI added by this PR is currently inert scaffolding; it changes no job-execution behavior.- The only runtime-observable change is classloading/packaging: previously
connector-cdc-base.jarcarried the Debezium classes into every CDC connector's merged classloader; now the connector's own jar does. Since every in-tree connector still resolvesdebezium.version=1.9.8.Final(unchanged), the set of classes visible to a running CDC job is identical before and after this PR. This matches the PR's own compatibility claim, and I independently confirmed it rather than taking it on faith. connector-cdc-tidb:TiDBSourceimplementsSeaTunnelSourcedirectly and only imports plainOption/enum holders fromconnector-cdc-basewith no Debezium type in their signature. TiDB is loaded withconnector-cdc-base.jarmerged into its classloader (perAbstractPluginDiscovery's "cdc"-name matching, which is unconditional and not Debezium-aware), but never touches a Debezium-referencing class path, so it is unaffected byconnector-cdc-baseno longer shipping Debezium. This was worth checking explicitly — a per-connector Debezium move could plausibly strand a non-Debezium CDC connector that shares the same force-merged base jar — but it does not.
Shade-filter chain (the mechanism worth verifying, not just accepting from a diff excerpt): I traced the full Maven plugin inheritance for the six connectors that don't locally override shade config (mysql, postgres, oracle, sqlserver, mongodb, vitess) — none of them declare their own maven-shade-plugin <execution>, so they inherit the shade <execution> binding from higher ancestors and the <filters combine.children="append"> block newly added in connector-cdc/pom.xml, which appends (does not replace) the debezium-core exclusion filter to whatever base filters exist upstream. connector-cdc-opengauss is the one exception: it already had its own local shade <execution> (for relocating org.postgresql) predating this PR, and a child execution's own <filters> (without combine.children="append" on its side) overrides rather than merges with the inherited one — which is exactly why the PR manually restates the same debezium-core exclusion filter inside connector-cdc-opengauss/pom.xml's own execution, with a comment explaining why. I verified this restatement is present and correct.
openGauss/Postgres coupling: connector-cdc-opengauss has a pre-existing (unchanged by this PR) plain compile-scope dependency on connector-cdc-postgres, and deliberately registers no DebeziumAdapter of its own, relying on the inherited PostgresDebeziumAdapter + META-INF/services merge. OpengaussDebeziumAdapterTest verifies exactly one adapter resolves for PostgresConnector. This matches the documented intent and is correctly guarded.
1.2 Compatibility Impact
Judgment: Partially incompatible
- API / config / checkpoint / serialization: unchanged for all in-tree connectors — no
Optionrenamed or removed, no default changed, no protocol/serialization format touched. - Runtime behavior: unchanged for in-tree connectors, because every connector still resolves the same
debezium.version=1.9.8.Finalbaseline before and after this PR (verified above). - Historical behavior: unaffected — the
DebeziumAdapterSPI is inert (no call sites in production code), so there is no new runtime decision path that could diverge from prior behavior. - Breaking surface: third-party/out-of-tree CDC connectors that built against
connector-cdc-baseand relied on it to transitively supplydebezium-api/debezium-embeddedmust now declare those themselves. This is honestly and specifically documented in bothdocs/enanddocs/zhincompatible-changes.md, including the caveat about how far connector Debezium versions may safely diverge givenconnector-cdc-base's own patched-class usage. Good documentation discipline here.
1.3 Performance / Side-Effect Analysis
- No CPU/memory/GC change at runtime.
- Packaging side effect (documented in the PR): the Debezium runtime (plus its Kafka Connect/compression/Jetty transitive deps) is now duplicated into each Debezium-based connector's jar instead of being shared via
connector-cdc-base, increasing dist size. Acceptable, explicitly called out. - Minor, purely positive side effect not mentioned in the PR: connectors that are CDC-named but Debezium-free (i.e.,
connector-cdc-tidbtoday) no longer incidentally carry the full Debezium runtime in their merged classloader, sinceconnector-cdc-baseitself no longer ships it. Dead weight removed, no functional change.
1.4 Error Handling and Logging
No new error-handling paths are introduced in production code (the SPI's IllegalStateException behavior in DebeziumAdapterFactory is pre-existing and unmodified). Nothing to flag.
2. Code Quality Assessment
2.1 Coding Standards
Comments are unusually thorough and explain why, not just what — e.g., the classloader-merge rationale in connector-cdc-base/pom.xml, the shade-filter-override rationale in connector-cdc-opengauss/pom.xml, and the Javadoc on every new adapter class and test. Every new class/field/method has a comment. This meets a high bar.
2.2 Test Coverage and Test Stability
Test stability rating: Stable. All 8 new test classes (PatchedDebeziumClassesTest + one *DebeziumAdapterTest per connector) are deterministic: they read compiled .class files or META-INF/maven/.../pom.properties off the classpath, use ServiceLoader, and make no use of Thread.sleep, network I/O, fixed ports, floating point, or shared mutable static state. No flakiness risk.
However, there is a real coverage gap in what these tests can actually prove (Issue 1 below): they cannot distinguish "the per-module debezium.version property override is correctly honored by Maven" from "every module happens to resolve the same parent-default value," and they don't inspect the final packaged/shaded jar to confirm the debezium-core exclusion filter actually took effect — they only check target/classes and classpath-resolved artifact metadata.
2.3 Documentation Updates
docs/en/introduction/concepts/incompatible-changes.md and docs/zh/introduction/concepts/incompatible-changes.md were both updated with matching, detailed content, including a specific and honest note about the limits of safe per-connector Debezium divergence given connector-cdc-base's own patched-class usage. This satisfies the "any user-visible change must update docs/en and docs/zh" requirement.
3. Architectural Soundness
3.1 Elegance of the Solution
Precise fix / long-term solution. This addresses the actual root cause (a shared classloader-merged jar pinning one project-wide Debezium version) rather than working around it, and the guardrail tests + SPI groundwork are a reasonable, incremental step toward true per-connector Debezium independence (acknowledged as not-yet-complete by the PR itself and its docs).
3.2 Maintainability
Improved: Debezium ownership is now explicit per module instead of implicit via a shared base jar. The one fragile point — the shade-filter merge/override interaction between the parent's combine.children="append" and any connector that declares its own local shade <execution> — is currently correctly handled but is exactly the kind of thing a future contributor could silently break (e.g., by adding a local shade execution to another connector without knowing to restate the debezium-core filter). See Issue 1/2.
3.3 Extensibility
The DebeziumAdapter SPI (still @Experimental, still unwired into production code) gives the project a real seam for future per-connector version/classloader isolation without another large refactor.
3.4 Historical-Version Compatibility
No impact on checkpoint/savepoint, serialization, or historically-saved job state — this PR does not touch any of those paths.
4. Issue Summary
| No. | Issue | Location | Severity |
|---|---|---|---|
| 1 | New guardrail tests cannot prove the two central architectural claims of this PR: (a) that Maven correctly resolves each connector's own debezium.version property override from the inherited dependencyManagement rather than the parent's default — every module currently pins the identical 1.9.8.Final, so a broken override would still pass; (b) that the debezium-core shade exclusion filter is actually applied in the final packaged/shaded jar — the tests only inspect target/classes and classpath-resolved pom.properties, never the shaded jar contents. |
connector-cdc-base/.../PatchedDebeziumClassesTest.java; every */debezium/*DebeziumAdapterTest.java |
Medium |
| 2 | The per-module version-ownership mechanism depends on a non-obvious Maven semantic (dependency-management ${property} interpolation resolved per-leaf-module, not per-declaring-ancestor). It is correct today, but there is no comment warning against the common "helpful" refactor of moving this into a <dependencyManagement><scope>import</scope> BOM, which uses different interpolation semantics and would silently defeat per-connector version ownership. |
connector-cdc/pom.xml |
Low |
5. Merge Recommendation
Conclusion: Ready to merge after fixes
-
Blockers — must be fixed
- None. I found no High-severity issue and no functional regression for any in-tree CDC connector (verified mysql, postgres, oracle, sqlserver, mongodb, opengauss, vitess, and confirmed tidb is correctly and safely left untouched).
-
Recommended fixes — non-blocking
- Issue 1 (Medium): add at least one packaging-level assertion (e.g., unzip a built connector jar in a test/verify step and assert the five patched class names are absent from the
debezium-corecontribution, or diff the shippedTableId.class/ChangeEventQueue.classbytes againstconnector-cdc-base's own compiled patched classes) so a future silent regression of the shade-filter merge is caught by CI instead of by production classloading nondeterminism. This is exactly the failure mode the PR is designed to prevent, so it deserves a real regression test, not just a hand-maintained comment trail. - Issue 2 (Low): add a short comment on the
connector-cdc/pom.xmldependencyManagementblock warning future maintainers not to convert it into animport-scope BOM.
- Issue 1 (Medium): add at least one packaging-level assertion (e.g., unzip a built connector jar in a test/verify step and assert the five patched class names are absent from the
Overall assessment: this is a well-scoped, well-documented packaging refactor. I independently re-derived (not merely trusted) the Maven plugin-inheritance chain that makes the shade-filter exclusion work for all seven Debezium-based connectors, confirmed the one connector requiring a manual restatement (openGauss) handles it correctly, and confirmed the one CDC connector correctly left out of scope (TiDB) is genuinely unaffected because it never touches Debezium-referencing code paths in connector-cdc-base. No in-tree connector loses functionality, no connector registration/plugin-mapping surface is touched, and the documented incompatibility is accurately scoped to third-party connector builders. The remaining gap is test coverage for the mechanism's own guarantees, not a defect in the mechanism as implemented today.
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
Re-checked this PR today. Head is still afb0fada4b164062ef719f273caff9ad1197428d — no new commits since my full review above, and no replies or new discussion since then either, so this is a confirmation pass rather than a fresh review.
What I re-verified independently against current source (fresh checkout of this exact head, not just re-reading the diff):
connector-cdc/pom.xmlstill carries the<filters combine.children="append">block appending theio.debezium:debezium-coreexclusion (the five patched classes:ChangeEventQueue/ChangeEventQueue$*/TableId/TableId$*) for the six connectors that inherit shade config.connector-cdc-opengauss/pom.xmlstill locally restates the samedebezium-coreexclusion inside its own<filters>block (withoutcombine.children), which is required precisely because a child<execution>'s own filters override rather than merge with the inherited ones — confirmed this is still correct.connector-cdc-tidbremains completely untouched by this diff (git diff origin/dev...HEAD --statshows zero changes underconnector-cdc-tidb/), consistent with it never touching Debezium-referencing code paths.- Full diff is still the same 31 files / +1719/-32 across the 7 Debezium-based connectors'
pom.xml, adapters,META-INF/servicesregistrations, and adapter tests, plus the EN/ZH incompatible-changes docs.
CI / merge state: Build, Pull Request Labeler, and Notify test workflow all still SUCCESS at this head. mergeable: MERGEABLE (no textual conflicts), though the branch is currently 44 commits behind dev — worth a rebase before merge just to keep CI validating against current dev, not a blocker in itself. PR is still marked draft and mergeStateStatus: BLOCKED (review-required gate, not a conflict).
Conclusion stands: Ready to merge after fixes, no High-severity issues. The two non-blocking items from my previous review are still open and still the right asks:
- (Medium) The guardrail tests don't assert against the actual shaded jar output, so they can't catch a regression in the shade-filter merge itself, nor prove the per-module
debezium.versionoverride is honored (every module currently pins the same1.9.8.Final). - (Low) A short comment on
connector-cdc/pom.xml'sdependencyManagementblock warning against converting it to animport-scope BOM (which would defeat per-connector version interpolation) would help future maintainers.
Neither blocks merge. Nice work — this is a clean, well-documented packaging refactor with no functional regression for any in-tree connector.
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
First, an apology. I reviewed this PR on 2026-08-10 and again on 2026-08-13 and concluded "Ready to merge after fixes, no High-severity issues." That conclusion was wrong, and the things I missed were all sitting in the diff I claimed to have traced at full depth. This is a from-scratch re-review of the same head afb0fada4b164062ef719f273caff9ad1197428d — no new commits since — and Issues 1, 2, 3, 5 and 7 below are carryover, not new findings: every one of them was catchable in the earlier rounds. In particular Issue 2 is a correctness hazard this PR creates for out-of-tree connectors, and I signed off on the migration documentation that omits it. Sorry for the wasted round-trip.
What Problem Does This PR Solve?
- User pain point.
connector-cdc-baseships the Debezium runtime (debezium-api,debezium-embeddedand, transitively,debezium-coreplus the Kafka Connect tail) at compile scope. BecauseAbstractPluginDiscovery#filterPluginJarunconditionally force-loadsconnector-cdc-base-*.jarinto the class loader of every plugin whose name containscdc(seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:514-523, and the matching "must resolve to exactly 2 jars" rule at:532-538), that one shared jar is the single project-wide Debezium version. Thedebezium.versionproperty introduced by #10799 is therefore cosmetic: no connector can actually move independently. - Fix approach. Flip
debezium-api/debezium-embedded/zstd-jnitoprovidedinconnector-cdc-base/pom.xml; relocate the version + exclusiondependencyManagementup into theconnector-cdcparent so it is interpolated per leaf module against each connector's owndebezium.version; have all seven Debezium-based connectors declare the Debezium runtime they ship; add amaven-shade-pluginfilter that dropsdebezium-core's stock copies of the classesconnector-cdc-basepatches; and add an inertDebeziumAdapterSPI registration plus guardrail tests per connector. - One-sentence summary. A packaging-only refactor that moves Debezium runtime ownership out of the shared
connector-cdc-basejar into each CDC connector's own jar, with no functional change for in-tree connectors today, but with a documentation gap that leaves out-of-tree connectors exposed to a silent class-resolution hazard.
Simple example. Before this PR, a MySQL CDC job's plugin class loader contained connector-cdc-mysql.jar (no io.debezium core classes at all — every debezium-connector-* dependency explicitly excludes debezium-core and debezium-api) plus connector-cdc-base.jar (Debezium core/embedded plus five patched io.debezium classes). After this PR the same class loader contains connector-cdc-mysql.jar (Debezium core/embedded, minus four patched class families) plus connector-cdc-base.jar (the five patched classes only). Same class set, different jar of origin.
1. Code Change Review
1.1 Core Logic Analysis
Coverage scope. I read the full 31-file diff (11 modified: 9 pom.xml + 2 docs; 20 added; 0 deleted; zero modified .java files), plus the surrounding unmodified source needed to test its premises: AbstractPluginDiscovery, DebeziumAdapter / DebeziumAdapterFactory, all nine connector-cdc-* poms before and after, seatunnel-connectors-v2/pom.xml, the root pom.xml shade pluginManagement block, seatunnel-format-compatible-debezium-json/pom.xml, seatunnel-dist/pom.xml, seatunnel-examples/seatunnel-engine-examples/pom.xml, all seven connector-cdc-*-e2e/pom.xml, and the three e2e test classes that import io.debezium types. I also inspected the actual debezium-core-1.9.8.Final artifact and the six debezium-connector-* artifacts to check which of them physically contain the patched classes.
Did any connector silently lose Debezium? No — verified individually, and the verification matters because provided is not transitive, so the moment connector-cdc-base marks Debezium provided every dependent loses it entirely.
| Module | Declares debezium.version |
Declares debezium-api / -embedded / zstd-jni |
Needs Debezium |
|---|---|---|---|
connector-cdc-base |
inherits 1.9.8.Final from parent |
yes, provided |
yes (compile only) |
connector-cdc-mysql |
yes | yes | yes |
connector-cdc-postgres |
yes | yes | yes |
connector-cdc-oracle |
yes | yes | yes |
connector-cdc-sqlserver |
yes | yes | yes |
connector-cdc-mongodb |
yes | yes | yes |
connector-cdc-vitess |
yes | yes | yes |
connector-cdc-opengauss |
yes | yes | yes |
connector-cdc-tidb |
n/a | untouched | no — git grep io\.debezium under connector-cdc-tidb/ returns zero hits, and it declares connector-cdc-base at provided, so it never packaged Debezium in the first place |
Does the shade filter actually reach the six connectors that inherit it? This is the part of the design that is easy to assert and hard to prove, so I re-derived it rather than trusting the comment:
- The root
pom.xml:725-767declaresmaven-shade-pluginunderpluginManagementwith the<filters>(the*:*signature filter) at plugin level, and its single<execution>(no<id>, so iddefault) carries only<transformers>. That is the load-bearing detail: had the root put<filters>at execution level, the new plugin-level<filters>inconnector-cdc/pom.xmlwould be overridden and silently dropped for all six connectors. - Plugin-management injection merges with the
<build><plugins>entry dominant, andXpp3Domreadscombine.childrenfrom the dominant node.connector-cdc/pom.xml:148carriescombine.children="append", so the root's*:*filter is appended rather than replacing the newdebezium-corefilter. Effective plugin-level filter list:[debezium-core exclusions, *:*, *:*]. - Execution-level config (
<transformers>only) merges over that;<filters>has no execution-level counterpart, so it survives intact. connector-cdc-opengauss/pom.xml:95-142is the exception: it declares its own execution (also iddefault, so no double shading) with its own<filters>and nocombine.children. UnderXpp3Dom's positional child pairing with dominant leaf values winning, the effective result is exactly openGauss's own two filters, verbatim — which is why the manual restatement at:121-133is genuinely required, not defensive noise. Confirmed present and correct.- Shade runs at
ResolutionScope.RUNTIME, soprovidedartifacts are outsideproject.getArtifacts(). That is what makesconnector-cdc-base's newprovidedscope actually keep Debezium out of its jar, and what keepsconnector-cdc-base.jaritself out of the connector jars.
Is the filter necessary, or cargo cult? Necessary. I verified against the real artifacts that io/debezium/relational/TableId.class, io/debezium/connector/base/ChangeEventQueue.class, io/debezium/heartbeat/HeartbeatFactory.class and io/debezium/relational/HistorizedRelationalDatabaseConnectorConfig.class exist only in debezium-core — all six debezium-connector-* artifacts, debezium-embedded, debezium-api and debezium-ddl-parser contain none of them. So the single io.debezium:debezium-core artifact selector is sufficient and correctly scoped. I also confirmed the parent-pom comment's historical claim is accurate: pre-PR the connector jars contained no debezium-core at all, because every debezium-connector-* managed entry excludes debezium-core and debezium-api and connector-cdc-base was provided. Without this filter the move would have introduced a genuine duplicate-class race, and ChangeEventQueue's own header (connector-cdc-base/src/main/java/io/debezium/connector/base/ChangeEventQueue.java:49) says it exists "to avoid the OOM in snapshot phase" — so losing that race is a real production failure mode, not a cosmetic one.
Does the normal CDC read/snapshot path reach the new code? No. DebeziumAdapterFactory.getAdapter(...) has zero production call sites anywhere in the tree — git grep finds it only in DebeziumAdapterFactory itself and in the new/pre-existing test classes. The interface Javadoc says concrete wiring "will be added in follow-up PRs". There is no reflection-based or string-based lookup of the old package path anywhere: no class was renamed or moved, so there is nothing to look up. The only runtime-observable change is which jar supplies which class inside a class loader that already contained both jars.
openGauss / Postgres coupling. connector-cdc-opengauss depends on connector-cdc-postgres at compile scope and deliberately registers no adapter of its own, reusing PostgresDebeziumAdapter via the shaded META-INF/services merge. Worth noting for a future reader: when a job loads both connector-cdc-opengauss.jar and connector-cdc-postgres.jar, two jars will carry an identical META-INF/services entry naming the same FQCN. ServiceLoader.parse() deduplicates by provider class name across config URLs, so DebeziumAdapterFactory's exactly-one-match rule still holds. I checked this specifically because it would have been a latent IllegalStateException once the SPI gets wired; it is safe.
1.2 Compatibility Impact
Judgment: Partially incompatible — deliberately, and for third parties only.
- Config / API / SPI surface: unchanged. No
Optionrenamed, removed or re-defaulted. No public class changed its fully-qualified name — the diff modifies zero.javafiles.DebeziumAdapterandDebeziumAdapterFactorypre-date this PR and are untouched. - Checkpoint / savepoint / serialized state: untouched. No offset, split, state-serializer or snapshot/incremental-read class is in the diff. I checked specifically for classes participating in snapshot, incremental-read or offset-tracking logic, since a refactor of this shape could plausibly touch them; none are affected. The additions are three files per connector (an adapter, a services registration, a test) that no execution path reaches.
- In-tree runtime behavior: unchanged, because all seven connectors resolve the same
1.9.8.Finaland the patched classes still come fromconnector-cdc-base.jar. Verified rather than assumed. - Out-of-tree connectors: breaking, and the documented migration is incomplete — see Issue 2. That is the compatibility finding that matters in this round.
1.3 Performance / Side-Effect Analysis
- No CPU / memory / GC change on the job path.
- Packaging. This is the largest side effect and the docs describe it only qualitatively. Estimating from the local artifacts: per connector the newly packaged runtime is roughly
zstd-jni5.6 MB +debezium-core0.93 MB + thedebezium-embeddedKafka Connect tail (connect-api~0.1 MB,connect-runtime~0.6 MB,connect-json,connect-file, Jetty, reflections/javassist) ~2 MB, so on the order of 8-9 MB per connector. Multiplied across seven connectors and netting out the ~8-9 MB removed fromconnector-cdc-base, the binary distribution'sconnectors/directory grows by roughly 45-55 MB, and eachinstall-plugin.shdownload of a CDC connector grows by ~8 MB. Treat these as estimates derived from artifact sizes, not from a built distribution — I did not build this PR. zstd-jniis the single biggest line item and has no Debezium-ownership justification — see Issue 5.- Duplicate-class overlap inside
connector-cdc-opengauss.jar: openGauss now shades both theconnector-cdc-postgresuber-jar (which itself carries a filtereddebezium-core) and the rawdebezium-coreartifact. Shade resolves the overlap first-writer-wins and both are1.9.8.Final, so the outcome is correct today; expect additional "overlapping classes" warnings in the build log. Not a defect, but a maintainer reading a noisier openGauss build should know why. - Positive side effect:
connector-cdc-tidb, which is Debezium-free, no longer drags the whole Debezium runtime into its merged class loader viaconnector-cdc-base.
1.4 Error Handling and Logging
No new error-handling paths in production code. DebeziumAdapterFactory's IllegalStateException behavior for the no-match and multi-match cases is pre-existing and unmodified, and its messages are actionable (they name the connector class and list matched providers with their Debezium versions). The test helpers wrap IOException in IllegalStateException with the failing resource path, which is appropriate for test code. Nothing to flag.
2. Code Quality Assessment
2.1 Coding Standards
High. Every new class, field and method carries Javadoc that explains why rather than restating the code, and the pom comments document the classloader-merge rationale, the shade-override rationale and the openGauss/Postgres coupling. License headers are present on all 20 new files (CI's "License header" and "Code style" jobs are green). The one standards problem is that three of those carefully written comments state something that is not true — see Issue 1.
The seven adapter classes are near-identical (constant name, one string, two trivial overrides). That is acceptable for an SPI whose whole point is per-connector ownership; pulling the shared shape into an abstract base in connector-cdc-base would reintroduce exactly the coupling the PR is removing. No objection.
2.2 Test Coverage and Test Stability
Test stability: Stable. All eight new test classes are deterministic — they read compiled .class files, classpath-resolved META-INF/maven/**/pom.properties, and ServiceLoader registrations. No sleeps, no network, no fixed ports, no shared mutable static state, no @DisplayName. No flakiness risk.
Coverage, however, does not reach the two claims that carry the PR:
- The per-connector
*DebeziumAdapterTestclasses prove that everything on the module's test classpath agrees on one Debezium version. They cannot distinguish "each module's owndebezium.versionoverride was honored" from "every module inherited the same parent default", because all seven currently pin the identical1.9.8.Final. - Nothing inspects a packaged jar.
PatchedDebeziumClassesTestreadstarget/classes; the adapter tests read classpath metadata. The shade-filter merge — the single fragile mechanism this PR introduces, and the one whose failure is silent — has no assertion at all. See Issues 3 and 4.
CI evidence is genuinely good and I want to give it full weight: the fork run exercised all seven CDC e2e modules (connector-cdc-mysql-e2e, -postgres-e2e, -sqlserver-e2e, -mongodb-e2e, -oracle-e2e, -opengauss-e2e, -tidb-e2e) across the eight updated-modules-integration-test parts on both JDK 8 and 11, plus a dedicated oracle-cdc-connector-it, all green. Those e2e suites run against a real built distribution through the real plugin-discovery class loader, so they do validate the classpath move end to end. What they do not do is fail if the shade filter silently stops applying, because both copies of TableId are byte-compatible at 1.9.8.Final — the regression this PR guards against is invisible until the versions diverge.
No test hardcodes the old shared location's package name; there is no old package name, since nothing moved packages.
2.3 Documentation Updates
docs/en and docs/zh incompatible-changes.md were both updated with matching content, which satisfies the "user-visible change must update both" rule. The "How far connectors may actually diverge" bullet is unusually honest and I want to keep it as-is. But the "Impact" bullet immediately above it is factually wrong and directly contradicts it (Issue 1), and the migration instruction it gives third parties is incomplete in a way that can cause a snapshot-phase OOM (Issue 2). Documentation is where this PR's remaining risk lives.
3. Architectural Soundness
3.1 Elegance of the Solution
Addresses the real root cause — a force-merged shared jar pinning one project-wide version — rather than working around it. The choice to manage versions in the connector-cdc parent's dependencyManagement while declaring them per module is the correct Maven idiom for "shared exclusion rules, per-module version", and the shade filter closes the duplicate-class hole the move would otherwise open. The design is sound; the gaps are in what is documented and what is enforced, not in the mechanism.
3.2 Maintainability
Net improvement: ownership is now explicit per module. Two fragile points remain, both hand-maintained and neither machine-checked: the shade-filter merge/override interaction (any connector that later adds its own shade <execution> must restate the debezium-core filter, as openGauss does, or silently lose it), and the correspondence between connector-cdc-base/src/main/java/io/debezium/** and the two exclusion lists. Issue 3 shows the second one has already drifted in letter.
3.3 Extensibility
The DebeziumAdapter SPI gives a real seam for future per-connector isolation. Two forward-looking notes for whoever writes the follow-up: (a) once versions actually diverge, a job that uses two CDC connectors at once puts two different Debezium runtimes on one merged class loader and reintroduces the same first-wins race one level up — the SPI will need real class loader separation, not just a version string; (b) ServiceLoader's name-based dedup means the openGauss/Postgres pair will resolve to whichever PostgresDebeziumAdapter class loads first, which is fine while they are identical and will need attention when they are not.
3.4 Historical-Version Compatibility
No impact. Nothing in the diff touches checkpoint/savepoint state, offset tracking, split serialization, or any persisted format. A job checkpointed on the previous release restores unchanged, because the class set visible to it is identical. Checkpoint/restore rigor is warranted for any cross-module CDC refactor, but it genuinely finds nothing here — I verified by confirming zero .java files were modified and that all 20 added files are unreachable from any production call path.
4. Issue Summary
| # | Issue | Location | Severity |
|---|---|---|---|
| 1 | Three places state connector-cdc-base no longer contains io.debezium classes; it still ships five, and the same doc entry says so two bullets later |
docs/en/introduction/concepts/incompatible-changes.md:150; docs/zh/introduction/concepts/incompatible-changes.md:143; seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:45-57 |
Medium |
| 2 | Third-party migration instructions omit the mandatory io.debezium:debezium-core shade exclusion, leaving out-of-tree connectors with a silent, order-dependent class-resolution race against the patched ChangeEventQueue |
docs/en/introduction/concepts/incompatible-changes.md:150; docs/zh/introduction/concepts/incompatible-changes.md:143 |
High |
| 3 | EXPECTED_PATCHED_CLASSES lists five classes, both shade filters exclude four; the omission is unexplained and the test never compares itself against the pom filters it claims to guard |
.../connector-cdc-base/src/test/java/.../PatchedDebeziumClassesTest.java:50-59; seatunnel-connectors-v2/connector-cdc/pom.xml:148-161; seatunnel-connectors-v2/connector-cdc/connector-cdc-opengauss/pom.xml:121-133 |
Medium |
| 4 | Guardrail tests assert nothing about the packaged/shaded jar and cannot prove the per-module debezium.version override is honored |
.../PatchedDebeziumClassesTest.java; every .../debezium/*DebeziumAdapterTest.java |
Medium |
| 5 | zstd-jni is swept into the per-connector move with no Debezium-ownership rationale, multiplying a 5.6 MB artifact across seven jars |
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:70-73 plus the seven connector poms |
Medium |
| 6 | Distribution size growth documented only qualitatively; no number given for a change that adds roughly 45-55 MB | docs/en/introduction/concepts/incompatible-changes.md:151; docs/zh/introduction/concepts/incompatible-changes.md:144 |
Low |
| 7 | Redundant restatement of the *:* signature filter, justified by a comment that hedges on a Maven merge rule that is in fact deterministic |
seatunnel-connectors-v2/connector-cdc/pom.xml:162-173 |
Low |
| 8 | PR is still a draft, and its base is 60 commits behind dev; the green CI dates from 2026-08-09 against a 2026-08-03 base |
PR metadata | Low |
Issue 1 — connector-cdc-base is documented as Debezium-class-free; it is not
- Location:
docs/en/introduction/concepts/incompatible-changes.md:150;docs/zh/introduction/concepts/incompatible-changes.md:143;seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:45-57. - Problem: The EN Impact bullet ends "
connectors/connector-cdc-base-*.jarno longer containsio.debeziumclasses", the ZH mirrors it, and the pom comment says keeping the dependencies provided "leaves the shaded connector-cdc-base jar free of Debezium classes". All three are false.connector-cdc-base/src/main/java/io/debezium/contains and ships five classes:connector/base/ChangeEventQueue,heartbeat/DefaultHeartbeatConnectionProvider,heartbeat/HeartbeatFactory,relational/HistorizedRelationalDatabaseConnectorConfig,relational/TableId. Line 152 of the same EN entry says so explicitly, so the document contradicts itself two bullets apart. - Potential risk: An out-of-tree connector maintainer who believes the base jar is Debezium-free has no reason to suspect a class overlap, and will package their own stock
TableId/ChangeEventQueue— which is exactly the failure Issue 2 describes. The statement is not merely imprecise; it actively points readers away from the one thing they must do. - Best improvement: Replace with something like "no longer contains the Debezium runtime (
debezium-api,debezium-embedded,debezium-coreand their Kafka Connect / compression / Jetty dependencies). It still ships its five patchedio.debeziumclasses, which every CDC connector jar must exclude from its owndebezium-corecopy." Mirror in ZH, and fix the pom comment to say "free of the Debezium runtime" rather than "free of Debezium classes". - Severity: Medium.
- Raised by another reviewer: No.
Issue 2 — Third-party migration path omits the mandatory shade exclusion
- Location:
docs/en/introduction/concepts/incompatible-changes.md:150;docs/zh/introduction/concepts/incompatible-changes.md:143. - Problem: The migration instruction is "Third-party CDC connectors ... must now declare
debezium-apianddebezium-embeddedthemselves." A third party who does exactly and only that ends up withdebezium-coreon their compile scope and therefore with stockio/debezium/relational/TableId.class,io/debezium/connector/base/ChangeEventQueue.class,io/debezium/heartbeat/HeartbeatFactory.classandio/debezium/relational/HistorizedRelationalDatabaseConnectorConfig.classinside their shaded connector jar — sitting next toconnector-cdc-base's patched copies on the same force-merged class loader. Which copy wins depends onFile#listFilesordering. This is the exact scenarioconnector-cdc/pom.xml:137-142describes as "the patches could silently stop applying", and the in-tree fix for it (theio.debezium:debezium-corefilter) is nowhere in the migration text. The instruction also omitszstd-jni, which every in-tree connector pom declares because the manageddebezium-embeddedentry excludes it. - Potential risk: Real and non-cosmetic.
ChangeEventQueueis patched, per its own header comment atconnector-cdc-base/src/main/java/io/debezium/connector/base/ChangeEventQueue.java:49, "to avoid the OOM in snapshot phase". A third-party CDC connector rebuilt against the new base can therefore start OOM-ing during snapshot on some machines and not others, with no diagnostic pointing at the cause. The PR knowingly creates this hazard for the exact audience the change breaks, and the breaking-change note is the only place it would be caught. - Best improvement: Extend the Impact bullet (EN and ZH) with the complete recipe: declare
debezium-api,debezium-embeddedandzstd-jni; and add amaven-shade-plugin<filter><artifact>io.debezium:debezium-core</artifact>excluding the four patched class families (with the$*inner-class variants), noting that a connector declaring its own shade<execution>must restate the filter inside that execution because execution-level<filters>replace the inherited ones. Copying the exact block fromconnector-cdc/pom.xml:149-161into the doc would make this unambiguous. Given the failure mode is a silent snapshot OOM, I would not merge without this. - Severity: High.
- Raised by another reviewer: No.
Issue 3 — Patched-class list and shade exclusion list disagree, and nothing checks them against each other
- Location:
.../connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:50-59;seatunnel-connectors-v2/connector-cdc/pom.xml:148-161;seatunnel-connectors-v2/connector-cdc/connector-cdc-opengauss/pom.xml:121-133. - Problem: The field Javadoc states "Every entry here must also be excluded from debezium-core by the CDC shade configuration."
EXPECTED_PATCHED_CLASSEShas five entries; both shade filters exclude four families —io/debezium/heartbeat/DefaultHeartbeatConnectionProvideris absent from both. I checked the artifact: that class genuinely does not exist indebezium-core-1.9.8.Final(which shipsHeartbeatConnectionProvider, an interface, but noDefaultimplementation), so the omission is harmless today and is presumably deliberate. Nothing says so. Separately, the test compares compiled classes against a hardcoded constant, never against the pom filters, so it structurally cannot detect a real list-versus-filter divergence — it can only nag when a new patched class appears, and then only if the author reads the failure message. - Potential risk: A maintainer cannot tell whether
DefaultHeartbeatConnectionProviderwas deliberately omitted or forgotten, which is the exact ambiguity a guardrail is supposed to eliminate. If a later Debezium line introduces that class intodebezium-core, the duplicate arrives and the guardrail stays green. - Best improvement: (a) Add one line recording that
DefaultHeartbeatConnectionProvideris a SeaTunnel-introduced class with nodebezium-corecounterpart, so no exclusion is needed. (b) Better, make the test machine-check the correspondence: the two poms sit at known relative paths from${project.basedir}, so the test can parse theirio.debezium:debezium-core<excludes>and assert they cover exactly the compiled patched set minus classes absent fromdebezium-core. That turns a comment-maintained invariant into an enforced one and subsumes half of Issue 4. - Severity: Medium.
- Raised by another reviewer: No — and this is carryover; it was visible at this head in both my earlier rounds.
Issue 4 — Guardrail tests do not exercise the packaged jar or the version-override mechanism
- Location:
.../PatchedDebeziumClassesTest.java; every.../debezium/*DebeziumAdapterTest.java. - Problem: (a) No test inspects a built jar, so a regression in the shade-filter merge — the one novel and fragile mechanism here — is undetectable by CI. (b) Every module pins the identical
1.9.8.Final, sopackagedDebeziumArtifactsAllMatchDeclaredVersionpasses equally well whether the per-module override is honored or silently ignored in favour of the parent default. - Potential risk: The failure this PR exists to prevent is precisely the one CI cannot see. Both classes of regression surface only in production class loading, non-deterministically.
- Best improvement: Add a packaging-level assertion — for instance a
maven-invoker/verify-phase check, or a test that openstarget/connector-cdc-*.jarand asserts the four patched entries are absent while, say,io/debezium/relational/TableIdParser.classis present (provingdebezium-corewas shaded in and the filter fired, rather thandebezium-coresimply being missing). For (b), a build-time assertion that${debezium.version}as resolved in the module equals the adapter's constant would at least pin the interpolation path. - Severity: Medium.
- Raised by another reviewer: Yes (@DanielLeens, review of 2026-08-10, reaffirmed 2026-08-13) — still open at this head.
Issue 5 — zstd-jni moved without a Debezium-ownership rationale
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:70-73(nowprovided) and the corresponding new blocks in all seven connector poms. - Problem: The PR's entire argument is that
connector-cdc-basemust stop packaging Debezium so each connector can own its Debezium version.com.github.luben:zstd-jnicontains noio.debeziumclasses, is not part of the Debezium API surface, and is referenced nowhere inconnector-cdc/**source (git grep -i zstdacross the CDC tree returns only pom hits). It is present only because the manageddebezium-embeddedentry excludes the transitive copy for Apple M1 compatibility and re-adds1.5.5-5explicitly. Making itprovidedin the base and re-declaring it in seven connectors does not advance version ownership; it just copies a 5.6 MB artifact seven times instead of once. - Potential risk: About 5.6 MB × 6 = ~34 MB of avoidable growth in the distribution and in every CDC connector download — the single largest contributor to Issue 6's number — with no stated benefit. It also widens the blast radius of any future
zstd-jnibump from one pom to eight. - Best improvement: Either leave
zstd-jniat compile scope inconnector-cdc-base(the merged class loader supplies it to every CDC connector exactly as before, and it does not participate in the Debezium version-pinning problem), or state explicitly in the pom comment why a connector owning its Debezium version must also own its zstd version. If the intent is future-proofing for a Debezium line requiring a different zstd, say so. - Severity: Medium.
- Raised by another reviewer: No.
Issue 6 — Distribution size growth is not quantified
- Location:
docs/en/introduction/concepts/incompatible-changes.md:151;docs/zh/introduction/concepts/incompatible-changes.md:144. - Problem: The packaging note says the distribution "increases in size" without a figure. My estimate from artifact sizes is roughly +45-55 MB in
connectors/and ~+8 MB per CDC connector download. - Potential risk: Low on its own, but this is the sort of number packagers, image builders and air-gapped users need before an upgrade, and an incompatible-changes entry is exactly where they will look for it.
- Best improvement: Add the measured (not estimated) delta from a built distribution, before and after, to both docs.
- Severity: Low.
- Raised by another reviewer: No.
Issue 7 — Redundant signature filter, with a comment that hedges on a deterministic rule
- Location:
seatunnel-connectors-v2/connector-cdc/pom.xml:162-173. - Problem: The restated
*:*META-INF/*.SF|DSA|RSAfilter is justified by "so the signature files stay excluded whether or not the inherited filter list is merged into this one". The merge is not conditional:combine.children="append"on line 148 is read from the dominant node during plugin-management injection, so the root's filter is always appended. The result is two identical*:*filters in the effective configuration. Harmless — shade applies each independently — but the comment records uncertainty about a rule the PR in fact depends on being deterministic (openGauss's manual restatement is correct only because that same merge behaviour is predictable). - Potential risk: Cosmetic. The risk is that a future maintainer reads the hedge, concludes the merge is unpredictable, and starts restating filters defensively in every connector.
- Best improvement: Keep the duplicate if you prefer belt-and-braces — removing it is not worth the tail risk of leaking signature files into an uber-jar — but rewrite the comment to state the actual rule: plugin-level
<filters>merge bycombine.children="append", execution-level<filters>replace, which is why openGauss must restate and the others need not. - Severity: Low.
- Raised by another reviewer: No.
Issue 8 — Draft status and stale base
- Location: PR metadata.
- Problem:
isDraft: true,mergeStateStatus: BLOCKED,reviewDecision: REVIEW_REQUIRED,mergeable: MERGEABLE(no textual conflict). The head is 60 commits behinddev, and the green CI run finished 2026-08-09 against a 2026-08-03 base. - Potential risk: A draft cannot be merged. And because this PR edits a shared parent pom's
build/pluginssection, a stale base is more consequential than usual — any shade or plugin change landing ondevin those 60 commits would interact with it and is not covered by the current green run. - Best improvement: Mark ready for review, rebase or
gh pr update-branchonto currentdev, and let CI re-run before merge. - Severity: Low.
- Raised by another reviewer: No.
5. Merge Recommendation
Conclusion: Ready to merge after fixes
-
Blockers — must be fixed
- Issue 2 (High): complete the third-party migration instructions in
docs/enanddocs/zhwith the mandatoryio.debezium:debezium-coreshade exclusion (andzstd-jni), including the note that a connector with its own shade<execution>must restate the filter inside it. Without this, this PR hands out-of-tree CDC connectors a silent snapshot-phase OOM risk with no documented mitigation. It is a two-paragraph fix. - Issue 1 (Medium, bundled with the above): correct the three statements claiming
connector-cdc-baseno longer containsio.debeziumclasses. This lives in the same doc bullet as Issue 2 and directly undercuts the fix for it, so it should land together.
- Issue 2 (High): complete the third-party migration instructions in
-
Recommended fixes — non-blocking
- Issue 3: explain or close the
DefaultHeartbeatConnectionProviderasymmetry, and preferably makePatchedDebeziumClassesTestcheck the pom filters directly instead of a hand-kept constant. - Issue 4 (carryover): add one packaging-level assertion against a built connector jar, so the shade-filter merge is covered by CI rather than by comment discipline.
- Issue 5: leave
zstd-jnicompile-scoped inconnector-cdc-base, or document why per-connector Debezium ownership requires per-connector zstd ownership. - Issue 6: replace the qualitative size note with a measured delta.
- Issue 7: fix the comment on the restated signature filter to state the real Maven merge rule.
- Issue 8: mark ready for review and rebase onto current
devbefore merge.
- Issue 3: explain or close the
-
Overall assessment
The mechanism is right and I verified it rather than accepting it. I re-derived the Maven plugin-inheritance and
Xpp3Dommerge chain that makes thedebezium-corefilter reach all six inheriting connectors, confirmed why openGauss is the one module that must restate it and that it does, confirmed shade'sResolutionScope.RUNTIMEis what makes theprovidedflip actually keep Debezium out of the base jar, confirmed against the real artifacts that onlydebezium-corecarries the patched classes (so the artifact selector is correctly scoped), confirmed every one of the seven Debezium-based connectors gained explicit declarations so none silently lost classes, confirmedconnector-cdc-tidbhas zeroio.debeziumreferences and is correctly untouched, and confirmed no.javafile was modified — so no public FQCN moved, no SPI or serialized-state reference changed, and theDebeziumAdapterSPI remains inert with zero production call sites. CI ran all seven CDC e2e suites green against a real distribution on JDK 8 and 11.What lets this down is documentation, and specifically the one bullet an out-of-tree connector maintainer will read. This PR deliberately breaks third-party CDC connectors and then tells them to do half of what is required; the omitted half prevents a non-deterministic snapshot-phase OOM. That is a blocker in my view even though no in-tree connector is affected, because the breaking-change note is the only channel through which the affected audience learns anything at all. The remaining items are quality-of-guardrail and packaging-hygiene asks, none of which should hold the change.
I should have caught Issues 1, 2, 3, 5 and 7 in my 2026-08-10 review and again on 2026-08-13. They were all present at this head, and "no High-severity issues" was the wrong call. Correcting that here.
|
Re-checked this PR today. Head is still
Conclusion is unchanged from the 08-16 review: Ready to merge after fixes, blocked on Issue 2 (High) and Issue 1 (Medium), with Issues 3-7 as non-blocking follow-ups and Issue 8 (draft status / rebase) still applying. I'll take another full pass as soon as a new commit lands. |
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
Re-checking my own PR again today. Head is still afb0fada4b164062ef719f273caff9ad1197428d — no new commits, and no new comments or reviews from anyone (including @SEZ9) since my last round on 2026-08-19. I re-skimmed the diff and it still matches what I reviewed then: moving Debezium ownership from connector-cdc-base into each individual CDC connector module. Build on the apache-side check is still green (SUCCESS). This PR remains in draft, and my prior conclusion holds unchanged — no new findings to report.
What Problem Does This PR Solve?
1. Code Change Review1.1 Core Logic AnalysisStatus of this round. Head is still
I am not restating the full plugin-inheritance derivation, the 1.2 Compatibility ImpactJudgment: Partially incompatible (unchanged). No in-tree connector's config, API, checkpoint, or serialization surface is touched. All seven Debezium-based connectors still resolve 1.3 Performance / Side-Effect AnalysisUnchanged from prior rounds: no CPU/memory/GC change on the job path; the Debezium runtime (~8-9 MB per connector including 1.4 Error Handling and LoggingNo new error-handling paths in production code; nothing to flag. 2. Code Quality Assessment2.1 Coding StandardsUnchanged: comments throughout are unusually thorough and explain why, not just what. License headers present on all new files. 2.2 Test Coverage and Test StabilityTest stability rating: Stable (unchanged) — all eight new test classes are deterministic (compiled-class/classpath/ Coverage gap unchanged: none of the guardrail tests inspect a packaged/shaded jar (so a regression in the shade-filter merge — the one genuinely fragile, novel mechanism here — would not be caught by CI), and every module currently resolving the identical 2.3 Documentation Updates
3. Architectural Soundness3.1 Elegance of the SolutionUnchanged assessment: this is a precise, long-term fix for the real root cause (a force-merged shared jar pinning one project-wide Debezium version), not a workaround. 3.2 MaintainabilityUnchanged: net improvement, with the same two hand-maintained (not machine-checked) fragile points as before — the shade-filter merge/override interaction, and the correspondence between the patched-class list and the two exclusion lists (which has already drifted in letter, per Issue 3). 3.3 ExtensibilityUnchanged: the 3.4 Historical-Version CompatibilityUnchanged: no impact on checkpoint/savepoint, offset tracking, or serialized state — confirmed again this round that zero 4. Issue Summary
5. Merge RecommendationConclusion: Ready to merge after fixes
On the other reviewer: @SEZ9's 2026-08-04 COMMENTED review ("LGTM, can merge") predates all of the above findings and was explicitly conditioned on verifying the shade filters in Overall assessment. Nothing has changed at the code level since my 2026-08-16 full review: same 31-file diff, same five patched classes, same shade-filter mechanism, same two open doc issues. The one new thing worth checking this round — a same-day |
|
@DanielLeens thanks for the thorough summary and re-verification — the description matches the change set, and I support the direction of moving Debezium ownership out of the shared The remaining blockers are the two documentation issues you flagged in review 4945111316:
Since the head is still I also agree with your drift analysis: no sync is required right now since nothing upstream touches the shade-filter chain this PR relies on. Once the docs commit lands, I'll do a fresh pass promptly. |
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
What Problem Does This PR Solve?
- User pain point:
connector-cdc-baseshipped the Debezium runtime (debezium-api,debezium-embedded, transitivelydebezium-core) at compile scope, and becauseAbstractPluginDiscovery#filterPluginJarforce-loadsconnector-cdc-base-*.jaronto the class loader of every "cdc" plugin, that one shared jar pinned a single project-wide Debezium version — no individual CDC connector could move independently. - Fix approach: flip
debezium-api/debezium-embedded/zstd-jnitoprovidedinconnector-cdc-base, relocate the version + exclusiondependencyManagementinto theconnector-cdcparent (interpolated per leaf module against each connector's owndebezium.version), have all seven Debezium-based connectors declare their own Debezium runtime dependencies, add amaven-shade-pluginfilter droppingdebezium-core's stock copies of the classesconnector-cdc-basepatches, and add an (currently inert)DebeziumAdapterSPI + guardrail tests per connector. - One-sentence summary: a packaging-only refactor moving Debezium runtime ownership out of the shared
connector-cdc-basejar into each connector's own jar, with no in-tree runtime behavior change, but with one still-open internal inconsistency in the safety mechanism itself.
This is a re-review of my own PR. I did not trust my prior rounds (2026-08-16, 08-21, 08-22) or @SEZ9's; I re-derived the registration/dependency matrix from the current head (c53bc733) from scratch, since this is exactly the class of change (moving a shared dependency's ownership across module boundaries) where a half-registered module is easy to miss.
1. Code Change Review
1.1 Core Logic Analysis
Runtime reachability. DebeziumAdapterFactory.getAdapter(...) still has zero production call sites — I grepped the full PR tree, not just the diff, and every call site is in the eight new *DebeziumAdapterTest.java files. The DebeziumAdapter SPI is inert scaffolding for a future PR; today's CDC read/snapshot path is unaffected by it. The change that does affect today's runtime is the dependency-scope/packaging change, not the SPI.
Full registration-matrix audit, verified directly against the current head (not the diff alone):
- Dependency scope flip is real and matches the stated rationale.
connector-cdc-base/pom.xmlmoveddebezium-api/debezium-embedded/zstd-jnifrom implicit compile scope toprovided, with a comment explaining why:connector-cdc-baseis loaded onto every CDC connector's class loader, so a compile-scope (and therefore shaded-in) Debezium dependency there would pin one project-wide version. I confirmed this by reading the actual dev-branch pom before the change. - All seven touched connectors (mongodb, mysql, opengauss, oracle, postgres, sqlserver, vitess) now declare their own Debezium dependencies, each shaded into that connector's own jar, replacing what they used to get transitively from
connector-cdc-base. - The untouched eighth sibling,
connector-cdc-tidb, is not affected. I checked this specifically because it's a CDC module under the same parent that this PR does not touch, and it inherited fromconnector-cdc-base(atprovidedscope) exactly like the others before this PR. I grepped its entire source tree forio.debeziumand got zero hits — TiDB CDC in this codebase is implemented ontikv-client-java, not Debezium, so the scope flip is a no-op for it, not a silent breakage. - The classloader-collision problem this PR's second commit fixes is real and correctly understood. Before this PR,
connector-cdc-base's own shade build merged its five hand-patchedio.debezium.*classes (ChangeEventQueue,TableId,HistorizedRelationalDatabaseConnectorConfig,HeartbeatFactory,DefaultHeartbeatConnectionProvider— overriding stock Debezium classes of the same fully-qualified name, e.g. for the snapshot-phase OOM fixChangeEventQueuecarries) together with the realdebezium-coreinto one jar; after this PR, the patched copies stay inconnector-cdc-base.jarwhile each connector's own jar now separately shades in the stockdebezium-core, and both jars sit on the same runtime class loader (AbstractPluginDiscovery). Without an explicit exclusion, which class wins would depend onFile#listFiles()order — a real, previously-latent-but-now-actualized risk, not a hypothetical one. - The exclusion mechanism is centralized correctly, with one confirmed gap. The exclusion filter lives once in the
connector-cdcparent's<build><plugins>(maven-shade-plugin,<filters combine.children="append">), so mysql/mongodb/oracle/postgres/sqlserver/vitess inherit it automatically — I checked all six and none of them re-declares its own<filters>block, so none of them can accidentally lose the parent's exclusion.connector-cdc-opengaussis the sole exception: it declares its own execution-level<filters>(for its own pre-existing relocations), which per the PR's own comment "takes precedence over the one inherited from the connector-cdc parent," so it manually restates the same filter — I compared the two lists and they match each other. - However: both filter lists (parent
connector-cdc/pom.xmlandconnector-cdc-opengauss/pom.xml) exclude only four of the five patched classes.io/debezium/heartbeat/DefaultHeartbeatConnectionProvideris missing from both<excludes>blocks — I grepped both files directly at the current head and confirmed it's absent from both.connector-cdc-base's ownPatchedDebeziumClassesTest(added by this PR) pins the set of compiled classes undersrc/main/java/io/debeziumto five entries includingDefaultHeartbeatConnectionProvider, so the PR's own guardrail test and its own shade filters already disagree with each other about which classes need protecting. I readDefaultHeartbeatConnectionProvider.java: it's a plainimplements HeartbeatConnectionProviderclass in the real Debezium packageio.debezium.heartbeat, using the "Default*" naming convention Debezium's own codebase uses for its stock implementations — nothing in the diff or the class itself indicates this name is unique to SeaTunnel's patch, so I can't rule out (and have no positive evidence ruling out) that the realdebezium-corejar ships a class with the identical name, in which case this specific class is exposed to exactly the same non-deterministic classloader race the rest of the mechanism was built to eliminate. See Issue 1 below — this is the one substantive gap I'm carrying forward from my own 2026-08-16/08-22 rounds, re-confirmed against the current head rather than assumed still true.
Delta since my 2026-08-22 round: one new commit, [Docs] Clarify Debezium CDC migration note (c53bc733, 2 files, docs only). I diffed it directly: it corrects exactly the two doc problems @SEZ9 and I both flagged — (a) it now states connector-cdc-base still ships five patched io.debezium classes, instead of implying zero, and (b) the third-party migration note now explicitly instructs out-of-tree connectors packaging io.debezium:debezium-core to exclude the stock copies of the four listed classes. Both are genuinely fixed at the current head. The commit does not touch the pom.xml filters or the test, so Issue 1 (the fifth class) is unchanged — and the doc fix, ironically, still only lists four classes to exclude, mirroring the pom gap rather than the test's five.
1.2 Compatibility Impact
Partially incompatible, correctly documented for in-tree connectors, and now correctly documented for out-of-tree ones too (with the same one-class gap as the code). All seven in-tree Debezium-based connectors still resolve debezium.version=1.9.8.Final, so the class set on a running CDC job's class loader is unchanged before/after this PR — no config, checkpoint, or serialization impact for existing jobs. docs/en|zh/introduction/concepts/incompatible-changes.md both carry a ### Dependency Upgrades entry describing the breaking surface for third-party CDC connectors that relied on transitive Debezium from connector-cdc-base. The migration instructions list four classes to exclude (ChangeEventQueue, TableId, HistorizedRelationalDatabaseConnectorConfig, HeartbeatFactory) — consistent with Issue 1, this should be five once that's resolved.
1.3 Performance / Side-Effect Analysis
The Debezium runtime (Debezium + its Kafka Connect/compression/Jetty dependencies, plus zstd-jni) is now duplicated into each of the seven connector jars instead of shared via one connector-cdc-base jar, which grows the binary distribution size. The docs note this qualitatively ("increases the size of the binary distribution") without a measured delta — Low, informational, not a blocker for a packaging-scope change.
1.4 Error Handling and Logging
No new runtime error-handling paths — this PR only touches build/packaging configuration and test-only SPI usage; nothing to flag here.
Formal issues, sorted by severity:
Issue 1 (Medium-High): DefaultHeartbeatConnectionProvider is missing from both shade-exclusion filters, while the PR's own test says it should be patched-and-protected
- Location:
connector-cdc/pom.xml(<filters><filter><artifact>io.debezium:debezium-core</artifact><excludes>...),connector-cdc-opengauss/pom.xml(same, restated), vs.connector-cdc-base/src/test/java/.../PatchedDebeziumClassesTest.java(EXPECTED_PATCHED_CLASSES, 5 entries). - Problem: the exclusion filters strip only 4 of the 5 classes
connector-cdc-baserecompiles underio.debezium. Ifdebezium-corealso ships a class atio/debezium/heartbeat/DefaultHeartbeatConnectionProvider, each connector's own shaded jar would carry the stock copy alongsideconnector-cdc-base's patched copy on the same runtime class loader, with the winner depending on file-listing order — exactly the non-determinism this PR's second commit exists to eliminate for the other four classes. - Potential risk: if this class is in fact patched to fix a real defect (the sibling classes are patched specifically for a snapshot-phase OOM issue per the PR's own commit message), a machine where the stock copy wins would silently lose that fix with no error, only a possible resource/behavior regression during CDC snapshot on some deployments and not others — a hard class of bug to reproduce and diagnose.
- Best improvement: either add
DefaultHeartbeatConnectionProvider.class/DefaultHeartbeatConnectionProvider$*.classto both exclusion lists (if the stock class exists and needs excluding), or, if this specific class is actually unique toconnector-cdc-baseand has nodebezium-corecounterpart, updatePatchedDebeziumClassesTest's Javadoc/comment to explain why it's intentionally excluded from the shade filters — right now the test and the filters simply disagree with no explanation either way. Whichever is correct,PatchedDebeziumClassesTestshould assert against the actual filter list (or a shared constant) instead of maintaining a second, independently-hand-written list, so this class of drift fails CI automatically next time. - Severity: Medium-High. Non-deterministic, environment-dependent, and the blast radius is every Debezium-based CDC connector's snapshot phase if the stock class does exist and diverges from the patch.
- Raised by another reviewer: No (carried forward and re-verified from my own 2026-08-16/08-22 rounds; not previously raised by @SEZ9, whose review has focused on the documentation issues, both now fixed).
Issue 2 (Low, carried forward): distribution size growth is documented qualitatively only
- Location:
docs/en|zh/introduction/concepts/incompatible-changes.md, "Packaging note" bullet. - A measured delta (even an approximate MB figure per platform build) would be more actionable for release-notes consumers than "increases."
- Raised by another reviewer: No (self-carried).
Issue 3 (Low, carried forward): guardrail tests don't inspect a built/shaded jar
- Location:
PatchedDebeziumClassesTest.java; every new*DebeziumAdapterTest.java. - All new tests operate on compiled classes/
ServiceLoaderon the test classpath, not on the actual shaded artifact each module'smvn packageproduces. A regression in the shade-filtercombine.children="append"merge (or, per Issue 1, an incomplete filter) would not be caught by any of this PR's own tests. - Raised by another reviewer: No (self-carried).
Issue 4 (Low, informational): branch drift
- Live re-check:
dev...c53bc733isahead_by: 3, behind_by: 128, status: diverged.mergeable: MERGEABLE(no textual conflict),mergeStateStatus: BLOCKEDis the review-required gate, not a conflict. Recommend rebasing before final merge given how far behind this now is. - Raised by another reviewer: No (self-carried; my 2026-08-22 round specifically checked that a same-day
devcommit touchingconnector-cdc-base/pom.xmldid not conflict with this PR's shade-filter mechanism — worth re-checking again given the additional 128-commit drift since then, thoughmergeable: MERGEABLEis a reasonable live signal that nothing textual has broken).
2. Code Quality Assessment
2.1 Coding Standards
License headers present on all new files. Comments throughout the pom.xml and test changes are unusually thorough and explain why (e.g. the combine.children="append" rationale, the opengauss filter-restatement rationale), which is exactly what this kind of build-configuration change needs to stay maintainable. See Issue 1 for the one place where a comment/test and the actual configuration have drifted apart.
2.2 Test Coverage and Test Stability
Rating: Stable. All eight new test classes are deterministic: compiled-class inspection, classpath/ServiceLoader inspection, and pom.properties resource reads — no sleeps, no network, no fixed ports, no shared mutable state, no order-sensitive assertions. OpengaussDebeziumAdapterTest in particular is a well-designed regression guard against the postgres/opengauss Debezium-version coupling described in its own Javadoc. Coverage gap: see Issue 3 (nothing exercises an actual packaged jar).
2.3 Documentation Updates
docs/en and docs/zh incompatible-changes.md are updated, symmetric, and — as of the latest commit — accurate about connector-cdc-base still shipping five patched classes and about the shade-exclusion obligation for third-party connectors. The one remaining gap (Issue 1) is that both the code and the docs currently agree on four classes, while the test's list says five; once Issue 1 is resolved one way or the other, the docs should be updated to match.
3. Architectural Soundness
3.1 Elegance of the Solution
A precise, long-term fix for the actual root cause (a force-merged shared jar pinning one project-wide Debezium version across every CDC connector), not a workaround. The centralized-filter-with-opt-out-restatement design (parent pom filter + opengauss's documented override) is a reasonable way to keep the exclusion list in one place for the common case.
3.2 Maintainability
Net improvement over the prior single-shared-jar model. The one fragile point is the hand-maintained correspondence between connector-cdc-base's actual patched-class set, the parent pom's exclusion filter, opengauss's restated filter, and PatchedDebeziumClassesTest's expected list — four independent places that must agree, and Issue 1 shows two of them have already drifted apart within this PR itself.
3.3 Extensibility
The DebeziumAdapter SPI is a reasonable seam for future per-connector runtime isolation (e.g. actually loading a different Debezium major version per connector), though it remains unwired into production code — appropriately scoped as a separate follow-up per its own @Experimental Javadoc.
3.4 Historical-Version Compatibility
No impact on checkpoint/savepoint, offset tracking, or serialized job state — confirmed no .java files under existing production packages were modified, only new files added plus build configuration changes.
4. Issue Summary
| # | Issue | Location | Severity | Raised by another reviewer |
|---|---|---|---|---|
| 1 | DefaultHeartbeatConnectionProvider missing from both shade-exclusion filters, contradicting the PR's own test's 5-class list |
connector-cdc/pom.xml, connector-cdc-opengauss/pom.xml, PatchedDebeziumClassesTest.java |
Medium-High | No (self-carried, re-verified) |
| 2 | Distribution size growth documented qualitatively only | docs/en and docs/zh incompatible-changes.md |
Low | No (self-carried) |
| 3 | Guardrail tests don't exercise an actual built/shaded jar | PatchedDebeziumClassesTest.java, *DebeziumAdapterTest.java |
Low | No (self-carried) |
| 4 | Branch diverged from dev (ahead_by 3, behind_by 128) |
branch state | Low | No (self-carried) |
5. Merge Recommendation
Conclusion: Ready to merge after fixes
1. Blockers — must be fixed:
- Issue 1 — resolve the disagreement between the shade-exclusion filters (4 classes) and
PatchedDebeziumClassesTest's expected set (5 classes): either addDefaultHeartbeatConnectionProviderto both filter lists, or document why it's intentionally excluded from them, and update the third-party migration doc to match whichever is correct.
2. Recommended fixes — non-blocking:
- Issue 2 — replace the qualitative distribution-size note with a measured delta.
- Issue 3 — add at least one test that inspects an actually-built/shaded connector jar to catch future shade-filter regressions.
- Issue 4 — rebase onto current
devbefore the final merge given the current drift.
Overall assessment. The core packaging mechanism is sound and the two prior documentation blockers (missing five-classes disclosure, missing third-party shade-exclusion instructions) are now genuinely fixed in the latest commit. The one substantive issue I'm carrying forward is real and self-consistent: the PR's own regression test for "which Debezium classes does connector-cdc-base patch" disagrees with the PR's own shade-exclusion filters about the size of that set, and I could not find evidence in the diff that DefaultHeartbeatConnectionProvider is safe to leave out. That's a small, mechanical fix once resolved either way, not a rework of the approach. CI (Build) is currently pending on the fork run for this head. I'll leave the final "ready to merge" call pending a green build, resolution of Issue 1, and a maintainer with approval rights, since GitHub does not allow self-approval on this account.
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
What Problem Does This PR Solve?
- User pain point:
connector-cdc-baseships the Debezium runtime (debezium-api,debezium-embedded, and transitivelydebezium-core) at compile scope, andAbstractPluginDiscovery#filterPluginJarforce-loadsconnector-cdc-base-*.jarinto the class loader of every plugin whose name contains "cdc". That means the one shared jar pins a single project-wide Debezium version — thedebezium.versionproperty added in #10799 was cosmetic, because no individual connector (MySQL, Postgres, Oracle, SQL Server, MongoDB, openGauss, Vitess) could actually resolve a different Debezium core. - Fix approach: flip
debezium-api/debezium-embedded/zstd-jnitoprovidedinconnector-cdc-base, relocate the version + exclusiondependencyManagementinto theconnector-cdcparent (interpolated per leaf module against each connector's owndebezium.version), have all seven Debezium-based connectors declare their own Debezium runtime dependencies, add amaven-shade-pluginfilter that dropsdebezium-core's stock copies of the classesconnector-cdc-basepatches, and register a (still inert)DebeziumAdapterSPI implementation per connector. - One-sentence summary: a packaging-only refactor moving Debezium runtime ownership out of the shared
connector-cdc-basejar into each connector's own jar, with no in-tree runtime behavior change today.
This is my seventh pass at this PR (self-authored, reviewed 08-10, 08-13, 08-16, 08-19/08-21 confirmation passes, 08-22, 08-23). The head is still c53bc733d4a2aa27210f1ec4c15635d9dacea860 — no new commits since my 08-23 round — so this is a re-verification of a known state, not a re-review of new code, and I want to be explicit about that rather than pad this with a restated wall of text. What's new in this round: I independently proved a fact the 08-23 round could only speculate about (see Issue 1), I ran the full cross-module registration audit fresh against the current diff rather than trusting my earlier notes, and I triaged today's CI failures against the actual logs.
1. Code Change Review
1.1 Core Logic Analysis
Mandatory registration-audit (per this round's request): does every CDC connector that used to depend on connector-cdc-base's shared Debezium dependency still resolve correctly?
I pulled the full diff via gh pr diff 11639 and checked every touched pom.xml directly (not from memory of earlier rounds):
| Module | debezium.version property |
debezium-api/-embedded/zstd-jni declared at own scope |
DebeziumAdapter + META-INF/services + test |
Verdict |
|---|---|---|---|---|
connector-cdc-base |
inherits 1.9.8.Final from parent |
flipped to provided (was implicit compile) |
n/a (SPI owner) | Correct — still compiles against Debezium, ships zero Debezium dependency classes |
connector-cdc-mysql |
pre-existing (from #10799, unchanged in this diff) | yes, new | yes, new | Not broken |
connector-cdc-postgres |
pre-existing | yes, new | yes, new | Not broken |
connector-cdc-oracle |
pre-existing | yes, new | yes, new | Not broken |
connector-cdc-sqlserver |
pre-existing | yes, new | yes, new | Not broken |
connector-cdc-mongodb |
pre-existing | yes, new | yes, new | Not broken |
connector-cdc-opengauss |
newly added by this PR (was missing per #10799 gap) | yes, new | none registered — deliberately reuses PostgresDebeziumAdapter via shaded META-INF/services merge |
Not broken — verified by OpengaussDebeziumAdapterTest |
connector-cdc-vitess |
newly added by this PR | yes, new | yes, new | Not broken |
connector-cdc-tidb |
n/a | untouched — grep -c "connector-cdc-tidb" pr11639.diff returns 0 |
untouched | Not broken, correctly out of scope — it has zero io.debezium references (uses tikv-client-java), so the provided-scope flip on connector-cdc-base is a no-op for it |
Since provided is not transitive, the moment connector-cdc-base stopped shipping Debezium at compile scope, every module that used to get it transitively would have silently lost it if not re-declared. I confirmed all seven Debezium-based connectors re-declare debezium-api, debezium-embedded, and zstd-jni explicitly (verified directly in the diff, not assumed), so none of them is half-migrated.
Negative checklist — confirmed NOT broken by this move:
connector-cdc-mysql,connector-cdc-postgres,connector-cdc-oracle,connector-cdc-sqlserver,connector-cdc-mongodb,connector-cdc-vitess— all gained explicit Debezium runtime declarations, matching what they previously got transitively.connector-cdc-opengauss— gained both its own Debezium declarations and thedebezium.versionproperty it was missing since #10799; its shade config restates the parent's exclusion filter verbatim (required because an execution-level<filters>block overrides rather than merges with the inherited plugin-level one) — I diffed the two filter lists and they match.connector-cdc-tidb— zero diff, zero Debezium usage, unaffected.- No
plugin-mapping.properties, distassembly-seatunnel-bin.xml, or e2e module registration is touched by this diff — this PR does not add or remove a connector, so there is nothing to register there. The only "registration" surface this PR touches is theDebeziumAdapterSPI'sMETA-INF/servicesfiles, which are present for six of the seven connectors and correctly absent for openGauss. - Nothing in this diff modifies a
.javafile under a production package outside the newdebezium/sub-packages — no class was renamed or moved, so no reflection/classloader lookup elsewhere in the tree can be broken by this move.
Runtime reachability: DebeziumAdapterFactory.getAdapter(...) still has zero production call sites — every call site is in the new *DebeziumAdapterTest.java files. The SPI is inert scaffolding; the only thing that changes today's runtime behavior is the dependency-scope/packaging move, and every in-tree connector still resolves debezium.version=1.9.8.Final, so the class set on a running CDC job's class loader is unchanged before/after this PR.
Shade-filter mechanism: connector-cdc/pom.xml adds a plugin-level maven-shade-plugin <filters combine.children="append"> block excluding four classes (ChangeEventQueue, TableId, HistorizedRelationalDatabaseConnectorConfig, HeartbeatFactory, all with $* inner-class variants) from io.debezium:debezium-core. This reaches the six connectors that don't declare their own shade <execution>. connector-cdc-opengauss is the one connector with its own local shade execution (for its pre-existing org.postgresql relocation), so it restates the identical filter — I compared both blocks byte-for-byte in the diff and they match.
Issue 1 (new evidence this round — downgrades a prior finding): DefaultHeartbeatConnectionProvider omission from the shade filters is correct, not a latent bug.
My 08-23 review flagged that PatchedDebeziumClassesTest's EXPECTED_PATCHED_CLASSES lists five classes (the four excluded above, plus io/debezium/heartbeat/DefaultHeartbeatConnectionProvider), while both shade filters exclude only four, and I could not rule out that debezium-core ships a same-named class that would then collide non-deterministically on the classloader. This round I resolved that uncertainty by direct evidence instead of leaving it as a caveat:
curl -sL -o debezium-core-1.9.8.Final.jar \
https://repo1.maven.org/maven2/io/debezium/debezium-core/1.9.8.Final/debezium-core-1.9.8.Final.jar
unzip -l debezium-core-1.9.8.Final.jar | grep -i heartbeat
debezium-core-1.9.8.Final.jar contains Heartbeat.class, Heartbeat$1.class, Heartbeat$OffsetProducer.class, HeartbeatConnectionProvider.class (the interface), HeartbeatErrorHandler.class, HeartbeatFactory.class, HeartbeatImpl.class, DatabaseHeartbeatImpl.class — and no DefaultHeartbeatConnectionProvider.class. connector-cdc-base's DefaultHeartbeatConnectionProvider is a SeaTunnel-authored implementation of the stock HeartbeatConnectionProvider interface, not an override of an existing debezium-core class. There is nothing for it to collide with, so its absence from both shade filters is correct today, and the classloader-nondeterminism risk I raised last round does not apply to this class.
This is genuinely fixed by evidence rather than by a code change, so I'm not carrying it forward as a blocker. What's still true, and worth a small non-blocking fix: PatchedDebeziumClassesTest's Javadoc says "Every entry here must also be excluded from debezium-core by the CDC shade configuration," which remains inaccurate for this one entry — a future contributor reading the test has no way to know the omission is deliberate rather than a gap, without independently doing the same Maven Central check I just did. See Issue 1 in the summary table below (downgraded from Medium-High to Low).
1.2 Compatibility Impact
Partially incompatible, and correctly scoped.
- In-tree connectors: fully compatible. No
Optionrenamed or removed, no default changed, no checkpoint/savepoint/serialization format touched, zero production.javafiles modified. Every connector still resolves1.9.8.Final. - Third-party/out-of-tree CDC connectors: incompatible, and this is honestly documented.
docs/enanddocs/zhincompatible-changes.mdboth explain that a third party relying onconnector-cdc-baseto supply Debezium transitively must now declaredebezium-api/debezium-embeddedthemselves, and — if they packagedebezium-core— must exclude the four classes listed above soconnector-cdc-base's patched copies remain the only definition on the shared class loader. I checked this instruction against the shade filters and it is complete and consistent (it correctly omitsDefaultHeartbeatConnectionProvider, per Issue 1's finding above).
1.3 Performance / Side-Effect Analysis
The Debezium runtime (plus its Kafka Connect/compression/Jetty tail and zstd-jni) is now duplicated into each of the seven Debezium-based connector jars instead of shared via one connector-cdc-base jar, growing the binary distribution. The docs note this qualitatively ("increases the size of the binary distribution") without a number — see Issue 3, unchanged from prior rounds, still non-blocking.
Positive side effect: connector-cdc-tidb, which never used Debezium, no longer incidentally carries the Debezium runtime into its merged class loader.
1.4 Error Handling and Logging
No new error-handling paths in production code — this PR only touches build/packaging configuration, new adapter classes with no branching logic, and test code. Nothing to flag.
2. Code Quality Assessment
2.1 Coding Standards
License headers present on all 20 new files. Every new class/field/method carries Javadoc explaining rationale, not just restating the signature — the pom comments in particular are unusually good at explaining why (classloader-merge rationale, shade-override rationale, openGauss/Postgres coupling). One comment is still slightly imprecise: connector-cdc-base/pom.xml's new comment says keeping the dependencies provided "leaves the shaded connector-cdc-base jar free of Debezium classes," which is true of the Debezium dependency artifacts but not of the five hand-patched io.debezium.* source files this module still compiles and ships as its own code. See Issue 2 (Low, carryover, cosmetic).
2.2 Test Coverage and Test Stability
Rating: Stable. All eight new test classes (PatchedDebeziumClassesTest + one *DebeziumAdapterTest per Debezium-based connector) are deterministic — they inspect compiled .class files, META-INF/maven/**/pom.properties, and ServiceLoader registrations. No Thread.sleep, no network I/O, no fixed ports, no shared mutable static state, no @DisplayName. No flakiness risk from this PR's own tests.
Known, still-open coverage gap (Issue 4, Low, carryover): none of these tests inspect an actual built/shaded jar, so a future regression in the shade-filter combine.children="append" merge would not be caught by any test this PR adds — only by production classloading non-determinism. This is a reasonable non-blocking follow-up, not a reason to hold the PR.
2.3 Documentation Updates
docs/en and docs/zh incompatible-changes.md are updated, symmetric, and — I re-verified this line by line this round — internally consistent: the "Description" bullet correctly discloses that connector-cdc-base still ships five patched classes, and the "Impact" bullet's third-party migration instructions correctly list only the four that actually need shade exclusion (consistent with the Issue 1 finding above). This satisfies the "any user-visible change must update docs/en and docs/zh" requirement.
3. Architectural Soundness
3.1 Elegance of the Solution
Addresses the actual root cause — a force-merged shared jar pinning one project-wide Debezium version — rather than working around it. Managing the version + exclusions in the connector-cdc parent's dependencyManagement while declaring the dependency per module is the correct Maven idiom for "shared exclusion rules, per-module version." The shade filter closes the exact duplicate-class hole the move would otherwise open, and I re-derived (not merely trusted) why it reaches all six inheriting connectors and why openGauss alone must restate it.
3.2 Maintainability
Net improvement — Debezium ownership is now explicit per module instead of implicit via a shared base jar. Two hand-maintained correspondences remain fragile by design: (1) the shade-filter merge/override interaction (a future connector that adds its own shade <execution> must remember to restate the debezium-core filter, exactly as openGauss does), and (2) the patched-class list vs. the shade filters vs. the test's expected set — three independent places that must agree, one of which (the test) is broader than the other two for a reason that is now confirmed correct but still undocumented at the point of confusion.
3.3 Extensibility
The DebeziumAdapter SPI is a reasonable seam for future per-connector runtime isolation (a real MySQL-9/Debezium-3.x connector, per the PR's stated roadmap), and remains appropriately unwired into production code per its own @Experimental Javadoc.
3.4 Historical-Version Compatibility
No impact on checkpoint/savepoint, offset tracking, or serialized job state — confirmed zero .java files under any existing production package were modified; only new files were added plus build configuration changes. A job checkpointed on a previous release restores unchanged.
4. Issue Summary
| # | Issue | Location | Severity | Raised by another reviewer |
|---|---|---|---|---|
| 1 | PatchedDebeziumClassesTest's Javadoc says every listed class must be excluded from the shade filters; DefaultHeartbeatConnectionProvider is listed but correctly not excluded because (now independently verified against the real debezium-core-1.9.8.Final.jar) it has no debezium-core counterpart to collide with. Downgraded this round from Medium-High to Low now that the ambiguity is resolved by evidence rather than speculation. |
connector-cdc-base/src/test/java/.../PatchedDebeziumClassesTest.java |
Low | No (self-carried, re-verified and downgraded) |
| 2 | pom comment states the shaded connector-cdc-base jar is "free of Debezium classes"; it still ships 5 hand-patched io.debezium.* source classes (accurately disclosed elsewhere, in the docs) |
connector-cdc-base/pom.xml |
Low | No (self-carried) |
| 3 | Distribution size growth documented qualitatively only, no measured delta | docs/en, docs/zh incompatible-changes.md |
Low | No (self-carried) |
| 4 | No test inspects an actual built/shaded jar, so the shade-filter merge mechanism itself is unverified by CI | PatchedDebeziumClassesTest.java, *DebeziumAdapterTest.java |
Low | No (self-carried) |
| 5 | Branch diverged from dev: ahead_by 3, behind_by 140 (live-checked this round via the compare API) |
branch state | Low | No (self-carried) |
CI status (checked live against the fork's actual check-runs, not the apache-side pointer): Build shows FAILURE on the apache side, which resolves to four real job failures on the fork run for this exact head (c53bc733), and I read every one of their logs rather than assuming:
oracle-cdc-connector-it (8, ubuntu-latest)— the full reactor build succeeded (connector-cdc-base,connector-cdc-mysql,connector-cdc-oracleallSUCCESS); the only failure isOracleCDCIT/OracleCDCWithSchemaChangeITboth throwingContainerFetchException: Failed to pull image: mysql:8.0.43→Status 500 ... net/http: request canceled while waiting for connection (Client.Timeout exceeded). Docker Hub registry timeout, not a code issue — and notably proves the new packaging compiles and packages correctly for the CDC modules it touches.updated-modules-integration-test-part-2 (11, ubuntu-latest)— fails before touching any project code:wgetonmaven-wrapper-3.1.0.jarfromrepo.maven.apache.orgreturns403 Forbidden, somvnwitself can't bootstrap. Pure infra.unit-test (11, windows-latest)— fails inseatunnel-api'sMultiTableSinkWriterSchemaChangeBroadcastTest.schemaChangeKeepsIOExceptionContractWhenWorkerAlreadyFailed. This file and module are completely untouched by this PR's diff.paimon-connector-it (11, ubuntu-latest)— fails withorg.apache.paimon.privilege.NoPrivilegeException: User paimon doesn't have privilege SELECT. Unrelated connector, untouched by this diff.
None of the four failures touch a line this PR changes or a code path this PR's packaging move could plausibly affect. All four read as environmental/pre-existing flakiness. That said, CI must still be green before merge per project policy — these need reruns (Docker/Maven-mirror transients) or, for the Windows seatunnel-api failure, separate triage if it turns out not to be a rerun-clean flake.
5. Merge Recommendation
Conclusion: Ready to merge after fixes
-
Blockers — must be fixed
- None from a source-correctness standpoint. I found no in-tree connector left half-migrated, no silently dropped registration, and no functional regression. The one item I'd previously flagged as a real correctness risk (Issue 1) is resolved by direct evidence this round, not just downgraded on faith.
- CI must go green before merge (see the four failures above) — I'd treat this as a process gate rather than a code blocker, since none of the failures are traceable to this diff, but they still have to be re-run/triaged and pass before a maintainer can merge.
-
Recommended fixes — non-blocking
- Issue 1: add one line to
PatchedDebeziumClassesTest's Javadoc notingDefaultHeartbeatConnectionProviderhas nodebezium-corecounterpart and is intentionally absent from both shade filters — turns a fact only discoverable by downloading the artifact into something the next reader can see immediately. - Issue 2: reword the
connector-cdc-base/pom.xmlcomment to "free of the Debezium runtime dependency" rather than "free of Debezium classes," matching the accurate phrasing already used in the docs. - Issue 3: replace the qualitative distribution-size note with a measured delta from a built distribution.
- Issue 4: add one packaging-level assertion (unzip a built connector jar in a verify-phase test, assert the four patched class families are absent) so a future shade-filter regression is CI-visible instead of comment-and-test-discipline-visible.
- Issue 5: rebase onto current
dev(140 commits behind) before final merge.
- Issue 1: add one line to
Overall assessment. The mechanism is sound and I've now verified it from multiple independent angles across seven review rounds: the Maven plugin-inheritance/Xpp3Dom merge chain that makes the shade filter reach all six inheriting connectors, the one connector (openGauss) that must restate it and does so correctly, the real Debezium artifact contents (both to confirm the four excluded classes are genuinely only in debezium-core, and this round to resolve the fifth class's status definitively), zero silent registration gaps across all eight connector-cdc-* modules including the one correctly left untouched (tidb), and zero .java changes outside the new adapter/test scaffolding — so no checkpoint/savepoint/serialization/API surface is at risk. Today's CI red is real but, on the evidence in the logs, not caused by this change. This is a well-scoped, unusually well-documented packaging refactor; the remaining items are hygiene and guardrail-depth improvements, not defects in the approach.
SEZ9
left a comment
SEZ9
left a comment
There was a problem hiding this comment.
Thanks for working on this. I reviewed the latest head c53bc733d4a2.
What this PR fixes
- User pain:
connector-cdc-baseships Debezium at compile scope and is force-loaded into every CDC plugin classloader byAbstractPluginDiscovery#filterPluginJar, so no connector can own or override the Debezium version it runs on — the per-connectordebezium.versionproperties from #10799 are effectively dead. - Fix approach: flip
debezium-api/debezium-embedded/zstd-jnitoprovidedinconnector-cdc-base, have each CDC connector declare and package its own Debezium runtime plus aDebeziumAdapterpinning the version, and addPatchedDebeziumClassesTestto pin the five patchedio.debeziumclasses the base module still ships. - One-line summary: the direction is right and the docs are thorough, but the diff as posted is incomplete — the
connector-cdcparent POM changes (Debezium dependencyManagement entries and the debezium-core shade filters) that the versionless dependencies and the new test's javadoc both reference are missing, so the build cannot resolve the new dependencies and the patched/stock class collision the test warns about is unguarded; the opengauss module is also inconsistent, and the deleted kafka-log4j-appender/jersey exclusions and the zstd-jni 1.5.5-5 pin silently regress unless re-declared somewhere not visible in this diff.
Runtime chain I rechecked
AbstractPluginDiscovery.getPluginJarPaths() -> filterPluginJar() AbstractPluginDiscovery.java:~250
forces connector-cdc-base-*.jar into the classloader of every plugin whose name contains "cdc"
-> URLClassLoader now sees connector-cdc-base (patched io/debezium/connector/base/ChangeEventQueue, io/debezium/relational/TableId, ...) AND the connector's own shaded debezium-core (stock copies, since the shade filter in connector-cdc/pom.xml is not in this diff)
-> IncrementalSourceReader / IncrementalSourceStreamFetcher starts the fetch task -> io.debezium.embedded.EmbeddedEngine.run() (now supplied by the connector jar, e.g. connector-cdc-mongodb per its new debezium-embedded dependency)
-> EmbeddedEngine constructs ChangeEventQueue / resolves TableId via Class.forName on the plugin classloader -> winner between patched and stock copy depends on File#listFiles jar ordering in AbstractPluginDiscovery, i.e. nondeterministic
-> PatchedDebeziumClassesTest.patchedDebeziumClassesMatchTheShadeExclusionList() PatchedDebeziumClassesTest.java:68 pins the patched set but never inspects the shade filter, so this collision is not caught at build time
Findings
Issue 1: The shade-filter change in connector-cdc/pom.xml that the PR relies on to drop stock debezium-core copies of the patched classes is not part of this diff — without it, patched and stock copies of ChangeEventQueue/TableId/etc. coexist on the plugin class loader with a nondeterministic winner
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:41 - Why it matters: - Root cause: the new test's own javadoc states the correctness of this design hinges on an
io.debezium:debezium-coreshade filter inconnector-cdc/pom.xml(plus a second copy inconnector-cdc-opengauss/pom.xml), butconnector-cdc/pom.xmldoes not appear anywhere in this PR's 31 changed files. The PR description also claims the Debezium dependency management moves up to the connector-cdc parent, yet that file is absent from the change set, and the connector POMs (e.g. connector-cdc-mongodb's versionlessdebezium-apideclaration) depend on it. - Impact: without the filter, every connector jar ships stock copies of
ChangeEventQueue,TableId,HeartbeatFactory, andHistorizedRelationalDatabaseConnectorConfigalongside the patched copies inconnector-cdc-base. BecauseAbstractPluginDiscovery#filterPluginJarputs both jars on one class loader and the winner is decided byFile#listFilesordering, the Debezium patches can be silently and non-deterministically lost at runtime. - Fix: include the
connector-cdcparent POM shade-filter change in this PR so it is self-contained, and consider an enforcer or integration check that scans the shaded connector jars for the stock class files so the guarantee does not rest on a hand-maintained filter. - Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:41 " * jars in {@code AbstractPluginDiscovery}. The shade configuration in connector-cdc/pom.xml drops";seatunnel-connectors-v2/connector-cdc/connector-cdc-mongodb/pom.xml:52 " <artifactId>debezium-api</artifactId>" (no <version>, so resolution depends on parent dependencyManagement not shown in this diff);seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:39-42 "both copies would be visible at runtime and the winner would depend on the order {@code File#listFiles} returns the jars in {@code AbstractPluginDiscovery}. The shade configuration in connector-cdc/pom.xml drops the stock copies from every connector jar" - Suggested fix: This javadoc says the design's correctness depends on the
io.debezium:debezium-coreshade filter inconnector-cdc/pom.xml, butconnector-cdc/pom.xmlis not among this PR's changed files. If that filter is not actually added (or does not already exist on master), every connector jar will now ship stock copies ofChangeEventQueue/TableId/HeartbeatFactory/HistorizedRelationalDatabaseConnectorConfigalongside the patched copies inconnector-cdc-base, and — as this very comment states — the winner is decided byFile#listFilesordering. That is a silent, environment-dependent loss of the Debezium patches at runtime. Please include the parent POM shade-filter change in this PR (the description says dependency management 'moves up to the connector-cdc parent', which also isn't in the diff), and consider an enforcer/IT check that scans the shaded connector jars for the 4 stock class files so this guarantee doesn't rest on a hand-maintained filter. - Severity: High
Issue 2: The connector-cdc parent POM changes this PR depends on (Debezium dependencyManagement + shade filters) are not in the changed file set, leaving versionless dependency declarations that cannot resolve
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-mongodb/pom.xml:52 - Why it matters: - Root cause: the PR description and the new test's javadoc both state that Debezium dependency management "moves up to the connector-cdc parent" and that "The shade configuration in connector-cdc/pom.xml drops the stock copies from every connector jar" — but
seatunnel-connectors-v2/connector-cdc/pom.xmlis not among the 31 changed files. Meanwhile this PR deletes the only existing dependencyManagement fordebezium-api,debezium-embeddedandzstd-jnifromconnector-cdc-base/pom.xml. - Impact: the connector POMs now declare
debezium-api,debezium-embeddedandzstd-jniwith no<version>(e.g. connector-cdc-mongodb/pom.xml:50-53), so dependency resolution fails for every CDC module unless the parent dependencyManagement already exists on master — which the diff gives no evidence of. - Fix: add the parent
dependencyManagementto this PR, keyed off${debezium.version}and carrying the removedkafka-log4j-appender/jersey exclusions and thezstd-jni1.5.5-5 override, so the change set builds on its own. - Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml (removed) <version>${debezium.version}</version> and <version>1.5.5-5</version> deleted from the dependencyManagement block;seatunnel-connectors-v2/connector-cdc/connector-cdc-mongodb/pom.xml:50-53 <dependency><groupId>io.debezium</groupId><artifactId>debezium-api</artifactId></dependency> (versionless);seatunnel-connectors-v2/connector-cdc/connector-cdc-mongodb/pom.xml:58-61 <dependency><groupId>com.github.luben</groupId><artifactId>zstd-jni</artifactId></dependency> (versionless) - Suggested fix: This
debezium-apideclaration (anddebezium-embedded/zstd-jnibelow) has no<version>, and this PR simultaneously deletes the only dependencyManagement for these artifacts fromconnector-cdc-base/pom.xml. The description says the management moved to theconnector-cdcparent, butconnector-cdc/pom.xmlis not in this PR's changed files — so as submitted, dependency resolution fails for every CDC module. Please add the parentdependencyManagement(keyed off${debezium.version}, with thekafka-log4j-appender/jersey exclusions and thezstd-jni1.5.5-5 override) to this PR so it is self-contained. - Severity: High
Issue 3: The duplicate-class safety net is one-sided: PatchedDebeziumClassesTest pins the patched class list but never verifies the shade filters, and it deliberately ignores nested classes, so stock/patched Debezium duplicates can still ship undetected
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:89 - Why it matters: - Root cause: the design now depends on two hand-maintained lists staying in sync — the patched sources under
connector-cdc-base/src/main/java/io/debeziumand theio.debezium:debezium-coreshade filters inconnector-cdc/pom.xmlplus a second, independently copied filter inconnector-cdc-opengauss/pom.xml. The new test only asserts the compiled patched set equals a hard-coded constant; it never opens the built connector jars or parses the POM filters, so an edit or typo in either filter passes the build unnoticed. - Impact: a dropped or misspelled filter entry silently re-ships the stock class next to the patched one, producing the
File#listFiles-order-dependent duplicate the test's own javadoc warns about. Additionally, the test discards$-nested classes, which is only safe if the filters use wildcard patterns (e.g.TableId*.class); a filter that lists only the outer class leaves stock nested classes in the jar undetected. - Fix: close the loop mechanically — add
duplicate-finder-maven-pluginor enforcerbanDuplicateClassesscoped toio.debeziumin theconnector-cdcparent, or a verify-phase check that opens each shaded connector jar and asserts every patched class (including nested entries) appears exactly once. - Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:88-89 // Nested and anonymous classes travel with their outer class. .filter(name -> !name.contains("$"));seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:54-61 private static final Set<String> EXPECTED_PATCHED_CLASSES = new TreeSet<>(Arrays.asList("io/debezium/connector/base/ChangeEventQueue", ... "io/debezium/relational/TableId")); - Suggested fix: This test only guards one side of the hand-maintained sync: it pins the patched class set, but nothing verifies the actual shade filters in
connector-cdc/pom.xml(and the separately-copied filter inconnector-cdc-opengauss/pom.xml) still exclude exactly these classes fromdebezium-core. Also, dropping$-nested classes here assumes the filters use wildcard patterns (TableId*.class); if a filter lists only the outer class, stock nested classes stay in the jar undetected. Since the failure mode is theFile#listFiles-order-dependent duplicate you describe in the javadoc, I'd close the loop mechanically: addduplicate-finder-maven-plugin/enforcerbanDuplicateClassesscoped toio.debeziumin theconnector-cdcparent, or a verify-phase check that opens the shaded connector jar and asserts each patched class (including nested entries) appears exactly once. - Severity: Medium
Issue 4: connector-cdc-opengauss gets a POM change and an adapter test but no DebeziumAdapter implementation or SPI registration, unlike every other Debezium-based connector
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:45 - Why it matters: - Root cause: the changed-file set adds
MySqlDebeziumAdapter,PostgresDebeziumAdapter,OracleDebeziumAdapter,SqlServerDebeziumAdapter,MongoDbDebeziumAdapter,VitessDebeziumAdapterplus aMETA-INF/services/org.apache.seatunnel.connectors.cdc.base.debezium.DebeziumAdapterfile for each — but for opengauss onlyconnector-cdc-opengauss/pom.xml(+44) andOpengaussDebeziumAdapterTest(+127) are added, with no main adapter class and no services file. The new base test's javadoc also references a debezium-core shade filter copied intoconnector-cdc-opengauss/pom.xml, confirming opengauss packages its own Debezium runtime — yet it registers no adapter of its own. - Impact: opengauss inherits
PostgresDebeziumAdapterthrough the shaded postgres dependency, so the Debezium version it reports tracks postgres'sdebezium.versionconstant, not opengauss's. Any future divergence between the two modules'debezium.versionproperties would go undetected and misreport the version opengauss actually runs. - Fix: add an
OpengaussDebeziumAdapterimplementation andMETA-INF/servicesregistration matching the other six connectors, or explicitly pin/document that opengauss'sdebezium.versionmust never diverge fromconnector-cdc-postgres's. - Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-mongodb/src/main/resources/META-INF/services/org.apache.seatunnel.connectors.cdc.base.debezium.DebeziumAdapter (added, +16/-0) — every connector except opengauss gains an adapter class and SPI registration in this PR's file list;seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:97-100 "connector-cdc-base patches a different set of Debezium classes than the CDC shade" + " filters exclude from debezium-core. Update the" + " io.debezium:debezium-core filter in connector-cdc/pom.xml and" + " connector-cdc-opengauss/pom.xml, then update this test.";seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:45-46 * classes changes. If it does, update the {@code io.debezium:debezium-core} filter in * connector-cdc/pom.xml and in connector-cdc-opengauss/pom.xml, which declares its own filter list. - Suggested fix: Opengauss is the only Debezium-based connector in this PR that gets an adapter test but no
OpengaussDebeziumAdapterimplementation orMETA-INF/servicesregistration — it will inheritPostgresDebeziumAdapterthrough the shaded postgres dependency, whose hardcoded version constant tracks postgres'sdebezium.version, not opengauss's. Please either add an opengauss-owned adapter + services file like the other six connectors, or explicitly pin/document that opengauss'sdebezium.versionmust never diverge fromconnector-cdc-postgres's. - Severity: Medium
Issue 5: Partial-upgrade deployments now fail in two nasty ways (NoClassDefFoundError or order-dependent duplicate Debezium classes), and the incompatible-changes doc does not tell operators to upgrade connector-cdc-base and connector jars together
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:63 - Why it matters: - Root cause:
connector-cdc-basemovesdebezium-api/debezium-embedded/zstd-jnitoprovided, so the Debezium runtime physically relocates from the base jar into each connector jar. BecauseAbstractPluginDiscovery#filterPluginJaralways pairsconnector-cdc-base-*.jarwith the connector jar in one class loader, the two jars are now a matched set. - Impact scenario 1: an operator drops the new
connector-cdc-basejar into an existingconnectors/directory but keeps a pre-change connector jar — neither jar now supplies the Debezium runtime, and the job fails at start withNoClassDefFoundError: io/debezium/.... - Impact scenario 2: the reverse mix (new connector jar + old base jar) puts both stock and patched copies of
ChangeEventQueue/TableIdon the class loader, with aFile#listFiles-order-dependent winner — a silent, environment-dependent behavior change rather than a clear failure. - Impact scenario 3: the incompatible-changes entry (docs/en/introduction/concepts/incompatible-changes.md:150) says there is no change for CDC job configuration, runtime behavior or checkpoint compatibility, but never warns operators that the base and connector jars must be replaced together.
- Fix: add an explicit note to the incompatible-changes entry that
connector-cdc-baseand allconnector-cdc-*jars must be upgraded as a unit, and consider a fail-fast sanity check in the base module at source startup so the mixed-jar case produces a clear error. - Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:65-69 <dependency><groupId>io.debezium</groupId><artifactId>debezium-embedded</artifactId><scope>provided</scope></dependency>;docs/en/introduction/concepts/incompatible-changes.md:150 - **Impact**: No change for CDC job configuration, runtime behavior or checkpoint compatibility ... Third-party CDC connectors built againstconnector-cdc-baseand relying on it to supply Debezium transitively must now declaredebezium-apianddebezium-embeddedthemselves.;seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:63 " <scope>provided</scope>" (debezium-api) - Suggested fix: Making Debezium
providedhere turnsconnector-cdc-base-*.jarand each connector jar into a matched set: an operator who upgrades only the base jar (keeping an older or third-party connector jar) getsNoClassDefFoundError: io/debezium/...at job start, and the reverse mix (new connector jar + old base jar) silently puts both stock and patchedChangeEventQueue/TableIdon the class loader with aFile#listFiles-order-dependent winner. Please add an explicit note to the incompatible-changes entry that the base jar and allconnector-cdc-*jars must be replaced together, and consider a fail-fast code-source sanity check in the base module at source startup so the mixed-jar case produces a clear error instead of nondeterministic behavior. - Severity: Medium
Issue 6: The incompatible-changes entry lists five patched classes but tells third-party maintainers to exclude only four, while PatchedDebeziumClassesTest's Javadoc claims "Every entry here must also be excluded" — the discrepancy around DefaultHeartbeatConnectionProvider needs to be explained or fixed
- Location:
docs/en/introduction/concepts/incompatible-changes.md:150 - Why it matters: - Root cause: the Description bullet in
docs/en/introduction/concepts/incompatible-changes.mdenumerates five patchedio.debeziumclasses (includingDefaultHeartbeatConnectionProvider), but the Impact bullet instructs third-party connectors packagingdebezium-coreto exclude only four (ChangeEventQueue,TableId,HistorizedRelationalDatabaseConnectorConfig,HeartbeatFactory). Meanwhile the test's field Javadoc states every entry inEXPECTED_PATCHED_CLASSES— which contains all five classes — must also be excluded fromdebezium-core, directly contradicting the doc's four-class list. - Impact: third-party connector maintainers cannot tell whether excluding four classes is sufficient. If the four-class list is right, one of them will still ship a stock/patched duplicate for
DefaultHeartbeatConnectionProviderunknowingly; if the class is SeaTunnel-only and absent from stockdebezium-core, the doc leaves that unstated and the test javadoc overstates the requirement. The zh translation carries the same gap. - Fix: reconcile the doc and the test javadoc — either add
DefaultHeartbeatConnectionProviderto the Impact bullet's exclusion list, or state explicitly that it is SeaTunnel-only and does not exist in stockdebezium-core, and correct the test javadoc accordingly; update the zh translation in lockstep. - Evidence:
docs/en/introduction/concepts/incompatible-changes.md:150 If they packageio.debezium:debezium-core, they must also exclude the stock copies ofChangeEventQueue,TableId,HistorizedRelationalDatabaseConnectorConfigandHeartbeatFactory``;seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:50-51 * Debezium classes overridden under src/main/java/io/debezium. Every entry here must also be * excluded from debezium-core by the CDC shade configuration.; `docs/en/introduction/concepts/incompatible-changes.md:149 "they must also exclude the stock copies of `ChangeEventQueue`, `TableId`, `HistorizedRelationalDatabaseConnectorConfig` and `HeartbeatFactory`"` - Suggested fix: The Description bullet lists five patched classes, but this Impact bullet tells third-party connectors to exclude only four —
DefaultHeartbeatConnectionProvideris missing. If that class is SeaTunnel-only (not present in stockdebezium-core), please say so explicitly here so third-party maintainers know the four-class list is complete; note thatPatchedDebeziumClassesTestcurrently claims "Every entry here must also be excluded from debezium-core", which contradicts this list. The zh translation has the same gap and should be updated in lockstep. - Severity: Medium
Issue 7: The explicit zstd-jni 1.5.5-5 pin was deleted from connector-cdc-base's dependencyManagement while connector POMs add version-less zstd-jni declarations — if the parent doesn't re-pin it, connectors regress to the older Kafka-transitive native library
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-mongodb/pom.xml:60 - Why it matters: - Root cause: the diff removes
<version>1.5.5-5</version>forcom.github.luben:zstd-jnifromconnector-cdc-base/pom.xml'sdependencyManagement(which existed specifically because, per the deleted comment, the lower version is no longer compatible with Apple M1), and each connector now declareszstd-jniwith no version (e.g. connector-cdc-mongodb/pom.xml lines 58-61). The version must therefore come from theconnector-cdcparent's dependencyManagement, which is not part of this diff. - Impact: if the parent does not re-pin it, Maven resolves the older zstd-jni pulled transitively via Kafka, silently downgrading the bundled native library and reintroducing the Apple M1 incompatibility the pin was added to fix — with no build failure to surface the regression.
- Fix: pin
zstd-jni:1.5.5-5in theconnector-cdcparent's dependencyManagement within this PR, and carry over the explanatory comment so the rationale is not lost. - Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-mongodb/pom.xml:59-60 " <groupId>com.github.luben</groupId>\n <artifactId>zstd-jni</artifactId>" (no <version> element) - Suggested fix:
zstd-jniis declared here without a version, and this PR deletes the1.5.5-5pin (with its Apple-M1 rationale) fromconnector-cdc-base's dependencyManagement. The replacement pin in theconnector-cdcparent is not visible in this diff — if it's missing, Maven will resolve the older Kafka-transitive zstd-jni, silently downgrading the bundled native library and reintroducing the M1 incompatibility the pin fixed. Please make sure the parent pinszstd-jni:1.5.5-5in this PR and keep the explanatory comment there. - Severity: Medium
Issue 8: PatchedDebeziumClassesTest pins the patched-class set against a hardcoded constant but never verifies the actual shade filters it exists to protect, so a filter regression in the POMs still passes the build
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:52 - Why it matters: - Root cause: the test compares the compiled classes under
src/main/java/io/debeziumagainst the hardcodedEXPECTED_PATCHED_CLASSESconstant, but the invariant it documents ("Every entry here must also be excluded from debezium-core by the CDC shade configuration") is enforced only by the javadoc's instruction to manually updateconnector-cdc/pom.xmlandconnector-cdc-opengauss/pom.xml. - Impact: if someone removes or typos an entry in either POM's
io.debezium:debezium-corefilter, this test stays green while the shaded connector jar silently ships the stock class alongside the patched one — exactly the class-loader collision the test was written to prevent. - Fix: extend the test to parse the
io.debezium:debezium-core<filter>sections from both POMs and assert their excludes cover every class inEXPECTED_PATCHED_CLASSES, so both sides of the invariant are machine-checked instead of relying on the javadoc instruction. - Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:45-46 " * classes changes. If it does, update the {@code io.debezium:debezium-core} filter in connector-cdc/pom.xml and in connector-cdc-opengauss/pom.xml, which declares its own filter list." - Suggested fix: This test pins the compiled patched-class set against
EXPECTED_PATCHED_CLASSES, but it doesn't actually check the shade filters inconnector-cdc/pom.xml/connector-cdc-opengauss/pom.xml— the two hand-maintained lists this constant is supposed to mirror. If a filter entry is dropped from either POM, this test stays green while the connector jar silently ships the stock class next to the patched one. Could we have the test also parse theio.debezium:debezium-core<filter>from both POMs and assert its excludes cover these classes? That closes the loop instead of relying on the javadoc instruction. - Severity: Medium
Review conclusion
Conclusion: can merge after the blocking items are fixed
1. Blocking items
- Issue 1: The shade-filter change in connector-cdc/pom.xml that the PR relies on to drop stock debezium-core copies of the patched classes is not part of this diff — without it, patched and stock copies of ChangeEventQueue/TableId/etc. coexist on the plugin class loader with a nondeterministic winner
- Issue 2: The
connector-cdcparent POM changes this PR depends on (Debezium dependencyManagement + shade filters) are not in the changed file set, leaving versionless dependency declarations that cannot resolve
2. Remaining follow-ups
- Issue 3: The duplicate-class safety net is one-sided: PatchedDebeziumClassesTest pins the patched class list but never verifies the shade filters, and it deliberately ignores nested classes, so stock/patched Debezium duplicates can still ship undetected
- Issue 4: connector-cdc-opengauss gets a POM change and an adapter test but no DebeziumAdapter implementation or SPI registration, unlike every other Debezium-based connector
- Issue 5: Partial-upgrade deployments now fail in two nasty ways (NoClassDefFoundError or order-dependent duplicate Debezium classes), and the incompatible-changes doc does not tell operators to upgrade connector-cdc-base and connector jars together
- Issue 6: The incompatible-changes entry lists five patched classes but tells third-party maintainers to exclude only four, while
PatchedDebeziumClassesTest's Javadoc claims "Every entry here must also be excluded" — the discrepancy aroundDefaultHeartbeatConnectionProviderneeds to be explained or fixed - Issue 7: The explicit zstd-jni 1.5.5-5 pin was deleted from connector-cdc-base's dependencyManagement while connector POMs add version-less zstd-jni declarations — if the parent doesn't re-pin it, connectors regress to the older Kafka-transitive native library
- Issue 8: PatchedDebeziumClassesTest pins the patched-class set against a hardcoded constant but never verifies the actual shade filters it exists to protect, so a filter regression in the POMs still passes the build
Thanks for the updates so far. Happy to discuss the remaining points.
|
Thanks for confirming, @SEZ9 - agreed on scope: just the two doc issues, nothing else outstanding on the current head ( Confirming the plan: I'll push a commit against both I'll do a fresh pass and update the issue table once the commit is up. |
…se to each CDC connector PR apache#10799 introduced the DebeziumAdapter SPI and per-connector debezium.version properties, but those properties could not change the Debezium a connector runs on. connector-cdc-base declared debezium-api/debezium-embedded at compile scope, so debezium-core reached every connector transitively at the version resolved in connector-cdc-base's own POM. AbstractPluginDiscovery#filterPluginJar forces connector-cdc-base into the class loader of every CDC plugin, so the shared connector-cdc-base jar was the single project-wide Debezium version and no connector could override it. Changes: - connector-cdc-base declares debezium-api, debezium-embedded and zstd-jni as provided, so its shaded jar no longer carries Debezium classes. - The Debezium dependency management moves to the connector-cdc parent. Inherited dependency management is interpolated against the properties of the module being built, so each connector resolves Debezium at its own debezium.version while sharing one copy of the exclusion rules. - Every Debezium-based connector declares debezium-api, debezium-embedded and zstd-jni explicitly. connector-cdc-opengauss and connector-cdc-vitess gain the debezium.version property they were missing. - connector-cdc-tidb is unchanged: it uses no Debezium classes. Each connector now registers a DebeziumAdapter implementation, so the SPI added in apache#10799 has real providers and the version a connector ships is introspectable at runtime. connector-cdc-opengauss deliberately registers none: it runs Debezium's PostgreSQL connector and shades connector-cdc-postgres, and the shade plugin merges META-INF/services, so a second provider for the same connector class would break the exactly-one-match rule. A test pins that. Per-connector tests assert the adapter resolves uniquely and that its declared version equals the debezium-core version actually resolved onto the module classpath, so a pom bump that forgets the adapter fails the build. Runtime behavior, checkpoint format and CDC job compatibility are unchanged: all connectors still resolve Debezium 1.9.8.Final, so the classes on a CDC job's class loader are the same as before, only relocated from the shared jar into the connector jar.
…tic across connectors Moving Debezium into the connector jars introduced two version-coherence problems that the previous commit did not cover. Duplicate patched classes. connector-cdc-base recompiles five Debezium classes under src/main/java/io/debezium: ChangeEventQueue, TableId, HeartbeatFactory, HistorizedRelationalDatabaseConnectorConfig and DefaultHeartbeatConnectionProvider. Four of them also exist in debezium-core. While Debezium was packaged inside connector-cdc-base, the shade plugin resolved that overlap at build time and the patched copies were the only ones present. Now that each connector packages its own Debezium, the stock copies arrive in the connector jar while the patched ones stay in connector-cdc-base, and both jars sit on the same class loader. The winner would depend on the order File#listFiles returns the jars in AbstractPluginDiscovery, so the patches could silently stop applying on some machines. The CDC shade configuration now drops those classes from debezium-core, keeping connector-cdc-base's patched copies the single definition exactly as before. connector-cdc-opengauss declares its own filter list at execution level, which takes precedence over the inherited one, so it restates the filter. PatchedDebeziumClassesTest pins the set of patched classes against the exclusion list so a newly patched class cannot be added without updating both. Version drift between modules. The per-connector tests now assert that every Debezium artifact on a connector's class path resolves to the single version its adapter declares, instead of spot-checking debezium-core. This matters most for connector-cdc-opengauss, which declares its own debezium.version for debezium-embedded but reaches debezium-connector-postgres through connector-cdc-postgres: bumping one module and not the other now fails the build rather than shipping two Debezium versions on one class path. Documentation now states how far connectors may actually diverge. Packaging ownership has moved, but connector-cdc-base is still compiled against a single baseline and is loaded into every CDC connector, using around 85 Debezium types across relational, pipeline, schema, config and jdbc. Divergence is therefore safe only within a Debezium line that keeps those APIs binary compatible.
c53bc73 to
ef25cb1
Compare
|
Thanks @SEZ9 for the careful review and for keeping the migration boundary precise. This update is now rebased onto the current
I also investigated the old red workflow: its failures were an unrelated Windows |
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
Thanks @SEZ9 for staying engaged on this across so many rounds, and for keeping the pressure on the migration-boundary documentation - the current incompatible-changes.md wording (upgrade connector-cdc-base and every Debezium connector jar as one set, the corrected five-vs-four patched-class explanation, the explicit "do not exclude DefaultHeartbeatConnectionProvider" note) reads much more precisely than the first pass, and that came directly out of this back-and-forth. I also went back through your 2026-08-25 review specifically: the two High-severity blockers in it (Issue 1/Issue 2, "the shade-filter change in connector-cdc/pom.xml is not part of this diff" / "the parent POM changes are not in the changed file set") do not hold at the current head - seatunnel-connectors-v2/connector-cdc/pom.xml has carried the debezium-core shade filter and the relocated dependencyManagement since the very first commit on this branch (cd39273884e, 2026-08-03), and it is in the diff today (95 lines added, confirmed via git diff <merge-base>..HEAD -- seatunnel-connectors-v2/connector-cdc/pom.xml). I think that review was looking at an incomplete diff view (GitHub's PR diff sometimes needs pagination/"Load diff" for large changesets), not the actual current state of the 31-file change set - I'm calling this out explicitly so it doesn't linger as an open blocker in the thread. Issues 3/7/8 from that same review (nested-class coverage, the zstd-jni pin, machine-checking the shade filter) are all independently resolved by the PatchedDebeziumClassesTest additions in d82872e3084/ef25cb1e3e6, detailed below.
This is my full re-review against the current head, ef25cb1e3e6346ce8bb88f34ea783dcafc4385b8 (rebased onto current dev, ahead_by=4, behind_by=0), and it surfaces one new, concrete, code-breaking regression that no prior round of review (including my own) caught.
What Problem Does This PR Solve?
- User pain point:
connector-cdc-baseshipped the Debezium runtime (debezium-api,debezium-embedded, transitivelydebezium-core) at compile scope, andAbstractPluginDiscovery#filterPluginJar/selectPluginJar(seatunnel-plugin-discovery/.../AbstractPluginDiscovery.java:514-546) force-loadsconnector-cdc-base-*.jarinto the class loader of every plugin whose name contains "cdc". That made the one shared jar the single project-wide Debezium version; the per-connectordebezium.versionproperties added in #10799 could not actually change what a connector ran on. - Fix approach: flip
debezium-api/debezium-embedded/zstd-jnitoprovidedinconnector-cdc-base, relocate the version + exclusiondependencyManagementinto theconnector-cdcparent (interpolated per leaf module against that module's owndebezium.version), have each Debezium-based connector declare and package its own runtime Debezium, add amaven-shade-pluginfilter droppingdebezium-core's stock copies of the five classesconnector-cdc-basepatches, and register a (still inert)DebeziumAdapterSPI implementation per connector, guarded by machine-checked tests. - One-sentence summary: a packaging refactor that correctly moves Debezium ownership into seven in-tree connectors it was written against, but never accounted for
connector-cdc-db2- a Debezium-based CDC connector that merged intodevafter this PR was opened and landed in this branch through the 2026-08-27 rebase - and that connector no longer compiles.
1. Code Change Review
1.1 Core Logic Analysis
Mandatory registration-audit, redone from scratch against the current head rather than trusted from any prior round:
I pulled every touched pom.xml and cross-checked it against every module under seatunnel-connectors-v2/connector-cdc/, not just the seven this PR's diff touches:
| Module | In this PR's diff? | Uses Debezium? | Own debezium-api/-embedded/zstd-jni? |
Result |
|---|---|---|---|---|
connector-cdc-base |
Yes | Compiles against it, ships 5 patched classes | Flipped to provided |
Correct - patched classes stay the only definition, stock runtime removed from its shaded jar |
connector-cdc-mysql |
Yes | Yes | Added | Not broken |
connector-cdc-postgres |
Yes | Yes | Added | Not broken |
connector-cdc-oracle |
Yes | Yes | Added | Not broken |
connector-cdc-sqlserver |
Yes | Yes | Added | Not broken |
connector-cdc-mongodb |
Yes | Yes | Added | Not broken |
connector-cdc-opengauss |
Yes | Yes | Added, restates the shade filter (execution-level filters override rather than merge) | Not broken - stockDebeziumCoreOverridesAreExcludedInEveryCdcShadeConfiguration verifies both filters byte-for-byte |
connector-cdc-vitess |
Yes | Yes | Added | Not broken |
connector-cdc-tidb |
No | No (io.debezium grep = 0 hits) |
n/a | Correctly untouched |
connector-cdc-db2 |
No | Yes, extensively | No | Broken - does not compile |
Issue 1 (new, High, blocking): connector-cdc-db2 no longer compiles because of this PR's provided-scope change, and CI on the current head proves it.
connector-cdc-db2 (merged to dev via #10780 after this PR's original branch point, and pulled into this branch by the 2026-08-27 rebase onto dev) was never migrated. Its pom.xml declares only:
<dependency>
<groupId>org.apache.seatunnel</groupId>
<artifactId>connector-cdc-base</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>io.debezium</groupId>
<artifactId>debezium-connector-db2</artifactId>
<exclusions>
<exclusion><groupId>io.debezium</groupId><artifactId>debezium-core</artifactId></exclusion>
<exclusion><groupId>io.debezium</groupId><artifactId>debezium-api</artifactId></exclusion>
...(seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/pom.xml:36-79, untouched by this PR).
Before this PR, connector-cdc-base declared debezium-api/debezium-embedded at compile scope. Maven's scope-inheritance table resolves provided (db2's own scope for its connector-cdc-base dependency) combined with compile (the transitive scope) to provided for db2 - visible on db2's compile classpath, which is exactly why Db2Dialect, Db2SourceConfig, Db2SourceFetchTaskContext, Db2SnapshotFetchTask, Db2SnapshotSplitReadTask, TableDiscoveryUtils and others could import io.debezium.relational.*, io.debezium.pipeline.*, io.debezium.jdbc.*, io.debezium.heartbeat.*, io.debezium.connector.base.ChangeEventQueue, etc. directly (grep -c "^import io\.debezium" across connector-cdc-db2/src/main/java returns 58 matches). After this PR, connector-cdc-base's own dependency on debezium-api/debezium-embedded is itself provided. Maven never propagates a provided-scope (or test-scope) transitive dependency at all, regardless of the direct dependency's scope - so db2's effective view of debezium-api/debezium-embedded/debezium-core collapses from "provided" to "absent". Since db2 also explicitly excludes debezium-core/debezium-api from its own debezium-connector-db2 dependency (the same pattern the six migrated connectors still use, but which only works for them because they separately re-added a direct debezium-api+debezium-embedded dependency - db2 never got that addition), there is no remaining path for db2 to see any io.debezium.* type at all.
This is not a theoretical scope-algebra argument - it is exactly what the current head's own CI run shows. I pulled the fork's actual job logs for head ef25cb1e3e6, not the apache-side pointer:
[ERROR] .../Db2SourceFetchTaskContext.java:[80,12] error: cannot find symbol
[ERROR] symbol: class ErrorHandler
[ERROR] .../Db2SourceConfig.java:[25,29] error: cannot find symbol
[ERROR] symbol: class RelationalTableFilters
[ERROR] location: package io.debezium.relational
[ERROR] .../Db2SnapshotFetchTask.java:[29,25] error: package io.debezium.config does not exist
[ERROR] .../Db2SnapshotSplitReadTask.java:[26,38] error: package org.apache.kafka.connect.errors does not exist
... (>20 more "cannot find symbol" / "package does not exist" errors, all under connector-cdc-db2)
[ERROR] mvn <args> -rf :connector-cdc-db2
(from unit-test (8, ubuntu-latest), unit-test (11, ubuntu-latest), unit-test (8, windows-latest), unit-test (11, windows-latest), updated-modules-integration-test-part-1 (11, ubuntu-latest) and updated-modules-integration-test-part-2 (8 and 11, ubuntu-latest) - all seven currently-failing jobs on run https://github.com/DanielLeens/seatunnel/actions/runs/33091568634, all failing at the exact same connector-cdc-db2 compile step, deterministically, across every JDK/OS combination). This is the real cause of today's red Build check - not the Docker/Windows/Paimon environmental flakes diagnosed in earlier rounds. Those earlier diagnoses were correct for the commits they were made against; the db2 connector simply was not part of the tree yet at that point, and no round of review re-ran the full connector matrix after the rebase pulled it in. This is precisely the kind of shared-registration gap that a move like this has to re-check on every rebase, not just once.
Runtime confirmation of the plugin-loading mechanism (seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:514-546):
Job submits a DB2-CDC source
-> AbstractPluginDiscovery.getPluginJarPaths()
-> filterPluginJar(): pluginName contains "cdc"
-> matches BOTH connector-cdc-db2-*.jar AND connector-cdc-base-*.jar
-> selectPluginJar(): throws unless exactly 2 jars matched
-> both jars loaded into ONE URLClassLoader
-> neither jar packages io.debezium.* any more (connector-cdc-base: provided scope;
connector-cdc-db2: no own declaration, exclusions strip it from debezium-connector-db2)
-> class load of Db2Dialect/Db2IncrementalSourceFactory fails with NoClassDefFoundError,
if the module even finished compiling - which it currently does not.
Even setting the CI compile break aside for a moment, this shows the break is on db2's normal startup path, not a boundary/exception case: Db2SourceFetchTaskContext, Db2Dialect and Db2SourceConfig are constructed on every DB2 CDC job, snapshot or streaming.
Everything I re-verified from prior rounds still holds for the seven connectors this PR does touch: the shade-filter mechanism in connector-cdc/pom.xml:60-179 reaches all six inheriting connectors via <filters combine.children="append">; connector-cdc-opengauss/pom.xml:112-133 restates the identical filter because an execution-level <filters> block overrides rather than merges with the inherited plugin-level one (I diffed both lists - they match exactly); DebeziumAdapterFactory.getAdapter(...) still has zero production call sites, so the SPI itself changes no runtime behavior today; and zero .java files under any existing production package were modified (only new adapter/test files plus build configuration), so checkpoint/savepoint/offset serialization is untouched for every connector that does compile.
1.2 Compatibility Impact
Partially incompatible, and the current doc overstates how safe it is. For the seven Debezium-based connectors this PR actually touches: fully compatible, no config/API/checkpoint change, all still resolve debezium.version=1.9.8.Final. For third-party/out-of-tree connectors: honestly documented now (upgrade base+connector jars together, five-vs-four patched class list corrected, explicit "do not exclude DefaultHeartbeatConnectionProvider" note). But for connector-cdc-db2, an in-tree connector, the current incompatible-changes.md claim "No change for CDC job configuration, runtime behavior or checkpoint compatibility" (docs/en/introduction/concepts/incompatible-changes.md, new "Impact" bullet) is incorrect - db2 does not build at all at this head, which is a correctness regression, not a no-op.
1.3 Performance / Side-Effect Analysis
Unchanged from what's documented: the Debezium runtime is now duplicated into each of the seven migrated connector jars instead of shared via connector-cdc-base, growing the distribution (still only a qualitative note, no measured delta - Issue 3 below). No CPU/memory/GC/concurrency change on any job path for connectors that build successfully.
1.4 Error Handling and Logging
No new error-handling paths in production code - this PR only touches build/packaging configuration and new SPI/test scaffolding. Nothing to flag here beyond Issue 1 above (which is a build-time failure, not a runtime error-handling gap).
2. Code Quality Assessment
2.1 Coding Standards
License headers present on all new files. Comments throughout - especially the pom comments explaining the classloader-merge rationale and the shade-filter override/merge distinction - are unusually good at explaining why, not just what. PatchedDebeziumClassesTest's class- and method-level Javadoc clearly states the invariant each test protects. No issues here.
2.2 Test Coverage and Test Stability
Rating: Stable. PatchedDebeziumClassesTest (connector-cdc-base/src/test/.../PatchedDebeziumClassesTest.java) and the eight per-connector *DebeziumAdapterTest classes are all deterministic: they inspect compiled .class output, parse the two source POMs with a hardened DocumentBuilderFactory (external entities disabled), and open the resolved debezium-core artifact's code source to check for a class's presence. No Thread.sleep, no network I/O, no fixed ports, no shared mutable static state, no @DisplayName. This is a genuine improvement over the prior round: stockDebeziumCoreOverridesAreExcludedInEveryCdcShadeConfiguration (lines 140-159) now mechanically parses both io.debezium:debezium-core shade filters and asserts they equal the expected exclude set (built from STOCK_DEBEZIUM_CORE_OVERRIDES with both the bare and $*-wildcard nested-class patterns), which closes the "one-sided safety net" and "ignores nested classes" gaps raised in the 2026-08-25 review - the wildcard patterns are now asserted, not just written and hoped-for. defaultHeartbeatConnectionProviderIsNotInStockDebeziumCore (lines 166-185) converts what was previously a one-time manual curl+unzip verification into a standing, dependency-upgrade-proof CI check.
Coverage gap that remains (Issue 4, Low, carryover): none of these tests open an actual built/shaded connector jar, so a future regression that reaches the shade-filter merge only at package time (e.g., a combine.children typo, or a plugin-configuration inheritance change elsewhere in the build) is still not CI-visible through these tests alone. This is a reasonable non-blocking follow-up.
The coverage gap that is not carryover: none of these tests, nor any existing test in the repository, would have caught Issue 1 - there is no guardrail (module-set assertion, reactor "does connector-cdc-db2 declare its own debezium-api" check, or otherwise) that would fail when a new Debezium-based CDC connector merges into dev without picking up this PR's per-connector pattern. The CI failure caught it this time only because the compile genuinely breaks; a subtler future case (a new connector that compiles fine against connector-cdc-base's test-scope visibility but fails only at runtime) would not be caught by anything this PR adds.
2.3 Documentation Updates
docs/en and docs/zh incompatible-changes.md are updated and symmetric (7 lines added to each, verified via git diff --stat). Content is materially more complete than the first pass - see 1.2 for the one remaining accuracy problem (the "no change for runtime behavior" claim, given Issue 1).
3. Architectural Soundness
3.1 Elegance of the Solution
This is a long-term fix for the actual root cause (a force-merged shared jar pinning one project-wide Debezium version), not a workaround, and the dependencyManagement-in-parent / dependency-in-leaf pattern is the correct Maven idiom for "shared exclusion rules, per-module version." The shade filter and its opengauss restatement are now mechanically verified rather than resting on comment discipline alone.
3.2 Maintainability
Net improvement for the seven connectors this PR covers, but the pattern this PR establishes is per-connector, hand-applied, and un-enforced at the "does every Debezium-based connector module opt in" level. connector-cdc-db2 is the concrete proof that this migration step is easy to miss when a new connector lands via a routine, unrelated feature PR. I'd treat closing Issue 1 as also an opportunity to add a cheap structural guard (e.g., a module-list-based test in connector-cdc-base or connector-cdc that fails when a new connector-cdc-* module under Debezium's line does not declare its own debezium-api), so this class of gap cannot recur silently a second time.
3.3 Extensibility
DebeziumAdapter remains a reasonable seam for future per-connector runtime isolation and stays correctly unwired into production code per its @Experimental Javadoc.
3.4 Historical-Version Compatibility
No impact on checkpoint/savepoint, offset tracking, or serialized job state for any connector that builds - zero .java files under existing production packages were modified. This does not extend to connector-cdc-db2, which currently cannot produce a runnable artifact at all at this head.
4. Issue Summary
| # | Issue | Location | Severity | Raised by another reviewer |
|---|---|---|---|---|
| 1 | connector-cdc-db2 was never migrated to declare its own debezium-api/debezium-embedded/zstd-jni; after connector-cdc-base's dependencies move to provided, db2 loses all io.debezium.* visibility and fails to compile - confirmed live in the current head's CI (7/7 currently-failing jobs, all at the same connector-cdc-db2 compile step) |
seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/pom.xml |
High | No (new finding this round) |
| 2 | The updated incompatible-changes.md "Impact" bullet states there is no runtime-behavior change, which is not true for connector-cdc-db2 given Issue 1 |
docs/en/introduction/concepts/incompatible-changes.md; docs/zh/introduction/concepts/incompatible-changes.md |
Medium (tracks Issue 1; resolves once Issue 1 is fixed) | No |
| 3 | Distribution size growth documented only qualitatively, no measured delta | docs/en, docs/zh incompatible-changes.md |
Low | Yes (SEZ9, 2026-08-25, "Issue 6"; Daniel, 2026-08-24, "Issue 3") |
| 4 | No test inspects an actual built/shaded jar, so the shade-filter merge mechanism at package time is still not directly CI-verified (the POM-level filter-content check added this round substantially narrows, but does not close, this gap) | PatchedDebeziumClassesTest.java |
Low | Yes (SEZ9, "Issue 8"; Daniel, "Issue 4") - partially addressed this round |
| 5 | No structural guard fails the build when a future new Debezium-based CDC connector omits this PR's per-connector declaration pattern (the exact class of bug behind Issue 1) | connector-cdc-base / connector-cdc test scope |
Medium | No |
5. Merge Recommendation
Conclusion: Not recommended for merge
-
Blockers - must be fixed
- Issue 1 (High): Add the same per-connector
debezium-api/debezium-embedded/zstd-jnideclarations (and, if aDebeziumAdapteris desired for parity with the other seven, an adapter +META-INF/servicesregistration + test) toconnector-cdc-db2/pom.xml, matching the pattern already applied to mysql/postgres/oracle/sqlserver/mongodb/vitess. This is a small, mechanical, low-risk change - db2's owndebezium.versionshould default from the parent the same way vitess/opengauss now do, since db2 wasn't given its own property before either. After the fix, rerun the fork'sunit-testandupdated-modules-integration-test-*jobs (all seven that are currently failing at theconnector-cdc-db2compile step) to confirm the reactor builds clean, since this is the actual, verifiable CI signal for this fix - not a rerun-and-hope situation. - Issue 2 (Medium, bundled with Issue 1): once db2 is fixed, no further doc change should be needed since the "no runtime behavior change" claim will then be true again for all in-tree connectors; if a doc change is still needed after the Issue 1 fix, call out db2 explicitly the way the note already calls out the general "upgrade as a unit" requirement.
- Issue 1 (High): Add the same per-connector
-
Recommended fixes - non-blocking
- Issue 3: replace the qualitative distribution-size note with a measured delta from a built distribution.
- Issue 4: add one packaging-level assertion (unzip a built connector jar in a verify-phase test, assert the four patched class families are absent) so a shade-filter regression at package time is CI-visible, not just POM-content-visible.
- Issue 5: add a lightweight structural check (e.g., enumerate
connector-cdc-*modules that depend on anydebezium-connector-*artifact and assert each also declares its owndebezium-api) so a future connector cannot silently repeat Issue 1.
On the other reviewer: @SEZ9's 2026-08-25 review raised two High-severity blockers (missing connector-cdc/pom.xml changes, unresolvable versionless dependencies) that do not hold against the actual diff at any commit on this branch, including the one that review cites - I'd treat those two as resolved/moot rather than carried forward, for the reasons in my opening paragraph. The Medium/Low items in that same review (nested-class filter coverage, the zstd-jni pin, machine-checking the shade filters) are real and are now addressed by d82872e3084/ef25cb1e3e6.
Overall assessment. The core mechanism - moving Debezium ownership per connector, closing the resulting duplicate-class hole with a mechanically-verified shade filter, and documenting the third-party migration path - is sound, well-tested for the seven connectors it targets, and does not touch any existing production .java file, so there is no checkpoint/savepoint/serialization risk for connectors that build. But the PR set out explicitly to make "every CDC connector that used to depend on connector-cdc-base's shared Debezium dependency" keep working, and after the 2026-08-27 rebase pulled in a connector that landed on dev in the interim, that promise is no longer true - connector-cdc-db2 cannot compile, and the currently-red Build check on this exact head is direct, reproducible proof of it, not an unrelated flake. This is a small, mechanical fix to apply, and I'd expect a fast follow-up round once it lands.
SEZ9
left a comment
SEZ9
left a comment
There was a problem hiding this comment.
Thanks for working on this. I reviewed the latest head ef25cb1e3e63.
What this PR fixes
- User pain:
connector-cdc-baseships Debezium at compile scope and is force-loaded into every CDC plugin's class loader byAbstractPluginDiscovery#filterPluginJar, so the per-connectordebezium.versionproperties added in #10799 are inert — no connector can actually run a different Debezium. - Fix approach: demote
debezium-api/debezium-embedded/zstd-jnitoprovidedinconnector-cdc-baseso each connector packages its own Debezium runtime, keep the five patchedio.debeziumclasses in the base jar, guard the four stock-class shade exclusions with the newPatchedDebeziumClassesTest, and document the break in bothincompatible-changes.mdfiles. - One-line summary: the ownership move is directionally right and the pinning test is a good idea, but the versionless
provideddeclarations make correctness depend entirely on theconnector-cdcparent pom's dependencyManagement shipping atomically with this change, there is no runtime fail-fast for missing or duplicate Debezium classes on a CDC class loader, and the patched-vs-stock class race is guarded only by hand-maintained excludes in two hardcoded POMs.
Runtime chain I rechecked
AbstractPluginDiscovery.createPluginClassLoader()
-> AbstractPluginDiscovery.getPluginJarPaths() -> filterPluginJar() AbstractPluginDiscovery.java (cdc branch: accepts <pluginJarPrefix>*.jar AND connector-cdc-base*.jar)
-> URLClassLoader over [connector-cdc-<x>.jar, connector-cdc-base.jar]
-> IncrementalSource startup -> DebeziumSourceFunction/SourceReader creates io.debezium.embedded.EmbeddedEngine
-> loads io.debezium.connector.base.ChangeEventQueue, io.debezium.relational.TableId, io.debezium.heartbeat.HeartbeatFactory, io.debezium.relational.HistorizedRelationalDatabaseConnectorConfig
-> pre-PR: single copy from connector-cdc-base.jar (294 io.debezium core/embedded classes)
-> post-PR: patched copy in connector-cdc-base.jar vs stock copy in connector jar's shaded debezium-core; winner decided by File#listFiles jar order unless the shade excludes in connector-cdc/pom.xml and connector-cdc-opengauss/pom.xml strip the stock outer + $* nested classes
-> HeartbeatFactory.createHeartbeat() -> io.debezium.heartbeat.DefaultHeartbeatConnectionProvider (SeaTunnel-only, supplied solely by connector-cdc-base.jar — must NOT be excluded)
Guard path (build-time only, no runtime equivalent):
PatchedDebeziumClassesTest.compiledPatchedDebeziumClassesMatchExpectedSet() PatchedDebeziumClassesTest.java:~100
-> SourceOptions.class.getProtectionDomain().getCodeSource().getLocation() (assumes exploded target/classes, no null check)
-> Files.walk(io/debezium) compared against EXPECTED_PATCHED_CLASSES PatchedDebeziumClassesTest.java:60-75
shade-exclude verification parses connector-cdc/pom.xml and connector-cdc-opengauss/pom.xml via DocumentBuilderFactory (default config, only these two hardcoded POMs)
Findings
Issue 1: connector-cdc-base drops its Debezium dependencyManagement and declares versionless provided dependencies, making build correctness depend entirely on the connector-cdc parent pom's management and shade exclusion lists landing atomically with this change
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:66 - Why it matters: - Root cause:
connector-cdc-base/pom.xmlremoves its<dependencyManagement>entries fordebezium-api,debezium-embeddedandzstd-jni, and re-declares them asprovidedwith no<version>element. Version resolution and the shade exclusion lists thatPatchedDebeziumClassesTestparses now live only in theconnector-cdcparent pom (connector-cdc/pom.xml). - Impact: this module is no longer self-contained. Any partial cherry-pick, backport, or downstream fork that takes the
connector-cdc-basechange without the matching parent pom change gets versionless Debezium dependencies that cannot resolve, and a pin test that parses exclusion lists that do not exist. The coupling is invisible from this module's pom alone. - Suggested action: confirm the
connector-cdcparent pom change ships in the same commit set as this module change, and consider adding a brief comment next to the versionless declarations pointing at the parent's dependencyManagement so the coupling is explicit for backporters. - Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:58 * <p>The exclusion lists are maintained by hand in connector-cdc/pom.xml and;seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:59 * connector-cdc-opengauss/pom.xml. This test reads both lists so changing either the patched source;seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:@@ -44,53 (removed) <exclusion><groupId>org.apache.kafka</groupId><artifactId>kafka-log4j-appender</artifactId></exclusion> - Suggested fix: This hunk removes the module's
<dependencyManagement>entries fordebezium-api/debezium-embedded/zstd-jniand re-declares themprovidedwith no<version>. Version resolution now depends entirely on theconnector-cdcparent pom's dependencyManagement, andPatchedDebeziumClassesTest(line 58) parses that sameconnector-cdc/pom.xmlfor the shade exclusion lists. Please confirm the parent pom change lands atomically with this one, and consider a short comment here pointing at the parent's management — a partial cherry-pick or backport of this module alone would otherwise fail in confusing ways (unresolvable versions, pin test asserting against missing exclusions). - Severity: High
Issue 2: connector-cdc-base's shaded jar becomes Debezium-free with no runtime fail-fast; mixed old/new plugin jars and third-party CDC connectors fail with an opaque NoClassDefFoundError
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:71 - Why it matters: - Root cause: with
connector-cdc-basenow declaringdebezium-api/debezium-embedded/zstd-jniasprovided, runtime correctness depends entirely on the deployer having a matched set of jars. The PR's own docs and test javadoc acknowledge the two failure modes: a missing Debezium runtime (old connector jar + new base jar, or a third-party connector that never redeclared Debezium) and a duplicate patched class where the winner depends on the orderFile#listFilesreturns the jars inAbstractPluginDiscovery. - Impact: an operator who upgrades only
connector-cdc-basegets a bareNoClassDefFoundError: io/debezium/...at source startup with no hint that the fix is a matched jar set. The docs declare mixed jars unsupported, but nothing in the code detects the condition. - Suggested action: add a fail-fast check in the base module (e.g., during
DebeziumAdapterresolution,Class.forName("io.debezium.embedded.EmbeddedEngine")and throw a descriptive error naming the missingdebezium-embeddeddependency and linking incompatible-changes.md). - Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:71 <scope>provided</scope> (debezium-embedded);seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:76 <scope>provided</scope> (zstd-jni);docs/en/introduction/concepts/incompatible-changes.md:224 mixing pre-change and post-change plugin jars in one plugin directory is unsupported and can leave the runtime with a missing or duplicate Debezium class - Suggested fix: Making
debezium-embeddedprovidedhere means the shadedconnector-cdc-basejar no longer suppliesio.debeziumruntime classes to the CDC plugin class loader thatAbstractPluginDiscovery#filterPluginJarbuilds. Anyone who upgradesconnector-cdc-basebut leaves an older (or third-party) connector jar in the plugins dir will get a bareNoClassDefFoundError: io/debezium/...with no explanation. Since the docs already declare mixed jars unsupported, please also add a fail-fast check in the base module (e.g., atDebeziumAdapterresolution,Class.forName("io.debezium.embedded.EmbeddedEngine")and throw a descriptive error naming the missingdebezium-embeddeddependency and linking incompatible-changes.md). That turns an opaque class-loading failure into an actionable message. - Severity: High
Issue 3: The patched-vs-stock Debezium class race (ChangeEventQueue, TableId, HeartbeatFactory, HistorizedRelationalDatabaseConnectorConfig) is guarded only by hand-maintained shade excludes and a pom-parsing unit test — there is no runtime detection, so a mispackaged jar loads a nondeterministic class definition silently
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:58 - Why it matters: - Root cause: with Debezium now shipped inside each connector jar while
connector-cdc-baseretains five patchedio.debeziumclasses, both a patched and a stock copy of four classes can be visible on the same plugin class loader. The test's own javadoc states the winner depends on the orderFile#listFilesreturns the jars inAbstractPluginDiscovery— filesystem-dependent, nondeterministic class resolution. - The only safeguards are hand-maintained
<exclude>patterns in two hardcoded in-tree poms plusPatchedDebeziumClassesTest, which runs at build time in this repository only. Third-party connectors, downstream repackagings, and mixed pre-/post-change deployments get no protection at all. - Impact: if a stock copy of
ChangeEventQueue(a backpressure-critical class) orTableIdsurvives into a connector jar, the loaded definition varies silently by filesystem — the worst kind of production bug. - Suggested action: add a runtime assertion in
connector-cdc-basethat the patched classes resolve from the base jar's code source, and consider relocating the patched classes into anorg.apache.seatunnel.*package behind theDebeziumAdapterSPI so duplicate FQCNs cannot exist at all. - Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:58 * <p>The exclusion lists are maintained by hand in connector-cdc/pom.xml and;seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:66 <scope>provided</scope>;seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:58-59 * <p>The exclusion lists are maintained by hand in connector-cdc/pom.xml and connector-cdc-opengauss/pom.xml. This test reads both lists - Suggested fix: This test is a good build-time guard, but it only protects in-tree poms. Third-party connectors, downstream repackagings, and operators mixing pre-/post-change jars (all called out in the docs) get no protection: if the stock
ChangeEventQueue/TableIdcopies survive into a connector jar, the class that actually loads depends onFile#listFilesorder — silent, filesystem-dependent behavior on a backpressure-critical class. Please add a runtime fail-fast inconnector-cdc-base(e.g. assertChangeEventQueue.class.getProtectionDomain().getCodeSource()resolves to the connector-cdc-base jar during source initialization and throw with a clear message otherwise), and consider relocating the patched classes into anorg.apache.seatunnel.*package behind theDebeziumAdapterSPI so duplicate FQCNs cannot exist at all. - Severity: Medium
Issue 4: PatchedDebeziumClassesTest resolves the compiled classes via ProtectionDomain code source and sibling pom.xml relative paths, which breaks when the code source is a jar or the working directory differs
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:108 - Why it matters: - Root cause: the test locates the patched Debezium classes via
SourceOptions.class.getProtectionDomain().getCodeSource().getLocation().toURI()and assertsFiles.isDirectory(debeziumRoot). This only works when the classpath entry is an explodedtarget/classesdirectory; if the module classes are on the classpath as a jar (aggregated test runs, surefireuseSystemClassLoadervariations, or IDE configurations referencing the packaged artifact), the directory assertion fails for an environmental reason rather than a real pin violation. - The shade-exclude checks read
connector-cdc/pom.xmlandconnector-cdc-opengauss/pom.xmlvia relative paths, which additionally couples the test to the working directory. - Impact: false failures in non-default build environments erode trust in a guard test that exists precisely to catch dangerous packaging mistakes.
- Suggested action: add a jar-entry fallback (or an explicit assumption with a clear message) for the class enumeration, and anchor the pom reads explicitly (e.g., resolve from a system property or basedir) instead of relying on the working directory.
- Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:110-112 Assertions.assertTrue(Files.isDirectory(debeziumRoot), "Expected compiled Debezium overrides under " + debeziumRoot);;seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/test/java/org/apache/seatunnel/connectors/cdc/base/debezium/PatchedDebeziumClassesTest.java:103-109 Path classesRoot = Paths.get(SourceOptions.class.getProtectionDomain().getCodeSource().getLocation().toURI()); - Suggested fix:
getCodeSource().getLocation()only points at an explodedtarget/classesdirectory in the plainmvn testlayout; when the module classes end up on the classpath as a jar theFiles.isDirectory(debeziumRoot)assertion on line 110 fails for an environmental reason rather than a real pin violation. SinceJarFileis already imported, please add a jar-entry fallback (or at least an assumption with a clear message) so this guard test doesn't produce false failures in aggregated/IDE runs. The same applies to the relative-path reads ofconnector-cdc/pom.xmlandconnector-cdc-opengauss/pom.xmlmentioned in the class javadoc — anchor them explicitly instead of relying on the working directory. - Severity: Medium
Issue 5: Downstream Maven consumers of connector-cdc-base lose not only transitive Debezium but also the curated exclusion rules (kafka-log4j-appender, glassfish jersey, zstd-jni M1 override), and the migration doc does not tell them to restate those exclusions
- Location:
docs/en/introduction/concepts/incompatible-changes.md:224 - Why it matters: - Root cause: this change deletes the
dependencyManagementblock fromconnector-cdc-base/pom.xml(thedebezium-embeddedexclusions forkafka-log4j-appender,org.glassfish.jersey.core:*, and thezstd-jni1.5.5-5 override) and moves it to theconnector-cdcparent pom. In-tree connectors inherit that parent, but third-party connectors depending onconnector-cdc-baseas a library do not — inherited dependency management does not flow through a plain dependency edge. - Impact: a third-party author who follows the migration note and naively redeclares
debezium-embeddedre-introduces the log4j-1.x appender and the Apple-M1-brokenzstd-jnirelease — regressions the deleted exclusions specifically guarded against. - Suggested action: add the exact
<exclusions>block to the migration note (en + zh), or publish the shared management as an importable BOM so external consumers canimportit. - Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:53-56 AbstractPluginDiscovery#filterPluginJar), so any Debezium artifact packaged here would become a single project-wide Debezium version that no connector could override. Keeping these dependencies provided keeps stock Debezium artifacts out of the shaded connector-cdc-base jar;docs/en/introduction/concepts/incompatible-changes.md:224 Third-party CDC connectors built againstconnector-cdc-baseand relying on it to supply Debezium transitively must now declaredebezium-api,debezium-embedded,zstd-jni, and their connector-specific Debezium artifact themselves. - Suggested fix: This bullet tells third parties to declare
debezium-api/debezium-embedded/zstd-jnithemselves, but the exclusion rules that used to travel withconnector-cdc-base's compile-scopedebezium-embedded(kafka-log4j-appender,org.glassfish.jersey.core:*, and the Apple-M1zstd-jnioverride) now live only in theconnector-cdcparent's dependencyManagement, which external consumers do not inherit. A third party who redeclaresdebezium-embeddednaively re-introduces the log4j-1.x appender and the M1-broken zstd-jni. Please add the exact<exclusions>block to this migration note (en + zh), or publish the shared management as an importable BOM. - Severity: Medium
Issue 6: The incompatible-changes entries are single wall-of-text paragraphs and don't state the first affected SeaTunnel release; the third-party shade guidance would be far more usable as a code block
- Location:
docs/en/introduction/concepts/incompatible-changes.md:224 - Why it matters: - Root cause: each new bullet in
docs/en/introduction/concepts/incompatible-changes.md(and the zh mirror) packs the description, upgrade rule, third-party migration steps, and the exact patched-class list into one multi-hundred-character sentence chain, and none of the bullets names the SeaTunnel version in which the break lands. - Impact: this document is the first thing operators read before upgrading; a user maintaining a third-party CDC connector has to reverse-engineer the four
maven-shade-plugin<exclude>patterns from prose, and cannot tell from the doc which release the requirement starts in. - Suggested action: state the first affected release, split the Impact into sub-bullets (upgrade-as-a-set / new dependency declarations / shade exclusions), and provide a fenced XML snippet with the exact exclude patterns in both language versions.
- Evidence:
docs/zh/introduction/concepts/incompatible-changes.md:210 - **影响**：CDC 作业配置、运行时行为和 checkpoint 兼容性均不受影响——... 其maven-shade-plugin配置必须同时排除 ...;docs/en/introduction/concepts/incompatible-changes.md:226 - **How far connectors may actually diverge**: ... It uses around 85 Debezium types acrossio.debezium.relational,io.debezium.pipeline,io.debezium.schema,io.debezium.configandio.debezium.jdbc...;docs/zh/introduction/concepts/incompatible-changes.md:213 - **连接器实际可以偏离到什么程度**：... 它使用了约 85 个 Debezium 类型 ... - Suggested fix: The content here is accurate (thanks for the five-vs-four patched-class correction and the explicit
DefaultHeartbeatConnectionProvidernote from earlier rounds), but as a single paragraph it's hard for a third-party connector author to act on. Could you (1) state the first SeaTunnel release this applies to, (2) split the Impact into sub-bullets (upgrade-as-a-set / new dependency declarations / shade exclusions), and (3) add a fenced XML snippet with the exact<exclude>patterns (io/debezium/relational/TableId.class,io/debezium/relational/TableId$*.class, etc.) so users can copy-paste instead of reconstructing them from prose? Same treatment for the zh mirror at line 210. - Severity: Low
Issue 7: The single module-local pin of the native zstd-jni library (1.5.5-5) was removed from connector-cdc-base's dependencyManagement; per-connector declaration risks future version drift of a JNI artifact across the eight CDC connector jars
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:76 - Why it matters: - Root cause: this change deletes
<version>1.5.5-5</version>forcom.github.luben:zstd-jnifromconnector-cdc-baseand switches the base declaration toprovided; every connector pom now declareszstd-jniitself. - Risk (not a defect in the current state, where all connectors resolve the same version via the parent's management):
zstd-jnibundles native binaries, so any future drift is worse than for a pure-Java dependency — one connector resolving an older release would re-introduce the Apple-M1 incompatibility the deleted comment guarded against, and a native-code CVE would need eight coordinated fixes instead of one. - Suggested action: keep a single pin in the
connector-cdcparent's dependencyManagement (with the M1 rationale comment carried over) and add an enforcer convergence rule for the CDC modules so a per-connector override fails the build. - Evidence:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:76 <scope>provided</scope> - Suggested fix: With the
1.5.5-5pin removed from this module's dependencyManagement,zstd-jniis now declared independently by eight CDC connector poms. All connectors currently resolve the same version through the parent's management, but since this is a native JNI artifact, future drift is riskier than usual (the deleted comment explains the Apple-M1 incompatibility of older releases, and a native-code CVE would then need eight fixes instead of one). Please keep a single pin in theconnector-cdcparent's dependencyManagement — carrying over the M1 rationale comment — and add an enforcer convergence rule for the CDC modules so a per-connector override fails the build. - Severity: Low
Issue 8: PR description's user-facing-change section contradicts the added pom comment and docs about whether connector-cdc-base still ships io.debezium classes
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:59 - Why it matters: - Root cause: the PR description states "
connectors/connector-cdc-base-*.jarno longer containsio.debeziumclasses", but the code comment added inconnector-cdc-base/pom.xmland bothincompatible-changes.mdentries correctly say the jar intentionally still contains five patchedio.debeziumclasses. - Impact: the PR description typically becomes the squash-merge commit message and feeds release-note tooling, so the inaccurate wording would propagate into the changelog and mislead operators auditing jar contents.
- Suggested action: update the description to say the jar no longer ships stock Debezium artifacts while retaining the five patched classes, matching the pom comment and docs.
- Evidence:
docs/en/introduction/concepts/incompatible-changes.md:223 ... The stock Debezium runtime now lives in each connector's own jar instead, whileconnector-cdc-basestill keeps five patchedio.debeziumclasses. - Suggested fix: This comment (and the docs) correctly say the base jar still ships five patched
io.debeziumclasses, but the PR description's user-facing-change section still claims the jar "no longer containsio.debeziumclasses". Since the description becomes the squash commit message, please align it with this wording (no stock Debezium artifacts, five patched classes retained) before merge. - Severity: Low
Review conclusion
Conclusion: can merge after the blocking items are fixed
1. Blocking items
- Issue 1: connector-cdc-base drops its Debezium dependencyManagement and declares versionless
provideddependencies, making build correctness depend entirely on theconnector-cdcparent pom's management and shade exclusion lists landing atomically with this change - Issue 2: connector-cdc-base's shaded jar becomes Debezium-free with no runtime fail-fast; mixed old/new plugin jars and third-party CDC connectors fail with an opaque NoClassDefFoundError
2. Remaining follow-ups
- Issue 3: The patched-vs-stock Debezium class race (ChangeEventQueue, TableId, HeartbeatFactory, HistorizedRelationalDatabaseConnectorConfig) is guarded only by hand-maintained shade excludes and a pom-parsing unit test — there is no runtime detection, so a mispackaged jar loads a nondeterministic class definition silently
- Issue 4: PatchedDebeziumClassesTest resolves the compiled classes via ProtectionDomain code source and sibling pom.xml relative paths, which breaks when the code source is a jar or the working directory differs
- Issue 5: Downstream Maven consumers of connector-cdc-base lose not only transitive Debezium but also the curated exclusion rules (kafka-log4j-appender, glassfish jersey, zstd-jni M1 override), and the migration doc does not tell them to restate those exclusions
- Issue 6: The incompatible-changes entries are single wall-of-text paragraphs and don't state the first affected SeaTunnel release; the third-party shade guidance would be far more usable as a code block
- Issue 7: The single module-local pin of the native
zstd-jnilibrary (1.5.5-5) was removed from connector-cdc-base's dependencyManagement; per-connector declaration risks future version drift of a JNI artifact across the eight CDC connector jars - Issue 8: PR description's user-facing-change section contradicts the added pom comment and docs about whether connector-cdc-base still ships io.debezium classes
Thanks for the updates so far. Happy to discuss the remaining points.
|
Thanks @SEZ9, this is another careful pass and every one of the eight issues has a concrete, checkable claim behind it - appreciated. Responding to each, with where I agree, where I'd push back on severity, and what I'm actually doing about it. Context first, since it changes what "blocking" means right now: the Issue 1 (versionless Issue 2 (no runtime fail-fast for missing Debezium classes) - agree, will add. The failure mode you describe is real, though it's only reachable through a configuration the docs already call unsupported (mixing pre- and post-change plugin jars in one plugins dir). Still, turning an opaque Issue 3 (patched-vs-stock class race, no runtime detection) - agree it's not fully closed, but it's narrower than it reads. Issue 4 (test fragile to jar-classpath / working-directory) - agree, straightforward, will fix. Issue 5 (downstream Maven consumers lose the curated exclusions) - agree, doc fix. Correct that Issue 6 (wall-of-text docs, no version stated) - agree on structure. Will split the Impact bullet into sub-bullets (upgrade-as-a-set / new dependency declarations / shade exclusions) and add a fenced XML snippet with the literal exclude patterns, both docs. On the release version: this PR is still pre-merge and blocked on the db2 fix above, so I'd rather not hardcode a version number that may shift before it actually lands - will fill that in once we're closer to a release cut rather than commit to a guess now. Issue 7 (zstd-jni pin, native-lib drift risk) - one factual correction, then agree on the ask. The Issue 8 (PR description contradicts the docs/pom comment on whether the jar still ships Summary of what happens next: the db2 fix (mechanical, matching the pattern already applied to the other seven connectors) is the real blocker and lands first; Issues 1 (comment), 4, 5, 6 and 8 are folded into that same follow-up since they're all doc/test-robustness fixes with no runtime behavior change; Issues 2, 3 and 7 are accepted non-blocking follow-ups I'll track separately so they don't get lost in a growing diff. Thanks again for staying on this - the shade-filter and patched-class discipline in this thread has meaningfully improved over the last several rounds, and Issue 8 in particular was a real accuracy bug in my own PR description that's now fixed. |
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
What Problem Does This PR Solve?
- User pain point:
connector-cdc-baseshipped the Debezium runtime (debezium-api,debezium-embedded, transitivelydebezium-core) at compile scope, andAbstractPluginDiscovery#filterPluginJar/selectPluginJarforce-loadsconnector-cdc-base-*.jarinto the class loader of every plugin whose name contains "cdc". That made one shared jar the single project-wide Debezium version — no individual connector could actually diverge, even though thedebezium.versionproperty added in #10799 looked configurable per module. - Fix approach: flip
debezium-api/debezium-embedded/zstd-jnitoprovidedinconnector-cdc-base, relocate the version + exclusiondependencyManagementinto theconnector-cdcparent, have each of seven Debezium-based connectors declare and package its own runtime Debezium, add a machine-checkedmaven-shade-pluginfilter that dropsdebezium-core's stock copies of the five classesconnector-cdc-basepatches, and register a (still inert)DebeziumAdapterSPI per connector. - One-sentence summary: a sound packaging refactor for the seven connectors it was written against, still blocked by one concrete, previously-identified regression (
connector-cdc-db2no longer compiles) that has not moved in the last four days.
Re-verification note (this is not a fresh full review)
I gave this PR a full, from-scratch re-review on 2026-08-28 (#pullrequestreview- at that timestamp) and found one new High-severity, code-breaking issue: connector-cdc-db2 — a Debezium-based CDC connector merged into dev after this branch was opened, pulled in by the 2026-08-27 rebase — was never migrated to declare its own debezium-api/debezium-embedded, so it no longer compiles under this PR's provided-scope change. I replied the same day (2026-08-29) committing to push a follow-up commit folding in that db2 fix together with several accepted non-blocking items from @SEZ9's parallel review.
Checking today (2026-08-31, three days later), per the mandatory "fetch all reviews/comments/CI first" step of this review pass:
- Head is unchanged:
git log -1on a fresh fetch of this exact PR ref still resolves toef25cb1e3e6346ce8bb88f34ea783dcafc4385b8, last commit timestamp2026-08-27T16:07:09Z— no commit has landed since my 2026-08-29 reply. - No new review or comment from anyone since my own 2026-08-29 reply.
- CI is still failing at the exact same run (
https://github.com/DanielLeens/seatunnel/actions/runs/33091568634, created2026-08-27T16:07:24Z) — same seven jobs red (unit-teston JDK 8/11 × ubuntu/windows,updated-modules-integration-test-part-1/2), same root cause. - I re-pulled
connector-cdc-db2/pom.xmldirectly from the current head (not trusting my 08-28 notes) to confirm the blocker is still literally present:Still no own<dependency> <groupId>org.apache.seatunnel</groupId> <artifactId>connector-cdc-base</artifactId> <scope>provided</scope> </dependency> <dependency> <groupId>io.debezium</groupId> <artifactId>debezium-connector-db2</artifactId> <exclusions> <exclusion><groupId>io.debezium</groupId><artifactId>debezium-core</artifactId></exclusion> <exclusion><groupId>io.debezium</groupId><artifactId>debezium-api</artifactId></exclusion> ...
debezium-api/debezium-embedded/zstd-jnideclaration — the exact gap identified on 08-28.gh pr diff --name-onlyconfirms the PR's changed-file set is still exactly the same 31 files, andconnector-cdc-db2/**is not among them. - The
docs/enanddocs/zhincompatible-changes.md"Impact" bullet still reads "No change for CDC job configuration, runtime behavior or checkpoint compatibility — every in-tree connector still resolves Debezium 1.9.8.Final" — still inaccurate whileconnector-cdc-db2fails to build (this is Issue 2 below, unchanged).
Since nothing about the code has changed, I am not re-deriving the shade-filter plugin-inheritance chain, the DebeziumAdapterFactory call-site check, or the openGauss/Postgres coupling analysis a third time — those were derived in full in my 2026-08-16 review and re-confirmed in full in my 2026-08-28 review, both still visible on this thread, and nothing in the diff or in dev's recent history has touched them since. Sections 1–3 below summarize those stable conclusions; only the Issue table and CI diagnosis reflect today's independent re-check.
1. Code Change Review
1.1 Core Logic Analysis (stable since 2026-08-28, independently re-confirmed today)
For the seven connectors this PR actually touches (mysql, postgres, oracle, sqlserver, mongodb, opengauss, vitess): the provided-scope flip in connector-cdc-base, the relocated dependencyManagement in the connector-cdc parent, and each connector's own debezium-api/debezium-embedded/zstd-jni declaration correctly restore per-connector Debezium ownership without changing the resolved class set (all still pin 1.9.8.Final). The debezium-core shade filter (connector-cdc/pom.xml, <filters combine.children="append">) correctly reaches the six inheriting connectors, and connector-cdc-opengauss correctly restates the identical filter at execution level (an execution-level <filters> block overrides rather than merges with the inherited plugin-level one). DebeziumAdapterFactory.getAdapter(...) still has zero production call sites, so the SPI itself changes no runtime behavior. connector-cdc-tidb is correctly untouched (zero io.debezium references).
connector-cdc-db2 is the one connector this analysis does not hold for, confirmed again today: it depends on connector-cdc-base (now provided-scope for Debezium) and excludes debezium-core/debezium-api from its own debezium-connector-db2 dependency without ever re-adding them directly — the same pattern the seven migrated connectors also use, but which only works for them because they separately re-added debezium-api+debezium-embedded. Maven never propagates a provided-scope transitive dependency, so db2's effective view of every io.debezium.* type collapses to nothing. This breaks Db2Dialect, Db2SourceConfig, Db2SourceFetchTaskContext, Db2SnapshotFetchTask, Db2SnapshotSplitReadTask and others, all constructed on every DB2 CDC job's normal startup path (snapshot or streaming) — not a boundary or recovery-only path.
1.2 Compatibility Impact
Partially incompatible, unchanged classification. Fully compatible for the seven Debezium-based connectors this PR migrates (no config/API/checkpoint change, same resolved Debezium version before/after). Honestly documented for third-party/out-of-tree connectors. Incorrect for connector-cdc-db2, an in-tree connector: the docs' "no change to runtime behavior" claim is false for it today, since it cannot produce a runnable artifact at all at this head.
1.3 Performance / Side-Effect Analysis
Unchanged: no CPU/memory/GC/concurrency change on any job path for connectors that build. The Debezium runtime is now duplicated into each of the seven migrated connector jars instead of shared, growing the distribution by an undocumented (only qualitative) amount — carried-over Low issue.
1.4 Error Handling and Logging
No new error-handling paths in production code (build/packaging + inert SPI/test scaffolding only). Nothing to flag beyond the Issue 1 compile failure below, which is a build-time failure, not a runtime error-handling gap.
2. Code Quality Assessment
2.1 Coding Standards
Unchanged: license headers present on all new files, comments explain why (classloader-merge rationale, shade-filter override/merge distinction) rather than restating what, Javadoc present on every new class/field/method that needs it. No issues here.
2.2 Test Coverage and Test Stability
Rating: Stable. All new test classes (PatchedDebeziumClassesTest + one *DebeziumAdapterTest per connector) are deterministic — compiled-class inspection, hardened XML parsing of the two source POMs (external entities disabled), ServiceLoader discovery, resolved-artifact code-source checks. No Thread.sleep, network I/O, fixed ports, floating-point comparisons, or shared mutable static state. No flakiness risk in what these tests do exercise.
Real, still-open gap: none of these tests would have caught, and still would not catch, a new Debezium-based CDC connector landing without this PR's per-connector pattern (exactly what happened with db2) — there is no structural/reactor-level guard asserting "every connector depending on debezium-connector-* also declares its own debezium-api". This is Issue 3 below (carried over from 08-28's Issue 5).
2.3 Documentation Updates
docs/en/docs/zh incompatible-changes.md remain updated and symmetric. The one remaining accuracy problem is the "no change to runtime behavior" claim given the still-open db2 compile failure (Issue 2 below).
3. Architectural Soundness
3.1 Elegance of the Solution
Precise, long-term fix for the actual root cause (a force-merged shared jar pinning one project-wide Debezium version), not a workaround. The dependencyManagement-in-parent / dependency-in-leaf pattern is the correct Maven idiom here, and the shade filter is now mechanically verified (stockDebeziumCoreOverridesAreExcludedInEveryCdcShadeConfiguration) rather than resting on comment discipline alone.
3.2 Maintainability
Net improvement for the seven connectors this PR covers, but the migration pattern is per-connector, hand-applied, and currently unenforced at the "does every Debezium-based connector module opt in" level — connector-cdc-db2 is the live proof that this is easy to miss when a new connector lands via an unrelated, routine feature PR, and it is still missed four days after being identified.
3.3 Extensibility
DebeziumAdapter remains a reasonable seam for future per-connector runtime isolation, correctly unwired into production code per its @Experimental Javadoc.
3.4 Historical-Version Compatibility
No impact on checkpoint/savepoint, offset tracking, or serialized job state for any connector that builds — zero .java files under existing production packages are modified by this diff. Does not extend to connector-cdc-db2, which cannot produce a runnable artifact at this head.
4. Issue Summary
| # | Issue | Location | Severity | Raised by another reviewer |
|---|---|---|---|---|
| 1 | connector-cdc-db2 was never migrated to declare its own debezium-api/debezium-embedded/zstd-jni; it no longer compiles after connector-cdc-base's Debezium deps moved to provided. Re-confirmed today against the live head (ef25cb1e3e6): the pom is byte-for-byte unchanged from 08-28, connector-cdc-db2/** is still absent from the PR's 31-file diff, and the fork's unit-test/updated-modules-integration-test-* jobs are still red at this exact compile step. |
seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/pom.xml |
High | No (originally raised by me, 2026-08-28; still unresolved) |
| 2 | The incompatible-changes.md "Impact" bullet still claims no runtime-behavior change, which remains false for connector-cdc-db2 given Issue 1 |
docs/en/introduction/concepts/incompatible-changes.md; docs/zh/introduction/concepts/incompatible-changes.md |
Medium (tracks Issue 1; resolves once Issue 1 is fixed) | No |
| 3 | No structural guard fails the build when a future new Debezium-based CDC connector omits this PR's per-connector declaration pattern (the exact class of bug behind Issue 1) | connector-cdc-base / connector-cdc test scope |
Medium | No |
| 4 | Distribution size growth documented only qualitatively, no measured delta | docs/en, docs/zh incompatible-changes.md |
Low | Yes (SEZ9, 2026-08-25; me, 2026-08-24/08-28) |
| 5 | No test inspects an actual built/shaded jar, so the shade-filter merge mechanism at package time is still not directly CI-verified (the POM-level filter-content check substantially narrows, but does not close, this gap) | PatchedDebeziumClassesTest.java |
Low | Yes (SEZ9; me, prior rounds) |
5. Merge Recommendation
Conclusion: Not recommended for merge
This is my own PR, so GitHub blocks self-approval regardless of my conclusion — posting as a comment so another maintainer can act on the recommendation below. It agrees with, and does not soften, my own 2026-08-28 full review's conclusion.
-
Blockers — must be fixed
- Issue 1 (High): add the same per-connector
debezium-api/debezium-embedded/zstd-jnideclarations (and aDebeziumAdapter+META-INF/services+ test for parity, if desired) toconnector-cdc-db2/pom.xml, matching the pattern already applied to the other seven connectors. This is small and mechanical — I committed to pushing it on 2026-08-29 and it is still outstanding three days later, which is the actual reason this PR remains not-mergeable today, not a new design problem. After the fix, the fork'sunit-testandupdated-modules-integration-test-*jobs need to go green on the new head before this can move to "ready to merge after fixes". - Issue 2 (Medium, bundled with Issue 1): once db2 is fixed, no further doc change should be needed since the "no runtime behavior change" claim becomes true again for all in-tree connectors.
- Issue 1 (High): add the same per-connector
-
Recommended fixes — non-blocking
- Issue 3: add a lightweight structural check (enumerate
connector-cdc-*modules depending on anydebezium-connector-*artifact, assert each also declares its owndebezium-api) so this exact class of gap cannot recur silently. - Issue 4: replace the qualitative distribution-size note with a measured delta.
- Issue 5: add one packaging-level assertion against a built/shaded connector jar.
- Issue 3: add a lightweight structural check (enumerate
On the record so far: @SEZ9's 2026-08-28 review (8 items) was already responded to in full in my 2026-08-29 comment — the db2 fix (this Issue 1) was scoped to land in the same follow-up commit as the accepted items from that review (their Issues 1, 4, 5, 6, 8 in my numbering there). None of that has been pushed yet; today's check is a pure re-verification, and it confirms the situation is exactly as described in that comment, just three days further along with no code movement.
Overall assessment: the core mechanism is sound and well-tested for the seven connectors it was designed for, and introduces no checkpoint/savepoint/serialization risk for any connector that builds. The blocker is small, well-understood, and has had a committed fix plan sitting unshipped since 2026-08-29 — the fastest path to green is simply pushing the db2 pom change that mirrors the other seven connectors' migration.
This PR moves Debezium ownership from connector-cdc-base to each CDC connector, but connector-cdc-db2 was added to dev by apache#10780 after this branch was cut and never received the debezium-api, debezium-embedded, and zstd-jni dependencies the other connectors (mysql, sqlserver, postgres, oracle, opengauss, mongodb, vitess) already declare. Without them, connector-cdc-base's now-provided-scope copies are absent from this connector's classpath and it fails to compile. Adds the same three dependencies with no explicit version, so they resolve through connector-cdc's dependencyManagement at the inherited debezium.version, matching the existing per-connector pattern exactly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
What Problem Does This PR Solve?
connector-cdc-base shipped the Debezium runtime (debezium-api, debezium-embedded, transitively debezium-core) at compile scope, and AbstractPluginDiscovery#filterPluginJar/selectPluginJar force-loads connector-cdc-base-*.jar into the class loader of every plugin whose name contains "cdc". That made one shared jar the single project-wide Debezium version — no individual connector could actually diverge, even though the debezium.version property added in #10799 looked configurable per module. This PR flips debezium-api/debezium-embedded/zstd-jni to provided in connector-cdc-base, relocates the version + exclusion dependencyManagement into the connector-cdc parent, and has each Debezium-based CDC connector declare and package its own runtime Debezium.
Re-review at the new head bf91ef1f42df. One real commit since my previous full review (2026-09-01, head ef25cb1e3e63): [Fix][Connector-V2] Add owned Debezium runtime deps to connector-cdc-db2. This is the exact fix for the High-severity blocker I first raised on 2026-08-28 and re-confirmed unresolved in every round since: connector-cdc-db2 — merged into dev after this branch was cut — was never migrated and no longer compiled once connector-cdc-base's Debezium deps moved to provided. I re-pulled the full diff and the complete current state of every touched module fresh at this head, not carried forward from memory.
1. Code Change Review
1.1 Core Logic Analysis
The db2 fix is correct and mechanically identical to the working pattern already verified for the other seven connectors. The commit adds exactly three unversioned dependency declarations to connector-cdc-db2/pom.xml — debezium-api, debezium-embedded, com.github.luben:zstd-jni — with no <version> tag, so they resolve through connector-cdc/pom.xml's dependencyManagement at the inherited debezium.version (1.9.8.Final), exactly the idiom already used by connector-cdc-mongodb (and the other six). I diffed db2's pom against mongodb's pom side-by-side to confirm the declarations are structurally identical, not just similar in spirit.
I also independently re-verified the two things this fix depends on rather than trusting the commit message:
- Shade filter coverage:
connector-cdc/pom.xml's<build><plugins><plugin>maven-shade-plugin>block (notpluginManagement) declares thedebezium-coreexclusion filter withcombine.children="append".connector-cdc-db2/pom.xmlhas no shade-plugin override of its own, so it inherits this filter the same way the six non-opengauss connectors already do (opengauss needed its own restatement only because it overrides the filter at execution level, which db2 does not). db2 needs no additional shade configuration. PatchedDebeziumClassesTeststill covers db2 correctly: this test only reads two shade-filter lists —connector-cdc/pom.xml's own list andconnector-cdc-opengauss/pom.xml's execution-level restatement — to assert both cover the same four patched classes. Since db2 inherits the first list rather than declaring its own, this test's existing coverage already extends to db2 with no test change required.
Registration-matrix check across every CDC connector in the repo (not just the ones the PR description names), independently enumerated via git ls-tree rather than trusting the PR's own file list:
| Module | Uses Debezium? | Migrated to own debezium-api/embedded/zstd-jni? |
|---|---|---|
| connector-cdc-mysql | Yes | Yes (prior commits) |
| connector-cdc-postgres | Yes | Yes (prior commits) |
| connector-cdc-oracle | Yes | Yes (prior commits) |
| connector-cdc-sqlserver | Yes | Yes (prior commits) |
| connector-cdc-mongodb | Yes | Yes (prior commits) |
| connector-cdc-opengauss | Yes | Yes (prior commits) |
| connector-cdc-vitess | Yes | Yes (prior commits) |
| connector-cdc-db2 | Yes | Yes — fixed this round |
| connector-cdc-tidb | No | N/A — independently confirmed zero io.debezium references in both its pom.xml and its entire src/ tree at this head |
I also walked dev's history since this branch's merge-base (e6caf5a4, 171 commits behind) specifically for anything touching seatunnel-connectors-v2/connector-cdc/: seven commits landed, all fixes/features to connectors already on the migrated list above (mongodb, postgres, oracle) — no new CDC connector module was added to dev in that window. So the registration matrix is now complete: every Debezium-based CDC connector in the repository, including ones that landed after this branch was originally cut, is covered.
One real, non-blocking inconsistency found on this pass: the other seven connectors each also got a dedicated XxxDebeziumAdapter.java + META-INF/services/...DebeziumAdapter registration + a XxxDebeziumAdapterTest; db2 got only the pom dependency fix, with no Db2DebeziumAdapter counterpart. Since DebeziumAdapterFactory.getAdapter(...) still has zero production call sites (confirmed unchanged this round — the SPI remains @Experimental and inert), this has no runtime effect today. But it does mean db2 is the one Debezium-based connector without adapter-SPI parity, which is worth closing before the SPI is ever wired into a real code path (see Issue 6 below).
Also observed: unlike its seven siblings, db2's pom does not declare its own local <properties><debezium.version> override — it inherits the parent's default (1.9.8.Final), which today resolves to the identical version every other connector uses, so there is no compatibility gap. It does mean db2 cannot yet independently diverge to a different Debezium line the way the docs describe as the point of this migration, until that property is added locally (see Issue 7 below).
1.2 Compatibility Impact
Now fully compatible for all in-tree connectors, correcting my prior "partially incompatible" classification. With db2 fixed, the incompatible-changes.md claim — "No change for CDC job configuration, runtime behavior or checkpoint compatibility — every in-tree connector still resolves Debezium 1.9.8.Final" — is now actually true for every connector in the repository, not just the seven this PR originally targeted. No further doc change is required; this was correctly anticipated in my 2026-09-01 review.
1.3 Performance / Side-Effect Analysis
Unchanged from prior rounds: no CPU/memory/GC/concurrency change on any job path for any connector. The Debezium runtime is now duplicated into each of the eight migrated connector jars instead of shared once, growing the distribution by an amount still only qualitatively documented (carried-over Low issue, unaffected by this round's fix).
1.4 Error Handling and Logging
No new error-handling paths. This round's change is a pure build/packaging fix — no runtime code is touched.
2. Code Quality Assessment
2.1 Coding Standards
The new dependency block in connector-cdc-db2/pom.xml carries the same explanatory comment style already used in the other seven connectors, correctly adapted to name connector-cdc-db2 specifically rather than being copy-pasted with a stale module name. License headers unaffected (pom-only change).
2.2 Test Coverage and Test Stability
Rating: Stable. No new test classes in this commit; the existing PatchedDebeziumClassesTest (deterministic, no Thread.sleep/network I/O/shared mutable state, per my prior rounds' analysis) already extends its coverage to db2 as explained above with no changes needed.
Still-open gap, unchanged: there remains no structural/reactor-level guard that fails the build when a future new Debezium-based CDC connector lands without this PR's per-connector declaration pattern — this is precisely the class of bug that made db2 an unmigrated blocker for two weeks. Issue 5 below (carried over, previously Issue 3) tracks this; the SPI-parity gap in Issue 6 is a related but distinct instance of the same underlying "the pattern is applied by hand, per-connector, with no enforcement" risk.
2.3 Documentation Updates
docs/en/docs/zh incompatible-changes.md are unchanged by this commit and correctly so — as noted in 1.2, no further doc update is needed now that db2 is fixed.
3. Architectural Soundness
3.1 Elegance of the Solution
The db2 fix is exactly the minimal, mechanical change needed — no scope creep, no attempt to also retrofit the adapter-SPI parity gap into this same commit, which keeps the fix easy to review and verify in isolation.
3.2 Maintainability
The core dependencyManagement-in-parent / dependency-in-leaf pattern remains a net maintainability improvement for the connectors it fully covers. The adapter-SPI and debezium.version-property gaps on db2 (Issues 6, 7) are exactly the kind of partial-migration residue that makes this pattern harder to audit at a glance — a reviewer scanning "does every CDC connector look the same" will now find db2 structurally different from its seven siblings in two small but real ways.
3.3 Extensibility
DebeziumAdapter remains correctly unwired into production code. Db2's missing property override (Issue 7) is the one place where this round's fix, while functionally correct today, doesn't yet deliver the stated extensibility goal ("each CDC connector declares the Debezium version it ships") for that specific connector.
3.4 Historical-Version Compatibility
No impact on checkpoint/savepoint, offset tracking, or serialized job state — this commit touches only connector-cdc-db2/pom.xml, zero .java files.
CI Status
The apache-side Build check shows pending/queued. I checked the fork (DanielLeens/seatunnel) run for this exact head (bf91ef1f42df) directly: it moved from queued to in_progress during this review, but the jobs that would actually validate this fix — unit-test and updated-modules-integration-test-*, the same jobs that were red on the previous head specifically because of the db2 compile failure — had not started yet at review time; only License header, Code style, and Dead links had completed (all success, unsurprising for a pom-only change). I am explicitly not claiming a green build for this head; I'm reporting what I verified by direct source and dependency-graph inspection (the pom declarations, the shade-filter inheritance chain, and the test's coverage of that inheritance), which is strong evidence the fix is correct, separately from the CI signal which simply hasn't reached the relevant jobs yet. No rerun is needed — this run is actively progressing on its own.
4. Issue Summary
| # | Issue | Location | Severity | Status |
|---|---|---|---|---|
| 1 | connector-cdc-db2 was never migrated to declare its own debezium-api/debezium-embedded/zstd-jni; failed to compile after connector-cdc-base's Debezium deps moved to provided |
seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/pom.xml |
High | Fixed this round (bf91ef1f42df), verified against source and the shade-filter/test inheritance chain |
| 2 | The incompatible-changes.md "Impact" bullet claimed no runtime-behavior change while db2 could not compile |
docs/en|zh/introduction/concepts/incompatible-changes.md |
Medium | Resolved — claim is now true for all in-tree connectors, no doc change needed |
| 3 | Distribution size growth documented only qualitatively, no measured delta | docs/en, docs/zh incompatible-changes.md |
Low | Still open (carried over) |
| 4 | No test inspects an actual built/shaded jar; the shade-filter merge mechanism at package time is not directly CI-verified | PatchedDebeziumClassesTest.java |
Low | Still open (carried over) |
| 5 | No structural guard fails the build when a future new Debezium-based CDC connector omits this PR's per-connector declaration pattern — the exact class of bug behind Issue 1 | connector-cdc-base / connector-cdc test scope |
Medium | Still open (carried over; db2's two-week-unmigrated window is a live demonstration of this exact risk) |
| 6 | connector-cdc-db2 has no Db2DebeziumAdapter + META-INF/services registration + adapter test, unlike its seven Debezium-based siblings |
seatunnel-connectors-v2/connector-cdc/connector-cdc-db2 |
Low | New this round — no runtime impact today since DebeziumAdapter has zero call sites, but should be closed before the SPI is ever wired in |
| 7 | connector-cdc-db2 does not declare its own local <properties><debezium.version> override, unlike its seven siblings; it inherits the parent default, which happens to match today but means db2 cannot yet independently diverge as the migration's docs describe |
seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/pom.xml |
Low | New this round |
5. Merge Recommendation
Conclusion: Ready to merge after fixes
This is my own PR, so GitHub blocks self-approval regardless of my conclusion; posting as a comment so an independent maintainer can act on the recommendation below.
-
Blockers — must be fixed
- CI has not yet produced a result from the jobs that actually validate this fix (
unit-test,updated-modules-integration-test-*) on this exact head. My source-level verification of the db2 pom change and its dependency on the already-proven shade-filter inheritance gives me high confidence this is correct, but I want the actual compile/test signal before calling this mergeable — this is a "wait for the run to finish" gate, not a design concern.
- CI has not yet produced a result from the jobs that actually validate this fix (
-
Recommended fixes — non-blocking
- Issue 5: add a lightweight structural check (enumerate
connector-cdc-*modules depending on anydebezium-connector-*artifact, assert each also declares its owndebezium-api) so this exact class of gap — which just cost two weeks on db2 — cannot recur silently for the next new CDC connector. - Issue 6: add
Db2DebeziumAdapter+META-INF/services+ test for parity with the other seven connectors, before the SPI is ever given a production call site. - Issue 7: add db2's own
<properties><debezium.version>declaration to match its siblings and actually deliver the per-connector override capability the docs describe. - Issue 3: replace the qualitative distribution-size note with a measured delta.
- Issue 4: add one packaging-level assertion against a built/shaded connector jar.
- Issue 5: add a lightweight structural check (enumerate
Overall assessment: the actual blocker from every prior round — db2's broken compile — is fixed with the same minimal, correct pattern already proven for seven other connectors, and I independently re-verified both the dependency resolution and the shade-filter inheritance rather than trusting the commit message. I also independently confirmed no other CDC connector, old or newly landed in dev since this branch diverged, was missed. The two new Low-severity findings (adapter-SPI parity, missing local version property) are real but non-blocking gaps in an otherwise-complete migration; the only actual gate left before "ready to merge" is a completed, green CI run on this exact head.
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
What Problem Does This PR Solve?
- User pain point:
connector-cdc-baseshipped the Debezium runtime (debezium-api,debezium-embedded, transitivelydebezium-core) at compile scope, andAbstractPluginDiscovery#filterPluginJar/selectPluginJarforce-loadsconnector-cdc-base-*.jarinto the class loader of every plugin whose name contains "cdc". That made one shared jar the single project-wide Debezium version — no individual connector could actually diverge, even though thedebezium.versionproperty added in #10799 looked configurable per module. - Fix approach: flips
debezium-api/debezium-embedded/zstd-jnitoprovidedinconnector-cdc-base, relocates the version + exclusiondependencyManagementinto theconnector-cdcparent, and has each Debezium-based CDC connector declare and package its own runtime Debezium. - One-sentence summary: each CDC connector now genuinely owns and can independently version its Debezium runtime instead of silently sharing one project-wide jar.
Re-review at the new head 26f64641. One new commit since my previous full review (2026-09-13 00:45 UTC, head bf91ef1f42df): Merge branch 'dev' into daniel-debezium-per-connector-versions. I re-read the entire diff against dev from scratch, not just the incremental delta.
0. Disposition of Prior Blockers
This round's only new commit is a merge, but unlike a trivial no-op merge, it does bring in substantive dev changes — I checked carefully whether any of them touch this PR's own scope. git diff bf91ef1f42df 26f646419 --stat shows 49 files changed, but scoping it to what actually matters for this PR:
- No
connector-cdc-*pom.xmlfile is touched by the merge — confirmed viagit diff bf91ef1f42df 26f646419 --stat -- '**/pom.xml', which lists only 36 unrelated new-connector poms (connector-amazondocumentdb,connector-azurecosmosdb,connector-deeplake,connector-facebook-ads,connector-firebase,connector-google-ads, fourconnector-http-*variants,connector-mem0,connector-nebulagraph,connector-snmp,connector-tiktok-ads, plus theirdist/e2ewiring) — none of them Debezium-based CDC connectors.connector-cdc-db2/pom.xml, the file this PR's last commit fixed, is byte-identical (git diff ... -- connector-cdc-db2/pom.xmlis empty). - The
.javachanges the merge does bring intoconnector-cdc-baseand several already-migrated connectors (SchemaChangeResolver,SnapshotSplitAssigner,IncrementalSourceReader,PostgresRelationSchemaChangeResolver, a newDb2Dialect.toTableId(TablePath)override, etc. — ~2,600 lines) are real production changes from already-merged, unrelateddevPRs (schema-change-resolution work for mysql/postgres/oracle/mongodb/db2), not anything authored by this PR. I spot-checked the one db2-specific addition,Db2Dialect.toTableId(TablePath)(implementing a newDataSourceDialectinterface method introduced by that merge), and it's a self-contained, unrelated feature addition — nothing here interacts with the Debezium dependency-scope change this PR makes. - No new Debezium-based CDC connector module landed in
devduring this window — the pom diff above confirms this directly, so the registration matrix I completed last round (all 8 Debezium-based connectors migrated,connector-cdc-tidbconfirmed Debezium-free) remains complete and doesn't need re-walking from scratch.
Issue 1 (High, connector-cdc-db2 compile failure) — fix unchanged, still not CI-confirmed after two rounds — see CI section, this is worth flagging as a process gap, not just a status update.
Issue 2 (Medium, stale incompatible-changes.md claim) — still resolved, unchanged.
Issues 3, 4, 5, 6, 7 — all unchanged, not touched by this round's merge (distribution-size documentation, packaging-level shade test, structural migration guard, db2 adapter-SPI parity, db2's missing local debezium.version property).
1. Code Change Review
1.1 Core Logic Analysis
No change to this PR's own logic this round — the merge is a no-op for every file this PR owns (all connector-cdc/pom.xml, connector-cdc-base/pom.xml, and the eight migrated connectors' pom.xml/XxxDebeziumAdapter.java/META-INF/services registrations are byte-identical to the previous round). The core design (dependency ownership moved to provided in the base, dependencyManagement centralized in the connector-cdc parent, per-connector dependency declarations resolving through it) remains what I traced and verified in prior rounds.
1.2 Compatibility Impact
Fully compatible, unchanged. With db2 fixed (still confirmed this round via unchanged pom + inherited shade-filter), the incompatible-changes.md claim remains true for every connector in the repository. This round's merge doesn't touch compatibility surface for this PR's own scope.
1.3 Performance / Side-Effect Analysis
Unchanged from prior rounds: no CPU/memory/GC/concurrency change on any job path. Distribution-size growth remains only qualitatively documented (Issue 3, carried over).
1.4 Error Handling and Logging
No new error-handling paths from this round's diff for this PR's own files.
2. Code Quality Assessment
2.1 Coding Standards
Unchanged assessment; no new production code owned by this PR this round.
2.2 Test Coverage and Test Stability
Rating: Stable, unchanged. PatchedDebeziumClassesTest's coverage of db2 (via inheriting connector-cdc/pom.xml's shade-filter list) is unaffected by this round's merge. The still-open structural-guard gap (Issue 5) is unchanged — worth restating given the merge window I just checked: no new Debezium-based connector landed, so the gap wasn't exercised again this round, but it also wasn't closed.
2.3 Documentation Updates
Unchanged; incompatible-changes.md not touched by this round.
3. Architectural Soundness
3.1-3.4
Unchanged from my last round's assessment — see prior review for the full trace. This round's merge doesn't touch anything relevant to elegance, maintainability, extensibility, or historical-version compatibility for this PR's own scope.
CI Status — a process concern worth being direct about
I need to correct something from my own last round. I wrote then: "this run is actively progressing on its own... no rerun is needed." That run (34726679909, head bf91ef1f42df) was subsequently marked cancelled — superseded by this round's merge push — before the jobs that actually validate the db2 fix (unit-test, updated-modules-integration-test-*) ever started. So my confidence that CI was "on its way to confirming" the fix did not pan out; it simply never got the chance to run. I want to be upfront about that rather than quietly re-asserting confidence this round.
The current head's run (34763680066, head 26f646419) is queued and has not started at review time. This means the db2 compile fix — the one High-severity blocker from four rounds ago — has now gone through three consecutive commits (bf91ef1f42df, and now 26f646419) without a single completed CI run reaching the jobs that would actually prove it compiles and passes tests. My source-level verification (pom dependency resolution through the parent's dependencyManagement, shade-filter inheritance, PatchedDebeziumClassesTest's coverage) remains unchanged and I still have high confidence in it — I traced it fresh again this round against the current head, not carried forward from memory — but "high confidence from source inspection" is not the same thing as an actual green run, and it's now been long enough without one that I think it's worth naming explicitly rather than repeating the same "should be fine, CI just hasn't caught up" framing a third time.
4. Issue Summary
| # | Issue | Location | Severity | Status |
|---|---|---|---|---|
| 1 | connector-cdc-db2 was never migrated to declare its own debezium-api/debezium-embedded/zstd-jni; failed to compile after connector-cdc-base's Debezium deps moved to provided |
connector-cdc-db2/pom.xml |
High | Fixed at source level (unchanged this round); still not CI-confirmed after three consecutive heads |
| 2 | incompatible-changes.md "Impact" bullet claimed no runtime-behavior change while db2 could not compile |
docs/en|zh/introduction/concepts/incompatible-changes.md |
Medium | Resolved, unchanged |
| 3 | Distribution size growth documented only qualitatively | docs/en, docs/zh incompatible-changes.md |
Low | Still open |
| 4 | No test inspects an actual built/shaded jar | PatchedDebeziumClassesTest.java |
Low | Still open |
| 5 | No structural guard fails the build when a future new Debezium-based CDC connector omits the per-connector declaration pattern | connector-cdc-base / connector-cdc test scope |
Medium | Still open; not re-exercised this round since no new Debezium connector landed in the merge window |
| 6 | connector-cdc-db2 has no Db2DebeziumAdapter + META-INF/services registration + adapter test, unlike its seven siblings |
connector-cdc-db2 |
Low | Still open |
| 7 | connector-cdc-db2 doesn't declare its own local debezium.version property override |
connector-cdc-db2/pom.xml |
Low | Still open |
5. Merge Recommendation
Conclusion: Ready to merge after fixes
This is my own PR, so GitHub blocks self-approval regardless of my conclusion; posting as a comment so an independent maintainer can act on the recommendation below.
-
Blockers — must be fixed:
- CI still has not produced a completed result from the jobs that actually validate the db2 fix (
unit-test,updated-modules-integration-test-*), across three consecutive heads now. I'd like an actual green run before calling this unconditionally mergeable — my source-level confidence is high and unchanged, but it needs to be backed by a real result at this point rather than reasserted a third time.
- CI still has not produced a completed result from the jobs that actually validate the db2 fix (
-
Recommended fixes — non-blocking:
- Issue 5: add a lightweight structural check (enumerate
connector-cdc-*modules depending on anydebezium-connector-*artifact, assert each also declares its owndebezium-api) so this exact class of gap cannot recur silently for the next new CDC connector. - Issue 6: add
Db2DebeziumAdapter+META-INF/services+ test for parity with the other seven connectors. - Issue 7: add db2's own
debezium.versionproperty declaration to match its siblings. - Issue 3: replace the qualitative distribution-size note with a measured delta.
- Issue 4: add one packaging-level assertion against a built/shaded connector jar.
- Issue 5: add a lightweight structural check (enumerate
Overall assessment: the substance of this migration remains sound and unchanged — I re-verified the full registration matrix and confirmed this round's merge brings in no new Debezium-based connector and touches none of this PR's own files. The one thing I want to be honest about is process, not code: the db2 fix has now gone three heads without CI actually validating it, and my own prior-round assumption that "the run is progressing, no action needed" turned out to be wrong once that run got superseded. I'm not downgrading my source-level confidence in the fix itself, but I don't want to keep restating "should be fine" in place of an actual result — let's get one completed run on the current head before treating this as unconditionally ready.
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
Follow-up on the CI state only — no new commit and no other new activity since my last full review at this exact head (26f646419).
CI fact changed since my last review. My previous round's only remaining blocker was "CI still has not produced a completed result from the jobs that actually validate the db2 fix, across three consecutive heads." That run has since completed, and it did not come back clean — Build finished with FAILURE.
Diagnosis. The failing jobs are updated-modules-integration-test-part-3 (8, ubuntu-latest) and updated-modules-integration-test-part-3 (11, ubuntu-latest) (fork run 34763680066), both failing on the same test: OpengaussCDCIT.testAddFieldWithRestore, an Awaitility ConditionTimeoutException on the restored-row assertion. I checked the raw job logs for a classloading/shading-related root cause (NoClassDefFoundError, NoSuchMethodError, missing-service exceptions) given that this PR does rewrite connector-cdc-opengauss's Debezium dependency scope and shaded-jar filters (including excluding the patched HistorizedRelationalDatabaseConnectorConfig/TableId/ChangeEventQueue/HeartbeatFactory classes from that module's own shading step) — I found none; the container starts, the job runs its full ~520-540s duration, and only the final assertion times out.
Importantly, I also found the identical failure — same test class, same method, same day — in the CI run of a completely unrelated PR that does not touch CDC/Debezium code at all (it's a read-only REST diagnostics endpoint). Both PRs are also missing the same 5 commits behind current dev, none of which touch CDC/Debezium/Opengauss. That cross-PR correlation is fairly strong evidence this is a pre-existing, environment-sensitive flake in OpengaussCDCIT.testAddFieldWithRestore rather than something this PR's shading-filter change caused. That said, I'm not fully closing this out as "unrelated" on that evidence alone — this PR does directly modify the shaded-class filter list for exactly the Debezium schema-history/relational classes this schema-change-and-restore test exercises in this exact module, so I'd rather see one clean, confirmed rerun on this head (or a rerun with -Dtest=OpengaussCDCIT in isolation) than assume it away. If it passes on rerun, that closes the loop; if it fails again specifically on this PR while passing on dev/other PRs, that would flip my read of it.
Blocker status, updated: Issue 1 (High) stays open, but the substance changes from "still waiting on a completed run" to "the completed run failed, on a test that plausibly intersects this PR's own module changes, and I want a clean confirmed rerun before calling this unconditionally CI-clean." Everything else from my last round (the recommended non-blocking fixes — Issues 3-7, and the disposition of the dev merge commit) is unchanged.
Overall conclusion is unchanged: Ready to merge after fixes, gated on Issue 1. Self-approval remains unavailable on this account regardless of the CI outcome; an independent maintainer still needs to give the final sign-off.
🤖 Generated with Claude Code
|
Synced |
…t connector-cdc-base no longer exports it connector-cdc-pg-base was added on dev after this PR was written and relied on the Debezium classes that connector-cdc-base used to export at compile scope. Debezium is now provided in connector-cdc-base, which is not transitive, so declare debezium-api and debezium-embedded here with provided scope. They are not bundled: connector-cdc-postgres and connector-cdc-opengauss keep owning the Debezium runtime.
|
已处理上一条评论中标记的 根因（基于 pom 作用域的推断，不是实测构建失败）： dev 上新增的 修复： 在 验证范围： 本地仅做了 pom 的 XML 语法校验；按 Apache SeaTunnel 的验证规则未在本地编译或运行测试，以 fork 上新 head 的 Build 为准（run 见下）。在该 run 通过之前，不能认为该修复已被验证。 |
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
Thanks for the follow-up. I re-read the complete current 33-file diff and the plugin-loading/CDC callers. The DB2 dependency repair remains present and I do not find a new supported-deployment source blocker. The current merge facts have changed, however: GitHub now reports actual merge conflicts, and this head's fork Build was cancelled.
What this PR solves
- User pain: connector-cdc-base bundles the stock Debezium runtime and is loaded into every CDC plugin, preventing a connector from owning its runtime version.
- Fix: make those base dependencies provided, declare runtime dependencies in each connector, filter duplicate stock classes and add adapter/packaging guards.
- In one sentence: move Debezium packaging ownership to the plugin while preserving shared SeaTunnel patches. For example, MySQL can own its supported binary-compatible runtime instead of inheriting the base jar's stock payload.
1. Code Change Review
1.1 Core Logic Analysis
Local branch seatunnel-review-11639 matches 51f37b1ae0091c5162a4e80df5cf6c04dd5a6fdd. Full Git diff/source searches and static XML inspection covered all changed POMs, adapters, service registrations, tests and both language docs. Adapter/test variants were also compared across modules, including OpenGauss's deliberately inherited PostgreSQL provider. No local build, test, container or job ran; no built-jar contents are claimed verified.
Before, base declared compile-scope Debezium artifacts. After:
<artifactId>debezium-embedded</artifactId>
<scope>provided</scope>Each runtime-owning connector declares compile-scope api/embedded/zstd; parent dependencyManagement retains version resolution and exclusions. The shade filter explicitly excludes each patched stock outer class and its $* nested classes.
plugin discovery -> filterPluginJar (AbstractPluginDiscovery.java:514)
-> connector jar + shared connector-cdc-base jar in CDC plugin loader
-> source/readers/stream fetch -> existing Debezium runtime calls
-> stock core/embedded/connector classes now supplied by connector jar
-> four replaced core class families supplied only by patched base classes
-> SeaTunnel-only DefaultHeartbeatConnectionProvider remains in base
Maven parent management -> per-module dependencies -> inherited shade filters
OpenGauss own filter list -> explicitly restated core exclusions
UT guards -> compiled patch set + both source POM filters + artifact metadata
SPI registration -> DebeziumAdapterFactory unique-provider lookup in adapter tests
Key findings:
- The normal CDC plugin path reaches the packaging change; this is not only a property rename.
- Base still contains five patched classes; four replace stock classes, while DefaultHeartbeatConnectionProvider has no stock counterpart.
- DB2 now declares all three runtime dependencies. Pg-base declares compile/test-needed Debezium as provided; PostgreSQL/OpenGauss package the runtime.
- All current version declarations remain 1.9.8.Final, with zstd-jni centrally pinned at 1.5.5-5. OpenGauss tests intentionally use exactly one inherited PostgreSQL adapter, avoiding duplicate providers for one connector class.
- A whole-tree call-site search finds adapter-factory invocations in tests, not normal production execution. These providers are preparatory metadata/SPI infrastructure; they do not yet isolate the base's direct Debezium API usage or enable arbitrary major-version divergence.
1.2 Compatibility Impact
Partially incompatible packaging contract, with migration documented in both incompatible-changes files. No CDC option/default, checkpoint or wire format is changed. Operators must upgrade the base and deployed CDC jars as one release set; mixed old/new sets are unsupported. Third-party connectors must explicitly supply runtime dependencies and stock-patch exclusions. Binary-incompatible Debezium upgrades still require moving direct base usage behind the SPI; this PR does not complete that work.
1.3 Performance / Side Effects
No new record-loop allocations, worker threads, locks, network requests, retries or resource ownership. Runtime/compression/Kafka dependencies are duplicated per plugin, increasing disk/download footprint and potentially loaded-class/native-library pressure across multiple plugin loaders. The description's approximate size estimates are not independently measured here; the guide remains qualitative (existing nonblocking Issue 3). Same-runtime source semantics are retained, but packaged-artifact CI must validate class resolution.
1.4 Error Handling and Logging
No new supported matched-release source blocker found. SPI lookup clearly rejects zero/multiple matches, but is not invoked by the ordinary production path, so it is not a runtime packaging fail-fast guard. Mispackaged third-party/mixed jars can still fail opaquely or resolve duplicates; they should not be described as supported.
Community reconciliation:
- I agree with @SEZ9's duplicate-patch concern: both parent and OpenGauss filters are now actually present, and the test parses outer/nested exclusions. I respectfully disagree that the current change is incomplete because a partial cherry-pick omitting the parent could fail: the parent dependencyManagement/filter change is in this very PR's full diff. Such an incomplete backport is not a defect in the matched change set.
- I also would not make a new runtime fail-fast redesign a blocker for the explicitly unsupported mixed-jar deployment. It is useful defensive work, but putting it only at adapter resolution would not cover normal source startup, based on the actual call-site search above.
- +1 to @SEZ9's packaging-test limitation: the current checks inspect exploded classes, source POMs and classpath pom.properties, not actual assembled plugin jars. This is the same optional scope as Daniel's existing Issue 4, not another numbered duplicate.
- +1 to the third-party migration guidance improvement: authors outside the parent hierarchy should also carry the curated exclusions/native-version policy. A copyable XML example would help; this is a nonblocking docs improvement, not proof that in-tree dependencies currently regress.
2. Code Quality Assessment
2.1 Coding Standards
Adapter responsibilities, pinned-version constants and patched-class invariants have comments; service entries name their corresponding provider classes and have ASF headers. The XML parser disables external entities/DOCTYPE processing. DB2 parity remains optional Issues 6/7: no adapter/provider test and no module-local version property, though it resolves the parent baseline.
2.2 Test Coverage and Test Stability
Stability rating: Stable in the normal Maven module layout; no sleep, port, async timing or external-service assertion is added. Streams/JarFiles close, sets compare without iteration-order assumptions, and filesystem separators are normalized.
The patch-set test assumes SourceOptions resolves from an exploded classes directory and locates source POMs by walking ancestors of user.dir. +1 to @SEZ9's nondefault-layout concern: a packaged-code-source or unrelated working directory can cause a deterministic environment failure. This limits test portability rather than showing a scheduling flake. Existing Issue 4's packaging-phase guard would cover the real deliverable; these unit tests alone do not.
Tests map adapters to exact connector names and check all listed Debezium artifact metadata versions. OpenGauss pins compatibility with the inherited PostgreSQL adapter. Existing Issue 5 recommends a structural dependency guard for future modules so DB2's earlier omission cannot recur. No current-head connector E2E result is inferred from previous heads.
2.3 Documentation Updates
Both language guides now accurately distinguish five patches/four stock exclusions, matched-release upgrades, native/runtime duplication and major-version limits. They agree. Copyable external dependency/exclusion guidance and a first-release heading remain useful optional improvements; source POM changes do not flow transitively as dependencyManagement to arbitrary external consumers.
3. Architectural Soundness
3.1 Elegance
A coherent staged packaging change, not full Debezium abstraction. It preserves shared patches while making connector runtime ownership explicit.
3.2 Maintainability
Central management retains native-version and exclusion policy; repeated OpenGauss filters are mechanically checked. Add a packaged-artifact invariant before expanding version diversity.
3.3 Extensibility
Binary-compatible connector divergence becomes possible. Major-version divergence still requires actual SPI adoption by production base code. Versioned shared runtime jars are an alternative if distribution growth becomes unacceptable, with a larger plugin-discovery design cost.
3.4 Historical-Version Compatibility
Configuration/checkpoint formats remain; plugin jars must migrate as a set. No real rolling-upgrade or packaged-classloader test was executed locally.
4. Issue Summary
| No. | Existing nonblocking improvement | Location | Severity |
|---|---|---|---|
| 5 | Guard future CDC runtime dependency declarations | connector-cdc parent/test scope | Medium |
| 3 | Quantify distribution-size impact | incompatible-changes guides | Low |
| 4 | Check actual shaded plugin artifacts | PatchedDebeziumClassesTest.java:101 | Low; overlaps @SEZ9's packaging concern |
| 6 | DB2 adapter/SPI/test parity | connector-cdc-db2 | Low |
| 7 | DB2 local version declaration | connector-cdc-db2/pom.xml | Low |
Earlier source Issue 1 (DB2 compile dependency omission) and Issue 2 (incorrect impact statement) remain fixed. @SEZ9's earlier approval and later change requests are acknowledged explicitly above: the current full parent/filter/migration changes address their original supported-deployment concern, while runtime defensive checks and artifact-test limitations remain follow-ups rather than silently dismissed facts.
5. Merge Recommendation
Conclusion: Ready to merge after fixes
- Blockers: resolve the actual merge conflicts against current
dev, then obtain current-head CI and independent write-capable maintainer review. No remaining Daniel source blocker is asserted for the matched release set. - Suggested improvements, nonblocking: existing Issues 3-7 and @SEZ9's artifact/runtime/migration hardening discussed above.
The previous OpenGauss-failure statement is not the current CI fact. The exact-head fork Build is completed/cancelled, and Apache has no adopted Build check in this snapshot. A cancelled run does not establish that the earlier OpenGauss restore failure still occurs.
GitHub now reports mergeable_state=dirty. Please sync with the latest dev and resolve those conflicts, then let CI validate the resolved head; this is an actual conflict gate, independently recorded from ordinary behind/diverged metadata. It is not advice to add unrelated dependencies or weaken failing assertions.
Overall, keep the staged ownership design and its documented binary-compatibility boundary. Daniel has READ permission and cannot approve his own PR or dismiss another review; an independent maintainer must make the final effective review decision after conflicts and CI are clear.
Snapshot 2026-11-01T14:24:01.847093+00:00: head 51f37b1ae0091c5162a4e80df5cf6c04dd5a6fdd, base dev; compare diverged, ahead 10 / behind 9; merge dirty / DIRTY, review decision REVIEW_REQUIRED. Drift is recorded separately; it does not itself request synchronization.
Resolve the incompatible-changes.md conflict caused by apache#13065 splitting the guide into per-release sections: keep dev's 2.3.13 Transform Changes and move this PR's Debezium ownership entry under the dev section's Dependency Upgrades.
connector-cdc-base now declares Debezium as provided, so the debezium-core and debezium-api exclusions that apache#12477 added to the OceanBase CDC dependency on connector-cdc-mysql left the OceanBase jar without the stock Debezium runtime. Drop the exclusions; the connector-cdc parent shade filter still removes the stock classes that connector-cdc-base patches. Extend TestOceanBaseCdcPackagingIT to assert the runtime is bundled and document the OceanBase upgrade note.
|
Pushed Problem. Fix.
TiDB CDC was also checked: it only uses Only |
DanielLeens
left a comment
DanielLeens
left a comment
There was a problem hiding this comment.
Self re-check of the current head b793521df7be42989fd189c2ca8b268f8b7c7e14. Since my last self-review on 51f37b1ae0 there are two commits: e7ea0feab6 (a plain dev merge that cleared the merge conflicts reported last time) and b793521df7b (the OceanBase CDC packaging fix). The rest of the 35-file diff is unchanged from the version I already reviewed, so this note focuses on the delta and on whether the earlier conclusion still holds.
What Problem Does This PR Solve?
- User pain point:
connector-cdc-baseused to ship the stock Debezium runtime and is force-loaded into every CDC plugin class loader, so no connector could own its Debezium version. - Fix approach: base declares Debezium as
provided, each Debezium-based connector declares and bundles its runtime, theconnector-cdcparent shade filter drops the four stock classes that base patches, and adapter/packaging tests guard the invariants. - New in this head: once #12477 (OceanBase CDC, already in the merge base) is combined with this PR, the OceanBase jar would have shipped no stock Debezium at all. This head closes that gap.
1. Code Change Review
1.1 Core Logic Analysis
connector-cdc-oceanbase/pom.xml:39-50 drops the debezium-core / debezium-api exclusions on its connector-cdc-mysql dependency:
<artifactId>connector-cdc-mysql</artifactId>
<version>${project.version}</version>
<!-- Keep debezium-core and debezium-api: connector-cdc-base declares Debezium as provided ... -->Why it was needed: those exclusions were written when connector-cdc-base supplied Debezium at compile scope. With base now provided, nothing else put io.debezium.config.Configuration or io.debezium.engine.DebeziumEngine into the OceanBase jar, and the MySqlIncrementalSource-based source would hit NoClassDefFoundError on the normal startup path. So this is a real regression this PR would have introduced, not a hypothetical one, and it is fixed in the right place (the OceanBase module, no shared class touched).
plugin discovery -> filterPluginJar (AbstractPluginDiscovery) -> [connector-cdc-oceanbase.jar, connector-cdc-base.jar]
-> OceanBase source (MySqlIncrementalSource path) -> io.debezium.* runtime classes
stock core/api/embedded: now bundled transitively via connector-cdc-mysql into the OceanBase jar
ChangeEventQueue / TableId / HistorizedRelationalDatabaseConnectorConfig / HeartbeatFactory:
excluded by the connector-cdc parent shade filter (connector-cdc/pom.xml:151-164),
so connector-cdc-base's patched copies (incl. Serializable TableId) stay the only definition
OceanBase has no module-level shade filter, so it inherits the parent filter list unchanged; the stock patched classes cannot reappear.
1.2 Compatibility Impact
No CDC option, default, checkpoint or wire-format change. OceanBase still resolves Debezium 1.9.8.Final. The only operator-visible effect is the one already documented for the other connectors: the OceanBase jar must be upgraded together with connector-cdc-base. The EN and ZH incompatible-changes.md entries both gained a matching OceanBase line (docs/en/introduction/concepts/incompatible-changes.md:504, docs/zh/introduction/concepts/incompatible-changes.md:426), and they agree.
1.3 Performance / Side Effects
The OceanBase jar grows by the Debezium runtime payload, the same per-connector duplication already called out under Issue 3. No hot-path, thread, lock or resource-lifecycle change.
1.4 Error Handling and Logging
Nothing new. Previous items stand.
2. Code Quality Assessment
2.1 Code Standards
The new pom comment explains why the exclusions were removed and how the patched classes stay unshadowed. The new test method carries a short why-comment. Fine.
2.2 Test Coverage
TestOceanBaseCdcPackagingIT.java:60-71 adds packagedJarShipsStockDebeziumRuntime, which asserts the packaged jar contains io/debezium/config/Configuration.class and io/debezium/engine/DebeziumEngine.class. It has real failure power: on 51f37b1ae0 (exclusions still present) both entries are absent and it goes red. It complements the two existing assertions, which only proved the absence of shadowed classes and therefore passed trivially on an empty Debezium set; that blind spot is now closed.
Stability: Stable. It reads the packaged artifact via the failsafe connector.jar property; no timing, ports or containers. Note it is a failsafe test, so it only runs in jobs with -DskipIT=false (the root pom defaults skipIT=true); the plain unit-test job will not execute it.
2.3 Documentation
EN/ZH parity confirmed for the new OceanBase bullet.
3. Architecture
The fix keeps the staged design intact: ownership moves into the plugin, shared patches stay in base. One gap the OceanBase module exposes is Issue 8 below: OceanBase now ships Debezium but does not own a debezium.version, has no adapter, and is not covered by the per-connector drift tests.
4. Issue Summary
| No. | Issue | Location | Severity | Status |
|---|---|---|---|---|
| 1 | DB2 compile dependency omission | connector-cdc-db2/pom.xml | Blocker (earlier) | Fixed |
| 2 | Incorrect impact statement | incompatible-changes.md | Blocker (earlier) | Fixed |
| 3 | Quantify distribution-size impact | incompatible-changes guides | Low | Open, non-blocking |
| 4 | Check actual shaded plugin artifacts for all connectors | PatchedDebeziumClassesTest.java:101 | Low | Partially addressed for OceanBase only |
| 5 | Guard future CDC runtime dependency declarations | connector-cdc parent / tests | Medium | Open, non-blocking |
| 6 | DB2 adapter/SPI/test parity | connector-cdc-db2 | Low | Open, non-blocking |
| 7 | DB2 local version declaration | connector-cdc-db2/pom.xml | Low | Open, non-blocking |
| 8 | OceanBase does not own its Debezium version | connector-cdc-oceanbase/pom.xml:39-50 | Low | New, non-blocking |
Issue 8: OceanBase bundles Debezium without owning its version
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-oceanbase/pom.xml:39-50 - Problem: OceanBase gets
debezium-api/debezium-embedded/zstd-jnionly transitively throughconnector-cdc-mysql. In the OceanBase build those versions are re-managed by the inherited parentdependencyManagementat the parent fallbackdebezium.version(connector-cdc/pom.xml:54), whiledebezium-connector-mysqlarrives at whateverconnector-cdc-mysqldeclares. - Risk: today everything is
1.9.8.Final, so nothing breaks. Ifconnector-cdc-mysqllater bumps its owndebezium.version, OceanBase would silently ship a mixed core / connector-mysql pair, and no adapter drift test in the OceanBase module would catch it (the shaded jar also inheritsMySqlDebeziumAdapter's service entry, which would then report a version that does not match the bundled core). - Best improvement: either declare
debezium.versionplus explicitdebezium-api/debezium-embedded/zstd-jniin the OceanBase pom like the other connectors, or add a one-line assertion toTestOceanBaseCdcPackagingITthat the bundleddebezium-coreanddebezium-connector-mysqlpom.propertiesversions match. Fine as a follow-up. - Severity: Low
- Raised by another reviewer: No
All of @SEZ9's earlier points remain as reconciled in my previous self-review; nothing in this delta reopens them.
5. Merge Recommendation
Conclusion: Ready to merge after fixes
- Blockers: only CI and process. The merge conflicts from the last check are resolved (
mergeable_stateis nowblocked, i.e. review-required, notdirty). The fork Build for this exact head (run 41572360686) is still queued, so there is no current-head CI result yet; it must go green, including the CDC integration jobs that executeTestOceanBaseCdcPackagingIT. No non-CI source blocker remains. - Non-blocking: Issues 3-8.
As the author I cannot approve this PR; an independent maintainer needs to give the effective review once CI is green. The branch is 3 commits behind dev, which is not itself a reason to sync.
Purpose of this pull request
Follow-up to #10799, which added the
DebeziumAdapterSPI and per-connectordebezium.versionproperties as a staged foundation for per-connector Debezium version management.Those properties do not yet control the Debezium a connector runs on.
connector-cdc-basedeclaresdebezium-apianddebezium-embeddedatcompilescope, sodebezium-corereaches every CDC connector transitively at the version resolved inconnector-cdc-base's own POM. Setting<debezium.version>inconnector-cdc-mysqlchanges onlydebezium-connector-mysql; the Debezium core stays pinned by the shared module.That is not just a POM detail.
AbstractPluginDiscovery#filterPluginJarspecial-cases CDC and forcesconnector-cdc-baseinto the class loader of every CDC plugin, requiring exactly two jars:Checking the published jars confirms where Debezium actually lives today:
io.debeziumcore/embedded classesdebezium-connector-*classesconnector-cdc-baseconnector-cdc-mysqlSo the shared
connector-cdc-basejar is the single project-wide Debezium version, and no connector can override it. This PR moves that ownership to the connectors.Does this PR introduce any user-facing change?
No change to CDC job configuration, runtime behavior or checkpoint compatibility. Every in-tree connector still resolves Debezium
1.9.8.Final, so the set of classes on a CDC job's class loader is unchanged — only relocated from the shared jar into the connector jar.Two things do change for people building against SeaTunnel, both documented in
docs/{en,zh}/introduction/concepts/incompatible-changes.md:connectors/connector-cdc-base-*.jarno longer contains the stockio.debeziumruntime classes; it still ships the five patchedio.debezium.*classes listed under "Version compatibility" below.connector-cdc-baseto supply Debezium transitively must now declaredebezium-apianddebezium-embeddedthemselves.What changed
Dependency ownership
connector-cdc-basedeclaresdebezium-api,debezium-embeddedandzstd-jniasprovided. It still compiles against Debezium but no longer imposes a version on its consumers, and its shaded jar is Debezium-free.connector-cdcparent. Inherited dependency management is interpolated against the effective properties of the module being built, so each connector resolves Debezium at its owndebezium.versionwhile sharing one copy of the exclusion rules (kafka-log4j-appender, glassfish jersey, the Apple-M1zstd-jnioverride).debezium-api,debezium-embeddedandzstd-jniexplicitly.connector-cdc-opengaussandconnector-cdc-vitessgain thedebezium.versionproperty they were missing in [Feature][Connector-V2] Foundation for per-connector Debezium version management (staged) #10799.connector-cdc-tidbis intentionally untouched — it uses no Debezium classes. Verified at the bytecode level: the onlyconnector-cdc-baseclasses it touches (SourceOptions,StartupMode,TemporalConversions) contain zeroio/debeziumororg/apache/kafkaconstant-pool references, becauseCompatibleDebeziumJsonDeserializationSchema.IDENTIFIERis a compile-time constant that javac inlines.The SPI gets real providers
Each connector now registers a
DebeziumAdapterviaMETA-INF/services, so the extension point added in #10799 is populated and the version a connector ships is introspectable at runtime.connector-cdc-opengaussdeliberately registers none. It runs Debezium's PostgreSQL connector and shadesconnector-cdc-postgresinto its own jar, and the shade plugin is configured withServicesResourceTransformer, which mergesMETA-INF/servicesentries. A second provider forio.debezium.connector.postgresql.PostgresConnectorwould therefore land in the same jar and make every openGauss CDC job fail the exactly-one-match rule.OpengaussDebeziumAdapterTestpins that so a future contributor cannot reintroduce it silently.How was this patch tested?
Unit tests per connector assert that the adapter resolves uniquely for its connector class, that it claims no other connector class, and — the drift guard — that its declared version equals the
debezium-coreversion actually resolved onto the module classpath (read fromMETA-INF/maven/io.debezium/debezium-core/pom.properties). A POM bump that forgets the adapter, or vice versa, fails the build.The ownership claim itself was verified with
dependency:tree. Before,debezium-corewas fixed for everyone. After, with onlyconnector-cdc-mysql's property temporarily set to1.9.7.Final:Two CDC connectors resolving genuinely different Debezium cores — not possible before this change. That probe was reverted; everything in this PR is on
1.9.8.Final.Version compatibility
Three things have to keep agreeing once Debezium ownership moves, and each is now enforced rather than assumed.
1. connector-cdc-base ships patched Debezium classes, and they must stay the only copies.
connector-cdc-baserecompiles five Debezium classes undersrc/main/java/io/debezium:io.debezium.connector.base.ChangeEventQueue(+$1,$Builder)io.debezium.relational.TableIdio.debezium.relational.HistorizedRelationalDatabaseConnectorConfigio.debezium.heartbeat.HeartbeatFactoryio.debezium.heartbeat.DefaultHeartbeatConnectionProviderComparing the shipped jars confirms the patched copies currently win — all four overlapping classes differ byte-for-byte from debezium-core, because the shade plugin resolved the overlap at build time inside one jar.
Moving Debezium into the connector jars breaks that: the stock copies would arrive in the connector jar while the patched ones stay in
connector-cdc-base, with both jars on the same class loader.AbstractPluginDiscoverybuilds that class path fromFile#listFiles, so which copy wins would depend on filesystem ordering, and the patches could silently stop applying on some machines.The CDC shade configuration now excludes those classes from
debezium-core, keepingconnector-cdc-base's patched copies the single definition exactly as before.connector-cdc-opengaussdeclares its own filter list at execution level, which takes precedence over the inherited one, so it restates the filter.PatchedDebeziumClassesTestpins the patched set against the exclusion list, so a newly patched class cannot be added without updating both.2. Modules that share a Debezium version must not drift apart.
The per-connector tests now assert that every Debezium artifact on a connector's class path resolves to the single version its adapter declares, rather than spot-checking
debezium-core. This matters most forconnector-cdc-opengauss: it declares its owndebezium.versionfordebezium-embeddedbut reachesdebezium-connector-postgrestransitively throughconnector-cdc-postgres, and it recompiles Debezium's PostgreSQL connection internals. Bumping one module and not the other now fails the build instead of shipping two Debezium versions on one class path.3. How far a connector may actually diverge today.
Packaging ownership has moved, but
connector-cdc-baseis still compiled against a single baseline and is loaded into every CDC connector's class loader. It uses roughly 85 Debezium types acrossio.debezium.relational,io.debezium.pipeline,io.debezium.schema,io.debezium.configandio.debezium.jdbc, plus the five patched classes above.So divergence is safe only within a Debezium line that keeps those APIs binary compatible — a connector can move within
1.9.xtoday, not to a release that reshapesrelational/pipeline. Making major-version divergence real requiresconnector-cdc-base's Debezium usage to move behind theDebeziumAdapterSPI, which is exactly what the SPI exists for and is not attempted here. This is stated inincompatible-changes.mdso the capability is not over-claimed.Trade-off for reviewers
This increases the size of the binary distribution. The Debezium runtime is no longer shared:
connector-cdc-basedrops from ~24 MB to roughly 1 MB (only ~0.37 MB of it was ever SeaTunnel code — the rest is Kafka Connect ~4.9 MB, Guava, Jetty, snappy, jackson and thezstd-jninative binaries), and each of the 7 Debezium-based connector jars gains that payload instead. Net growth is on the order of +140 MB.That cost is inherent to the goal: if a connector can pin its own Debezium, it has to carry it. Sharing one copy is exactly what makes overriding impossible today.
If the size is not acceptable, the alternative worth discussing is shipping versioned shared Debezium runtime jars (e.g.
connector-cdc-debezium-1.9.8) and havingAbstractPluginDiscoveryload the one each connector declares. That reclaims the sharing for connectors on the same version while still allowing divergence, but it rewires the plugin-discovery path — a runtime change deliberately kept out of this staged PR. Raising it here as a draft so that direction can be settled before anything in the runtime path moves.Concrete follow-up milestone: MySQL 9.x connector on Debezium 3.x
The first real consumer of per-connector versions is a MySQL 9 CDC connector. MySQL 8.4/9.0 removed the legacy replication commands Debezium 1.9 depends on (
SHOW MASTER STATUSis gone in favor ofSHOW BINARY LOG STATUS), so current MySQL-CDC cannot follow MySQL past 8.x, and Debezium only added MySQL 9 support in its 2.7/3.x line. Landing that connector has two hard prerequisites, which is why it is not part of this PR:java.version=1.8, CIInstallruns Temurin 8), so a module depending on Debezium 3.x cannot compile anywhere in today's build. [Improve][Core] Raise minimum Java runtime to 11 and move CI to JDK 11/17 #11545 (JDK 17 CI) is the gating PR.connector-cdc-basecompiles against ~85 Debezium 1.9 types and patches five Debezium classes; Debezium 2.0 renamed the schema-history API this framework implements (DatabaseHistory→SchemaHistory), among other reshapes. Those touchpoints must move behind theDebeziumAdapterSPI before a connector can bring a 3.x core onto a class path shared with cdc-base.One additional trap for that PR, noted here so it is not rediscovered:
AbstractPluginDiscoveryresolves CDC plugin jars by filename prefix, and a jar namedconnector-cdc-mysql*anything would make the new plugin unresolvable (its jar matches two plugin identifiers and is discarded, leaving only cdc-base and failing the exactly-two-jars rule). The new module needs a non-prefix-colliding artifact id or a discovery fix.Staged order: this PR (packaging ownership) → #11545 (JDK 17) → SPI-ify cdc-base's Debezium touchpoints →
MySQL9-CDCconnector on Debezium 3.x with MySQL 9 E2E.Check list
incompatible-changes.mdupdated inenandzh