DanielLeens commented on PR #11639:
URL: https://github.com/apache/seatunnel/pull/11639#issuecomment-5168793217
# What Problem This PR Solves
- User pain point
Today every Debezium-based CDC connector inherits Debezium from
`connector-cdc-base`, and because that base jar is loaded into every CDC
connector classloader, no connector can really own or override the Debezium
runtime it ships with.
- Fix approach
This PR makes `connector-cdc-base` compile against Debezium with
`provided` scope, moves the runtime Debezium dependencies into each connector
module, adds a lightweight adapter SPI plus module-local adapter tests, and
documents the packaging change in both `docs/en` and `docs/zh`.
- One-sentence summary
The goal is to move Debezium runtime ownership from the shared CDC base
jar to the individual CDC connector modules.
# I. Code Change Review
## 1.1 Core Logic Analysis
### Exact change description
The core packaging changes are in:
- `seatunnel-connectors-v2/connector-cdc/connector-cdc-base/pom.xml:50-103`
- `seatunnel-connectors-v2/connector-cdc/pom.xml:44-99`
- per-connector poms such as:
- `connector-cdc-mysql/pom.xml`
- `connector-cdc-postgres/pom.xml`
- `connector-cdc-opengauss/pom.xml:32-85`
- `connector-cdc-oracle/pom.xml`
- `connector-cdc-sqlserver/pom.xml`
- `connector-cdc-mongodb/pom.xml`
- `connector-cdc-vitess/pom.xml`
- SPI and guardrail tests:
-
`connector-cdc-base/src/main/java/org/apache/seatunnel/connectors/cdc/base/debezium/DebeziumAdapter.java:22-44`
- connector-local `*DebeziumAdapter.java`
- connector-local `*DebeziumAdapterTest.java`
### Before / after snippets
Before, `connector-cdc-base` shipped the shared Debezium runtime:
```xml
<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:
```xml
<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
- The packaging change matches the real plugin classloader problem described
in `connector-cdc-base/pom.xml:52-58`.
- All Debezium-based in-tree connectors now explicitly declare their own
`debezium.version` and runtime Debezium dependencies.
- `connector-cdc-opengauss` deliberately reuses the PostgreSQL adapter by
depending on `connector-cdc-postgres`, and the added test documents that
exactly-one-adapter rule.
- The new `DebeziumAdapter` SPI is currently a guardrail layer and
forward-compatibility seam, not a runtime behavior change by itself.
### Deep logic analysis
The relevant packaging/runtime path is:
```text
Plugin discovery
-> connector-cdc-base is loaded into every CDC connector classloader
-> if base ships Debezium classes, that shared copy wins for everybody
This PR
-> connector-cdc-base/pom.xml:50-74
-> Debezium dependencies become provided only
-> base jar no longer ships io.debezium classes
Per connector ownership
-> connector-cdc parent pom manages exclusions and baseline version
-> each Debezium-based connector pom declares:
- its own debezium.version
- debezium-api
- debezium-embedded
- zstd-jni
- connector-specific debezium-connector-*
Guardrails
-> DebeziumAdapter SPI + META-INF/services entry per connector
-> connector-local adapter tests
-> verify one adapter per connector.class
-> verify packaged Debezium version matches module declaration
```
For openGauss specifically:
```text
connector-cdc-opengauss
-> declares its own debezium.version [opengauss pom:32-40]
-> packages debezium-api / debezium-embedded / zstd-jni [56-67]
-> depends on connector-cdc-postgres [81-85]
-> inherits the PostgreSQL adapter registration
-> OpengaussDebeziumAdapterTest
-> verifies exactly one Postgres adapter is visible
-> prevents accidental double-registration later
```
Local verification results used for this review:
- `gh pr view 11639 --json ...`: completed, used to confirm the latest head
(`35882cf6313a2b413dc7363f3934a653817b37c9`) and current checks.
- `gh api
repos/apache/seatunnel/compare/dev...35882cf6313a2b413dc7363f3934a653817b37c9`:
completed, current compare status is `ahead`, `ahead_by=1`, `behind_by=0`.
- `gh api repos/apache/seatunnel/pulls/11639`: completed, `mergeable=true`,
`mergeable_state=blocked`, `rebaseable=false`.
- `gh api repos/apache/seatunnel/pulls/11639/reviews`: completed, no prior
reviews were present at the time of this pass.
- `git diff upstream/dev...seatunnel-review-11639 -- <changed files>`:
completed for the full changed set.
- Build / unit tests / E2E execution: not run locally. Per this batch's
SeaTunnel review rule, I relied on source analysis and current GitHub check
state.
## 1.2 Compatibility Impact
Judgment: **Partially incompatible**
Impacts:
- API: no user config contract change for in-tree CDC connectors.
- Configuration: no CDC job config change for in-tree connectors.
- Default behavior: no intended runtime behavior change for in-tree
connectors because they still resolve Debezium `1.9.8.Final`.
- Protocol / serialization / checkpoint: unchanged for in-tree connectors.
- Historical behavior: third-party CDC connectors that relied on
`connector-cdc-base` to bring Debezium transitively now need to declare
Debezium dependencies themselves.
Migration note:
- Third-party connectors built against `connector-cdc-base` should
explicitly declare `debezium-api` and `debezium-embedded` instead of relying on
the base jar to ship them.
## 1.3 Performance / Side-Effect Analysis
- CPU / memory / GC: unchanged at runtime for in-tree connectors.
- Network / concurrency / retry / idempotency / resource release: unchanged.
- Packaging side effect: the Debezium runtime is now duplicated into each
Debezium-based connector jar instead of being shared through
`connector-cdc-base`, so binary distribution size increases. The PR documents
that explicitly.
## 1.4 Error Handling and Logging
I did not find a new source-side blocker in the current head.
# II. Code Quality Assessment
## 2.1 Coding Standards
The 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 Stability
The new adapter tests cover the most important guardrails:
- discoverability via `META-INF/services`
- one-adapter-per-connector-class uniqueness
- declared `debezium.version` matches the Debezium actually resolved on the
module classpath
- openGauss intentionally reuses the PostgreSQL adapter without introducing
a second provider
Test stability conclusion: **Stable**
- The added tests are deterministic and do not rely on time, fixed ports,
external services, or concurrent scheduling.
## 2.3 Documentation Updates
The incompatible-change note was updated in both:
- `docs/en/introduction/concepts/incompatible-changes.md`
- `docs/zh/introduction/concepts/incompatible-changes.md`
That matches the user-visible packaging change for third-party connector
builders.
# III. Architecture Rationality
## 3.1 Elegance of the Solution
This is a long-term packaging fix rather than a workaround. It addresses the
shared-classloader root cause directly.
## 3.2 Maintainability
The 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 Extensibility
The 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 Compatibility
For 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 Summary
No formal issues found in the current head.
# V. Merge Recommendation
### Conclusion: Can be merged
1. Blocking items (must fix)
- No new source-side blocker from Daniel's side in the current head.
2. Suggested improvements (non-blocking)
- None from the current source review.
Overall assessment:
This is a well-reasoned packaging cleanup with good guardrail tests and the
right compatibility note for third-party CDC connector authors.
Remaining gates:
- The current-head CI is still pending on GitHub.
- Since this PR changes packaging for multiple CDC connectors, the final
merge should still rely on an independent write-capable maintainer gate once CI
is complete.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
