Repository navigation
[Fix][Connector-V2] Fix DB2 CDC capture and savepoint recovery - #12970
DanielLeens wants to merge 4 commits into
Conversation
DanielLeens
left a comment
There was a problem hiding this comment.
Thanks for this focused DB2 CDC follow-up. Transparency note first: this PR is authored by the DanielLeens account, so this is a maintainer-style self-review written as neutrally and adversarially as I can, and another committer should make the final call. I reviewed the full diff at head ce17596241db1df706170bbc87884855eddb912b, traced the capture-table and history-restore paths, and checked the Debezium 1.9.8 Db2Connection bytecode directly to confirm the root-cause claim.
What Problem Does This PR Solve?
- User pain point: on an active ASN Capture registration with a non-null
CD_OLD_SYNCHPOINT, the DB2 CDC reader stops emitting committed changes at or beyond that point; savepoint restore puts DB2's two-partschema.tablehistory into the wrong namespace; lowercase unquotedtable-namesdo not match DB2's uppercase metadata;enable_concurrent_read = falseis not forwarded. - Fix approach: a DB2-local
Db2CdcConnectionthat overrideslistOfChangeTables()(start = OLD, stop = NULL for the active registration); the sharedConnectTableChangeSerializerhonors its existinguseCatalogBeforeSchemaargument andJdbcSourceFetchTaskContextderives it from the splitTableId; factory-level case folding of unquoted names; null-safety in the dialect and metadata provider; independent close of both connections. - One-sentence summary: DB2 CDC keeps streaming after the capture sync point advances and restores history into the right schema.
1. Code Change Review
1.1 Core Logic Analysis
Root cause check: in Debezium 1.9.8, Db2Connection.listOfChangeTables runs select r.SOURCE_OWNER, r.SOURCE_TABLE, r.CD_OWNER, r.CD_TABLE, r.CD_NEW_SYNCHPOINT, r.CD_OLD_SYNCHPOINT, ... and builds new Db2ChangeTable(new TableId("", col1, col2), col4, col9, Lsn.valueOf(col5), Lsn.valueOf(col6)), i.e. start = NEW and stop = OLD. So the PR description is accurate, and the override in Db2CdcConnection is the right boundary. listOfNewChangeTables uses a query with AND 1=0, so there is no second code path that re-introduces the old mapping at runtime.
// Db2CdcConnection.listOfChangeTables (new)
new Db2ChangeTable(new TableId("", owner, table), cdTable, objectId,
Lsn.valueOf(rs.getBytes(4) /* CD_OLD_SYNCHPOINT */), Lsn.NULL);History restore before/after:
// before: argument ignored, two-part ids always parsed catalog-first
toTable(struct.getStruct(TABLE_KEY), TableId.parse(tableId));
// after
toTable(struct.getStruct(TABLE_KEY), TableId.parse(tableId, useCatalogBeforeSchema));
// JdbcSourceFetchTaskContext.registerDatabaseHistory
boolean useCatalogBeforeSchema = tableId.catalog() != null && !tableId.catalog().isEmpty();IncrementalSplit restore (checkpoint/savepoint)
-> JdbcSourceFetchTaskContext.registerDatabaseHistory(split)
for split TableId: historyTableChanges bytes -> JsonConverter -> Struct
-> ConnectTableChangeSerializer.deserialize(struct, catalog non-empty)
two-part "DB2INST1.CUSTOMERS" + catalog "" -> schema=DB2INST1 (was catalog=DB2INST1)
-> EmbeddedDatabaseHistory.registerHistory -> Debezium schema lookup by TableId
-> Db2StreamingChangeEventSource -> Db2CdcConnection.listOfChangeTables()
-> active registration: start=OLD, stop=NULL -> no premature stop
Shared-path audit of the serializer change: only two callers exist. In JdbcSourceFetchTaskContext, MySQL split ids have a non-empty catalog (unchanged, catalog-first); Oracle, SQL Server and Postgres split ids are three-part, which TableId.parse handles independently of the flag. Only two-part ids with an empty catalog (DB2) change. The second caller is Vitess (see Issue 3).
1.2 Compatibility Impact
Partially incompatible, pending Issue 2: option names, defaults, offsets and checkpoint formats are unchanged, and the MySQL/Oracle/SQL Server/Postgres restore paths are unaffected (inference from the TableId shapes above). However, the new case folding also rewrites the database component of configured table names, which can change table identity for existing jobs.
1.3 Performance / Side-Effect Analysis
No hot-path cost: normalization runs once at source creation, and the capture query runs at the same points as before.
1.4 Error Handling and Logging
Issue 1: The DB2 connector does not compile at the current head
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/db2/source/Db2IncrementalSourceFactory.java:142 - Problem:
options.getConfData()does not exist onReadonlyConfig(available accessors aregetSourceMap(),toMap(),toConfig()). The fork Build for this head fails withcannot find symbol: method getConfData()inconnector-cdc-db2on every unit-test lane and bothupdated-modules-integration-test-part-2lanes. - Risk: none of the 16 unit regressions or the three new DB2 E2E scenarios have run.
- Best improvement: use
new LinkedHashMap<>(options.getSourceMap()), compileconnector-cdc-db2and its tests, and re-run the Build. Please also drop "Independent static review found no source-level blocker" from the PR description, since it cannot be verified by readers and the head does not compile. - Severity: High
- Raised by another reviewer: No
Issue 2: Case folding also uppercases the database component
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/db2/utils/TableDiscoveryUtils.java:111(normalizeConfiguredTableName), used byDb2IncrementalSourceFactory.normalizeDb2OptionsandtoConfiguredDb2TableId:92 - Problem:
"testdb.DB2INST1.CUSTOMERS".toUpperCase()becomesTESTDB.DB2INST1.CUSTOMERS. Debezium filtering only usesschema.table, and DB2 database aliases are case-insensitive on the server, but on the SeaTunnel side the database name is part of theTablePaththat flows into theCatalogTable, the row table id, sink placeholders such as${database_name}, and per-table sink state keys. A user whose existing config uses a lowercase database (common, because the JDBC URL usually does) gets a different table identity after upgrade, including when restoring a savepoint taken before the upgrade. The new lowercase E2E uses an uppercase database (E2E), so this path is not covered. - Risk: silent identity change for existing jobs: different downstream table names, and sink writer state that no longer matches on restore.
- Best improvement: fold only the schema and table components (the parts DB2 stores in uppercase metadata) and keep the database component as configured; add a unit test with a lowercase database component and, ideally, an E2E config with a lowercase database in both
urlandtable-names. If folding the database is intended, document it indocs/en/introduction/concepts/incompatible-changes.mdwith migration guidance. - Severity: Medium
- Raised by another reviewer: No
Issue 3: Vitess restore behavior changes, but the description says it does not
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/main/java/org/apache/seatunnel/connectors/cdc/debezium/ConnectTableChangeSerializer.java:126; callerVitessTableSchemaState.deserialize(passesfalse) - Problem: before this PR the
falseargument was ignored, so Vitess checkpoint bytes were restored catalog-first (TableId(db, null, table)). After it, they restore schema-first (TableId(null, db, table)). That matchesVitessTableSchemaState.fromCatalogTableand, per the new test, the real decoder, so it looks like a real fix, but it is a runtime behavior change for Vitess restores, not "keeps its existing schema-first call". - Risk: reviewers and release notes miss a behavior change on a second connector.
- Best improvement: state in the PR description that Vitess checkpoint restore now produces the schema-first identity, and confirm
VitessTableSchemaStateTestrestores bytes produced by the pre-PR serializer (same JSON) into the decoder's identity. - Severity: Low
- Raised by another reviewer: No
Issue 4: Missing catalog metadata is now silently treated as "no key"
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/db2/source/Db2Dialect.java:163-176 - Problem: when
tableMaphas no entry for aTableId,getPrimaryKeynow returns empty andgetConstraintKeysan empty list instead of throwing an NPE. That avoids a crash, but an identifier mismatch (the very class of bug this PR fixes) would now degrade silently into a table without keys, which changes snapshot splitting. - Best improvement: log a WARN with the table id when the lookup misses, so a normalization mismatch is visible.
- Severity: Low
- Raised by another reviewer: No
2. Code Quality Assessment
2.1 Coding Standards
New classes and methods carry Javadoc. Minor wording: the Javadoc on normalizeTableConfig says "Preserves Apache's custom primary keys"; "user-configured primary keys" is clearer.
2.2 Test Coverage and Test Stability
The test design is good and would fail if the fix were reverted: the PR states that the original DB2 E2E reconciliation now fails under the old stop-LSN mapping because the synthetic registration has a non-null OLD point, and the history-restore tests compare catalog and schema separately. The new E2E scenarios use Awaitility with bounded waits and a real savepoint/restore; the class keeps its existing @DisabledOnContainer scope. Stability rating: Risk present until they actually run (Issue 1); I see no sleeps or fixed host ports in the added code. Gap: lowercase database component (Issue 2).
2.3 Documentation Updates
docs/en and docs/zh are both updated and consistent. One suggestion: the paragraph about the synthetic ASNCDC catalog in the E2E (docs/en/connectors/source/DB2-CDC.md:211 and the zh counterpart) describes test internals rather than user behavior; it belongs in the PR description. If Issue 2 is kept as designed, the docs should say that the database component is uppercased too.
3. Architectural Soundness
3.1 Elegance
Precise fix. The capture mapping is corrected at the JDBC boundary inside the DB2 module without replacing a third-party class, and the shared serializer change makes an existing argument effective instead of adding a connector special case to connector-cdc-base.
3.2 Maintainability
Good: the DB2-specific behavior stays in the DB2 module.
3.3 Extensibility
Other connectors that use two-part schema-first ids can rely on the same split-id-driven flag.
3.4 Historical-Version Compatibility
Checkpoint and offset formats are unchanged. Residual risks: (a) the synthetic ASNCDC catalog does not validate IBM's real ASN Capture behavior, for example how far CD_OLD_SYNCHPOINT can lag after pruning and whether starting from it re-reads already delivered changes on a restart without an offset (the PR states this limitation honestly); (b) Issue 2 for existing configs with a lowercase database.
4. Issue Summary
| Number | Issue | Location | Severity |
|---|---|---|---|
| 1 | ReadonlyConfig.getConfData() does not exist; DB2 module does not compile |
seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/db2/source/Db2IncrementalSourceFactory.java:142 | High |
| 2 | Case folding also uppercases the database component (table identity change, untested) | seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/db2/utils/TableDiscoveryUtils.java:111 | Medium |
| 3 | Vitess restore identity changes; PR description says it does not | seatunnel-connectors-v2/connector-cdc/connector-cdc-base/src/main/java/org/apache/seatunnel/connectors/cdc/debezium/ConnectTableChangeSerializer.java:126 | Low |
| 4 | Missing table metadata silently becomes "no key" | seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/db2/source/Db2Dialect.java:163 | Low |
5. Merge Recommendation
Conclusion: Not recommended for merge
Blockers (must fix):
- Issue 1: fix the compile error and get a green Build on the new head, including the three new DB2 E2E scenarios.
- Issue 2: either limit folding to schema and table, or document it as an incompatible change with a migration note; add a lowercase-database test either way.
Recommended fixes (non-blocking): Issues 3 and 4, plus the docs and description cleanups.
Overall assessment: the root cause is real and confirmed against the Debezium 1.9.8 bytecode, and the fix is in the right place. The CI failure is caused by this PR (compile error), not by being 5 commits behind dev, so a sync is not needed for that.
|
Addressed the four points in the existing review with commit
Also clarified the user-configured-key comment, kept test implementation details in this PR instead of the user guide, and removed the inaccurate prior static-check statement. Local Spotless checks passed for DB2, Vitess and DB2 E2E; compilation, unit tests and E2E remain GitHub CI responsibilities under the agreed validation policy. The new exact-head Build is queued at this snapshot. No passing runtime or merge-readiness claim is made. |
DanielLeens
left a comment
There was a problem hiding this comment.
Transparency note first: this PR is authored by the DanielLeens account, so this is a maintainer-style self-review written as neutrally and adversarially as I can, and another committer should make the final call. Head bfe60da4e81dd3f6c63f0ec61249a3b3b469fdf8 moved since my previous review on ce175962; the new commit is the follow-up to my four earlier points. I re-reviewed the complete diff against dev, not only the delta, and re-traced the capture-table, history-restore and factory paths. Code reading plus CI facts only; I did not compile or run anything locally.
Status of my earlier four points on this head:
- Compile error (
getConfData()): fixed;Db2IncrementalSourceFactory.normalizeDb2Optionsnow copiesoptions.getSourceMap(), and the exact-head fork Build compilesconnector-cdc-db2. - Database component uppercased: fixed;
TableDiscoveryUtils.normalizeConfiguredTableNamefolds only what follows the first dot. - Vitess behavior change: now stated in the PR description and pinned by
VitessTableSchemaStateTest. - Silent missing key metadata:
Db2Dialect.getPrimaryKeyandgetConstraintKeyslog a WARN with the table id.
What Problem Does This PR Solve?
- User pain point: on an active ASN Capture registration with a non-null
CD_OLD_SYNCHPOINT, the DB2 CDC reader stops emitting committed changes at or beyond that point; savepoint restore puts DB2's two-partschema.tablehistory into the wrong namespace; lowercase unquotedtable-namesdo not match DB2's uppercase metadata;enable_concurrent_read = falseis not forwarded. - Fix approach: a DB2-local
Db2CdcConnectionthat maps an active registration to start = OLD, stop = none; the sharedConnectTableChangeSerializerhonors its existinguseCatalogBeforeSchemaargument andJdbcSourceFetchTaskContextderives it from the splitTableId; case folding of unquoted schema/table names in the DB2 factory; null-safety and independent connection close in the DB2 module. - One-sentence summary: DB2 CDC keeps streaming after the capture synchronization point advances and restores saved history into the right schema.
1. Code Change Review
1.1 Core Logic Analysis
IncrementalSplit restore (checkpoint/savepoint)
-> JdbcSourceFetchTaskContext.registerDatabaseHistory(split) (cdc-base, JdbcSourceFetchTaskContext.java:~176-200)
history key was written by ConnectTableChangeSerializer.serialize(): tableChange.getId().toDoubleQuotedString() (:192)
useCatalogBeforeSchema = split TableId.catalog() non-empty
-> ConnectTableChangeSerializer.deserialize(structs, useCatalogBeforeSchema) -> TableId.parse(id, flag) (:122-126)
DB2: split id has empty catalog -> "SCHEMA"."TABLE" restores as schema.table (was catalog.table)
MySQL: catalog non-empty -> catalog-first, identical to the pre-PR parse default
Oracle / SQL Server / Postgres: split ids are three-part (catalog, schema, table); parse is flag-independent
-> EmbeddedDatabaseHistory.registerHistory
-> DB2 streaming: Db2CdcConnection.listOfChangeTables()
SELECT ... FROM ASNCDC.IBMSNAP_REGISTER r LEFT JOIN SYSCAT.TABLES t ... -> Db2ChangeTable(startLsn = CD_OLD_SYNCHPOINT, stopLsn = Lsn.NULL)
Shared-path audit (the serializer is shared by every JDBC CDC connector): the only runtime caller that previously relied on the ignored argument is registerDatabaseHistory, and the MySQL, Oracle, SQL Server and Postgres split-id shapes keep their prior result (inference from the TableId constructors in each dialect, and from the unchanged MySQL/Oracle CDC lanes passing on this head). The Vitess caller passes false and now genuinely gets schema-first identity, which is what its decoder and fromCatalogTable already use.
Other deltas verified: Db2SourceConfigFactory.create forwards enableConcurrentRead (the option only changes snapshot split generation in AbstractJdbcSourceChunkSplitter:71, so savepoint restores of existing splits are unaffected); Db2SourceFetchTaskContext.close() closes both connections independently; the three source == null checks in Db2EventMetadataProvider now test sourceInfo.
1.2 Compatibility Impact
Partially incompatible, pending Issue 1. Option names, defaults, offsets and checkpoint writing formats are unchanged, and the shared-serializer change preserves the other connectors' restore results. The DB2 connector shipped in the v3.0.0 release, so configuration that worked there must keep working; see Issue 1 for the one place I cannot show that.
1.3 Performance / Side-Effect Analysis
Normalization runs once at source creation; the capture query runs at the same points as before; the CD_NEW_SYNCHPOINT/owner columns are no longer selected, which is slightly cheaper. Nothing sits on a per-row path.
1.4 Error Handling and Logging
Missing catalog metadata now logs a WARN with the table id instead of an NPE. Close failures are logged per connection. Validation errors for explicit table-names still print the user's original spelling.
Issue 1: Folding unquoted names to uppercase can break configs that matched lowercase objects in 3.0.0
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/db2/utils/TableDiscoveryUtils.java(normalizeConfiguredTableName), applied inDb2IncrementalSourceFactory.normalizeDb2Options - Problem: before this PR the configured schema/table text reached the catalog lookup unchanged. A DB2 object created with a quoted lowercase name (for example
"customers") was referenced in 3.0.0 by writing it in lowercase without quotes. After this PR the same config becomes...CUSTOMERSand no longer matches. The new docs describe the folding, but it is not recorded as an incompatible change, and no test covers a lowercase-created object. I could not show from source that such configs are impossible; this is an unverified compatibility edge, not a demonstrated regression. - Risk: existing jobs that targeted case-sensitive lowercase objects fail at startup with "not enabled for capture" (fail-fast, no data loss) after an upgrade.
- Best improvement: either (a) fold only when the folded name is found and fall back to the configured spelling when only that one matches, or (b) keep the folding and add an entry to
docs/en/introduction/concepts/incompatible-changes.md(and zh) telling such users to quote the name, plus a unit test for the lowercase-object case. - Severity: Medium
- Raised by another reviewer: No
Issue 2: A single double quote disables folding for the whole name
- Location:
TableDiscoveryUtils.java, first line ofnormalizeConfiguredTableName(if (configuredTable.contains("\""))) - Problem:
e2e.db2inst1."Customers"keepsdb2inst1in lowercase, so the unquoted schema part fails to matchDB2INST1even though the table part is intentionally case-preserved. - Risk: surprising partial matching for mixed quoted/unquoted names (fail-fast, so low impact).
- Best improvement: fold per component (split on dots outside quotes) or document that quoting any component disables folding for all. The existing tests only cover a fully quoted table part.
- Severity: Low
- Raised by another reviewer: No
Issue 3: enable_concurrent_read = false now actually takes effect, and it is not listed as a behavior change
- Location:
Db2SourceConfigFactory.java(config.setEnableConcurrentRead(enableConcurrentRead)) - Problem: jobs that already set the option to
falsewere silently running with concurrent snapshot splitting; after upgrade a new snapshot phase will use a single full-table split (AbstractJdbcSourceChunkSplitter:71). That is the documented meaning of the option, but it can change snapshot duration and memory for those jobs. - Risk: performance surprise on large tables for users who set the option without realizing it was ignored.
- Best improvement: mention it in the DB2 CDC user guide changes or the incompatible-changes page in one sentence.
- Severity: Low
- Raised by another reviewer: No
2. Code Quality Assessment
2.1 Coding Standards
New classes and non-trivial methods carry Javadoc that explains why (for example the Db2CdcConnection class comment on Debezium's NEW/OLD mapping). No wildcard imports; @Slf4j used for logging. The ASF license header is present on Db2CdcConnection and the License header job is green.
2.2 Test Coverage and Test Stability
Rating: Stable on the evidence of CI. On the exact head, the fork Build run 40163820782 completed successfully; all four unit-test lanes (JDK 11/17, ubuntu and windows), License header, Code style, the updated-modules-integration-test-part-1 to part-8 lanes on both JDKs, and mysql-cdc-connector-it and oracle-cdc-connector-it on both JDKs are green. In the log of the updated-modules-integration-test JDK 11 lane that runs connector-cdc-db2-e2e, Db2CDCIT ran 4 tests with 0 failures and 0 skipped, so the three new E2E scenarios (lowercase config, savepoint restore with stopped-job writes, new table after savepoint) did execute. The unit tests are behavioral and would fail on revert (capture mapping start/stop, history restore catalog/schema compared separately, normalization with the database component preserved, absent key metadata, close-failure cleanup). The only gap I see is Issue 1 and Issue 2 inputs. Limitation already stated in the description: the synthetic ASNCDC catalog does not validate real IBM ASN Capture behavior (for example how far CD_OLD_SYNCHPOINT can lag after pruning).
2.3 Documentation Updates
docs/en/connectors/source/DB2-CDC.md and the zh file are updated consistently (capture registration semantics, folding rules, recovery). Add the compatibility notes from Issues 1 and 3 there or in incompatible-changes.md.
3. Architectural Soundness
3.1 Elegance
The capture mapping is corrected at the JDBC boundary inside the DB2 module without replacing a third-party class, and the shared serializer change makes an existing argument effective instead of adding a connector special case to connector-cdc-base.
3.2 Maintainability
DB2-specific behavior stays in the DB2 module; the shared change is a two-line, caller-derived flag.
3.3 Extensibility
Other connectors with two-part schema-first ids can rely on the same split-id-driven flag.
3.4 Historical-Version Compatibility
Checkpoint and offset formats are unchanged and other connectors' restore results are preserved; the open items are the DB2 name-matching edge (Issue 1) and the now-honored option (Issue 3).
4. Issue Summary
| Number | Issue | Location | Severity |
|---|---|---|---|
| 1 | Uppercase folding may break 3.0.0 configs that matched lowercase-created objects; not documented as incompatible | TableDiscoveryUtils.normalizeConfiguredTableName |
Medium |
| 2 | One quote disables folding for the whole name | TableDiscoveryUtils.normalizeConfiguredTableName |
Low |
| 3 | enable_concurrent_read=false now honored, not listed as behavior change |
Db2SourceConfigFactory.create |
Low |
5. Merge Recommendation
CI facts for head bfe60da4e81dd3f6c63f0ec61249a3b3b469fdf8 (read once): the fork Build run 40163820782 is completed with success; the apache-side PR signal is action_required (a pointer only). Merge state: mergeable, 18 commits behind dev, no conflict; BLOCKED is the review requirement, so no sync is requested.
Conclusion: Ready to merge after fixes
- Blockers - must be fixed
- Issue 1: either make the lookup backward compatible or record the change in
incompatible-changes.md(en and zh) with the quoting migration, and add a test for a lowercase-created object.
- Recommended fixes - non-blocking
- Issue 2 and Issue 3.
Overall assessment: the root cause (Debezium 1.9.8 maps NEW/OLD to start/stop) is real and fixed at the right boundary, the shared serializer change keeps the other connectors' results, and CI including the DB2 E2E is green on the exact head. This is my own PR, so I am not approving it; an independent committer approval is required after Issue 1 is resolved.
Better alternative: none needed.
…nfigured unquoted Resolve each configured DB2 table name against the catalog: the folded uppercase spelling is used when such an object exists, and the configured spelling is kept when only it exists, so configurations that matched quoted-lowercase objects in 3.0.0 keep working. Startup capture validation accepts either spelling.
|
Synced with Merge with
Issue 1 (blocker): backward-compatible table-name lookup instead of a documented break
Issue 3 (non-blocking): the PR description now has an explicit "Behavior change" paragraph for Local validation was limited to |
DanielLeens
left a comment
There was a problem hiding this comment.
This is a self-review: DanielLeens authored this PR, so an independent committer must make the approval decision. I reviewed the complete current diff at 74a90d2 against dev, including the new catalog fallback and the actual v3.0.0 lookup path. The fallback fixes the lowercase-only case, but the both-spellings case still changes an existing job's table identity, and the documented quoting escape hatch does not work through the actual catalog/capture path. I cannot close the compatibility blocker. My earlier suggestion to prefer the folded spelling was incomplete; the evidence below corrects it.
What Problem Does This PR Solve?
User pain point: DB2 changes can be skipped when an active registration's OLD synchronization point is interpreted as a stop LSN. Saved two-part DB2 schema identities can also be restored in the wrong namespace, and lowercase configuration may not match ordinary uppercase DB2 metadata.
Fix approach: Override change-table discovery inside the DB2 module, honor the serializer's existing namespace argument using the split ID, resolve configured names consistently before catalog/capture use, and fix option forwarding plus resource/optional-metadata handling.
One-sentence summary and example: An active capture with OLD=7 must not discard every committed change at or after 7; its change table now starts at OLD with no synthetic stop, while saved DB2 schema.table history is restored as schema.table.
1. Code Change Review
1.1 Core Logic Analysis
Db2IncrementalSourceFactory resolves options before CatalogTableUtil discovery and per-table override merging. Db2IncrementalSource passes these names to Db2SourceConfigFactory, which strips the database prefix for Debezium table.include.list. Db2Dialect discovers capture-enabled IDs and validates explicit names. The DB2-local connection supplies OLD as start and Lsn.NULL as stop; the existing streaming reader can still assign a successor stop. On restore, JdbcSourceFetchTaskContext derives catalog-first versus schema-first parsing from the authoritative split TableId and passes that into ConnectTableChangeSerializer.
Before:
new Db2ChangeTable(id, capture, objectId, NEW, OLD);
TableId.parse(serializedId); // ignores the supplied namespace flagAfter:
new Db2ChangeTable(id, capture, objectId, OLD, Lsn.NULL);
TableId.parse(serializedId, useCatalogBeforeSchema);Key Findings
- Debezium 1.9.8 Db2Connection maps NEW/OLD to start/stop; its streaming reader skips records at/after an available stop. The override removes that stop interpretation for the returned registrations.
- IBM describes OLD as the capture-start handshake/pruning marker and NEW as advancing when rows are added; see IBMSNAP_REGISTER documentation. This supports distinguishing OLD from a retired-instance stop, but does not prove arbitrary retention-gap recovery.
- DB2-specific SQL remains in Db2CdcConnection; the factory switches only DB2 connections. It passes raw capture names to Debezium's quoting logic.
- MySQL split IDs remain catalog-first; Oracle/PostgreSQL/SQL Server three-part IDs retain their interpretation. The SQL Server schema-change resolver is another direct serializer caller, in addition to shared history restore and Vitess. Its database/schema/table records remain three-part.
- Vitess's existing false argument now becomes effective, matching its FIELD decoder and catalog bootstrap schema-first identity.
- enable_concurrent_read now reaches each reader configuration; false skips snapshot split analysis and returns one full-table split.
- Missing DB2 key metadata warns and returns empty keys; connection close attempts are independent for SQLExceptions, and optional source structs are checked correctly.
- New option resolution preserves database spelling, patterns, primaryKeys and snapshotSplitColumn, but its folded-first precedence and raw quoting have the blockers below.
Complete runtime path
factory options -> DB2 catalog spelling resolution -> CatalogTableUtil / per-table overrides
-> Db2IncrementalSource -> reader config -> Debezium include filter
-> Db2Dialect discovery -> explicit capture validation -> snapshot/stream splits
-> Db2CdcConnection change-table mapping -> Debezium streaming reader -> rows
restore split -> saved table-history bytes -> split-derived namespace flag
-> ConnectTableChangeSerializer -> EmbeddedDatabaseHistory -> restored schema
The capture/start-stop and history-namespace fixes address concrete source defects. The JSON writer and split/offset fields are unchanged. The synthetic capture IT demonstrates the selected mapping against manually inserted change rows, not IBM ASN agent operation, DDL history, pruning or every capture replacement. The name-selection compatibility claim is not yet true: catalog identity and selected capture identity must stay coherent for an existing configuration, including coexistence of distinct case-sensitive objects.
1.2 Compatibility Impact
Partially incompatible. Offsets and checkpoint-writing formats are unchanged, and the shared namespace choice preserves the audited catalog-first/three-part paths while correcting DB2/Vitess schema-first restore. No dependency, connector registration, POM or distribution item is removed. However, Issue 1 shows a previously valid v3.0.0 explicit configuration can fail after upgrade because it is redirected to an uppercase catalog identity. The PR body's unconditional backward-compatibility claim must not remain. The now-effective concurrent-read option is also an intentional behavior correction.
1.3 Performance / Side-Effect Analysis
Name resolution is discovery-time only and caches each distinct configured name. Lowercase names that would change open an additional short-lived catalog and perform existence probes; uppercase/quoted names skip that extra catalog. Resource lifetime is bounded by try-with-resources. Capture discovery retains the previous scan/join shape. No new per-row allocation, executor or retry loop is introduced. A single full-table snapshot with concurrent read disabled can change snapshot duration; it is not a promise to materialize the whole table in heap.
1.4 Error Handling and Logging
Missing explicit capture registrations fail rather than silently disappearing. Catalog existence errors propagate; the new resolver does not silently ignore permission/connection failures. Missing key metadata emits table-ID warnings. Closing a DB2 data connection with SQLException does not prevent attempting metadata connection close. The runtime-specific exception message still comes through existing discovery handling.
Issue 1: Folded-first lookup still changes an existing table identity when both spellings exist
- Location:
seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/db2/utils/TableDiscoveryUtils.java:140-149; Db2IncrementalSourceFactory.java:127-143 - Problem description: The resolver returns the uppercase object as soon as tableExists(folded) is true, without checking the configured object. Concrete case: both e2e.db2inst1.customers and e2e.DB2INST1.CUSTOMERS exist, but only the lowercase object is capture-enabled. In v3.0.0 the factory passed the lowercase string unchanged; Catalog.getTables -> TablePath.of -> DB2Catalog uses case-sensitive TABSCHEMA/TABNAME comparisons and resolves the lowercase object. This head resolves the uppercase catalog object and forwards that spelling into source configuration. Discovery returns the lowercase captured ID, but validateExplicitCaptureTables now compares the uppercase configured ID and rejects the formerly valid job. The new both-exist unit assertion explicitly encodes this changed precedence.
- Potential risk: A production job that previously selected the lowercase object fails on upgrade or restore; table identity is changed before discovery. Merely proving the lowercase-only case does not establish backward compatibility. This is a source-derived failure path, not a locally executed reproduction.
- Best improvement: Preserve an existing exact configured match first and fold only when it does not exist, or fail explicitly on ambiguity rather than silently prefer another object. Keep the final identity consistent in catalog lookup, table overrides and capture validation. Add an end-to-end resolver/factory/discovery regression with both physical names and only the configured one capture-enabled, plus the uppercase-only convenience case. Correct the compatibility claim. Do not treat a quoting instruction alone as a fix.
- Severity: High; blocking.
- Raised by another reviewer: Continuation/correction of the prior Daniel compatibility finding; my earlier folded-first suggestion omitted the coexistence case.
Issue 2: The documented quoted-name escape hatch retains literal quote characters
- Location:
docs/en/connectors/source/DB2-CDC.md: Capture registrations and recovery; TableDiscoveryUtils.normalizeConfiguredTableName/toConfiguredDb2TableId - Problem description: The new guide says quoting a component selects its spelling, but the helper returns the entire quoted string unchanged. Catalog.getTables maps it through TablePath.of, which only splits on dots and does not unquote. DB2Catalog then compares TABNAME with a value containing the literal double quotes, and toConfiguredDb2TableId similarly retains those quotes while capture metadata uses raw names. For an ordinary DB2 object created as "customers", configured e2e.DB2INST1."customers" therefore does not resolve the object named customers. The added test checks string preservation only; it does not exercise this downstream lookup. A single quote also bypasses folding for the unquoted schema component.
- Potential risk: Users following the advertised migration/selection escape hatch fail catalog lookup or explicit capture validation. This raw-quote limitation predates the helper, but the PR newly presents it as the way to avoid its changed name-selection behavior.
- Best improvement: The smallest safe path is to preserve exact raw-name lookup as in Issue 1 and remove/correct the unsupported quoting guidance. If quoted identifier syntax is intended, implement a DB2-local component parser that removes delimiters, handles escaped quotes/dots and produces consistent raw catalog/capture IDs, with actual downstream lookup tests. Do not change the shared TablePath parser for this connector-specific fix.
- Severity: Medium; blocking.
- Raised by another reviewer: Expands the earlier Low whole-name-quote finding using the actual catalog and capture callers; the documented workaround is not valid.
Issue 3: Persist the concurrent-read behavior note in the user guide
- Location:
docs/en/connectors/source/DB2-CDC.md and docs/zh/connectors/source/DB2-CDC.md - Problem description: The PR description now correctly explains enable_concurrent_read=false taking effect, but the changed user guides still omit that upgrade-visible behavior.
- Potential risk: Users reading released connector documentation can miss why a fresh snapshot now uses one full-table split, despite an unchanged option value.
- Best improvement: Add the concise behavior/migration note to both user guides or incompatible-change notes: false now applies, default true is unchanged, existing generated splits are unaffected, and true retains prior concurrent splitting.
- Severity: Low; non-blocking.
- Raised by another reviewer: Continuation of the previous Daniel Issue 3; the PR description is improved but the user-documentation request remains.
2. Code Quality Assessment
2.1 Coding Standards
Nontrivial new methods explain namespace choice, resolver intent, cached per-name resolution, capture mapping and connection cleanup. Existing URL notBlank validation and its tests survive the upstream import conflict. The required changes can stay in the DB2 name boundary; the shared serializer need not grow DB2-specific branches.
2.2 Test Coverage and Test Stability
Test stability: Stable. Unit tests use local mocks, scoped static mocks and deterministic history fixtures; history registry entries are removed in finally. They assert namespace components separately because TableId equality can hide catalog/schema swaps. The fixed Vitess JSON fixture is independent of the current writer but is source-derived, not captured from an old running binary. DB2 ITs use the pinned existing DB2 image/mapped host port, bounded reconciliation and savepoint waits, deterministic distinct LSNs, stopped-job writes and source/sink row equality. The new terminal streaming jobs rely on the existing per-invocation container teardown; no new claim of standalone future cancellation is made. The lowercase IT targets ordinary uppercase objects with lowercase config, not quoted lowercase objects or the both-exist blocker. These coverage gaps are correctness gaps described above, not a demonstrated timing flake. No local compilation, tests or containers were run.
Local analysis: fetched the head, created independent branch seatunnel-review-12970-r2, read the complete 22-file diff and traced affected callers with git show/git grep. No local compilation, unit tests, integration tests, E2E, containers, or job execution was performed, as requested. These are source-analysis conclusions; execution evidence is the current-head CI below.
2.3 Documentation Updates
EN/ZH capture/recovery notes are present. They must be corrected for Issues 1-2, and should retain the concurrent-read note in a released document. The PR candidly limits its ASN fixture and Vitess history evidence; those limitations remain material.
3. Architectural Soundness
3.1 Elegance of the Solution
Precise fix. The capture mapping belongs in the DB2 JDBC adapter, and the generic serializer should honor its existing namespace parameter derived from authoritative split identity. Those are the correct boundaries. The new name resolver is also DB2-local, but its compatibility policy needs correction. The smallest safe fix is exact-match-first resolution and honest syntax documentation; a complete quoted-identifier feature needs a DB2-local parser and wider tests, not a shared catalog rewrite.
3.2 Maintainability
Keep catalog lookup, per-table configuration and capture matching on one resolved identity. Avoid separate folding rules in validation that can appear to accept an identity different from the one actually used. The shared restore change needs the existing MySQL, three-part and Vitess tests retained.
3.3 Extensibility
Additional DB2 identifier syntax and real ASN lifecycle support need explicit source-to-capture identity tests. Future serializer users must choose catalog-first/schema-first intentionally; combined rendered-name equality is insufficient to validate namespace preservation.
3.4 Historical-Version Compatibility
I checked v3.0.0 factory and DB2 catalog source for the existing raw-spelling contract. The current fallback only preserves it when the uppercase counterpart is absent, so the compatibility blocker remains. Saved history writing is unchanged, but no current-head live old-binary upgrade matrix or real IBM ASN retention/DDL validation is claimed.
4. Issue Summary
| Number | Issue | Location | Severity |
|---|---|---|---|
| 1 | Folded-first lookup still changes an existing table identity when both spellings exist | seatunnel-connectors-v2/connector-cdc/connector-cdc-db2/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/db2/utils/TableDiscoveryUtils.java:140-149; Db2IncrementalSourceFactory.java:127-143 |
High |
| 2 | The documented quoted-name escape hatch retains literal quote characters | docs/en/connectors/source/DB2-CDC.md: Capture registrations and recovery; TableDiscoveryUtils.normalizeConfiguredTableName/toConfiguredDb2TableId |
Medium |
| 3 | Persist the concurrent-read behavior note in the user guide | docs/en/connectors/source/DB2-CDC.md and docs/zh/connectors/source/DB2-CDC.md |
Low |
5. Merge Recommendation
Conclusion: Ready to merge after fixes
- Blockers — must be fixed
- Issue 1: Folded-first lookup still changes an existing table identity when both spellings exist; Issue 2: The documented quoted-name escape hatch retains literal quote characters Fix Issues 1 and 2, then obtain green current-head CI including the affected DB2/shared-history/Vitess suites and an independent committer review. This self-review supplies no independent approval.
- Recommended fixes — non-blocking
- Issue 3: Persist the concurrent-read behavior note in the user guide
Overall assessment: The capture and restore fixes remain well located, but the new fallback does not fully preserve table-selection semantics. The quoted-name guidance cannot be used to close that gap.
Existing-review reconciliation: There is no independent formal reviewer decision in the refreshed history. The earlier compile API, database-component case, Vitess fixture and missing-key warning requests remain resolved. The lowercase-only fallback test is useful but does not close the original compatibility finding. I explicitly retract the sufficiency of my earlier folded-first suggestion and the assumption that quoting was a working escape hatch. The option-forwarding change is now explained in the PR body; Issue 3 asks that the same information survive in the user docs. Author replies have been incorporated into this single full review.
Head 74a90d2ae7fd1ee8f90378b24b03528e012796cc. Live base dev at 3ac2d1cec6aa7b3741c4dacdd23020f778e23c02; comparison ahead, ahead 4 / behind 0; merge state BLOCKED. Behind/diverged alone is recorded metadata, not a request to synchronize upstream. Checks: No current-head checks reported.
The apache PR currently reports no head checks. The exact-head fork Build 41162579586 is QUEUED, attempt 1. Prior-head Build success and its DB2 four-test result do not validate 74a90d2. There is no executed current-head failure to diagnose or retry, and this head contains the fetched dev tip; no upstream-sync action is warranted. No rerun was triggered.
Daniel has READ repository permission. This COMMENT records the technical conclusion; it is not an effective maintainer approval or a dismissal of any historical requested-changes entry. A write-capable maintainer owns the final repository approval gate.
Purpose of this pull request
Yes, I am contributing these DB2 CDC correctness fixes to Apache SeaTunnel.
This is a focused follow-up to the DB2 source introduced in #10780 (feature issue #10735) and the CDC history restoration introduced in #7463.
An active ASN Capture registration can contain both
CD_OLD_SYNCHPOINTand an advancingCD_NEW_SYNCHPOINT. Debezium 1.9.8 maps those fields to a change table's stop/start LSNs, so SeaTunnel's reader skips committed changes at or beyond the old synchronization point. The connector now maps the raw registration at its JDBC boundary: OLD is the start, and an active registration has no stop. The existing streaming reader can still assign a stop when replacing an older capture instance. Reading raw capture names also avoids quoting mixed-case names twice; no third-party class is replaced.Saved DB2 history encodes two-part
schema.tableidentifiers. The shared serializer previously ignored its existinguseCatalogBeforeSchemaargument and restored them ascatalog.table. The serializer now honors that argument, and the JDBC restoration entry point derives it from the authoritative split table ID, preserving MySQL's catalog-first history. This also corrects Vitess checkpoint restoration: its existingfalseargument previously had no effect, so two-part history was restored ascatalog.table; it now restores asschema.table, matching the actual decoder and catalog bootstrap. The JSON writer is unchanged.The DB2-specific follow-up also normalizes only unquoted schema/table components before catalog lookup/filtering while retaining the configured database spelling, preserves custom keys and
snapshotSplitColumn, forwards the existingenable_concurrent_readsetting, warns with the table ID when catalog key metadata is absent, handles absent source structs, and closes the auxiliary connection even if closing the primary connection fails. The name normalization is backward compatible: the uppercase spelling is used when such an object exists, and the configured spelling is kept when only it exists (TableDiscoveryUtils.resolveConfiguredTableName), so an object created with quoted lowercase identifiers that 3.0.0 matched unquoted keeps matching after the upgrade. Startup validation accepts either spelling for the same reason.Does this PR introduce any user-facing change?
Yes. Active DB2 captures continue producing changes after their synchronization points advance, and saved schema history is restored into the correct DB2 namespace. Lowercase unquoted schema/table configuration matches DB2's uppercase metadata without changing the database component of SeaTunnel table identities; configuration that matched a lowercase-created object in 3.0.0 keeps matching it. Vitess restores existing checkpoint history into the schema-first namespace used by its decoder.
Behavior change:
enable_concurrent_read = falsenow takes effect for DB2 CDC. Before this PR the option was accepted but not forwarded, so such jobs silently ran concurrent snapshot splitting; after the upgrade a new snapshot phase for those jobs uses a single full-table split, which can change snapshot duration and memory on large tables. Jobs that leave the option at its default (true) and savepoint restores of already generated splits are unaffected. Set the option totrueto keep the previous snapshot behavior.Existing option names, aliases, defaults,
table-patternexpressions, quoted spelling, public APIs, dependency versions, offset/state fields and checkpoint writing formats are retained. The Apache snapshot backfill implementation andexactly_oncesetting are retained. No POM, connector registration, distribution, workflow or unrelated connector implementation is changed.How was this patch tested?
Added 17 unit regression cases across DB2 and Vitess, plus three DB2 container scenarios alongside the original test:
TableDiscoveryUtilsTest.shouldKeepCapturingLowercaseCreatedObjectConfiguredUnquotedandDb2IncrementalSourceFactoryTest.shouldKeepConfiguredSpellingWhenOnlyLowercaseCreatedObjectExistsfail when an unquoted lowercase configuration for a table that exists only in lowercase is folded to uppercase or rejected by capture validation.Local validation is intentionally limited to formatting under the contributor's Apache SeaTunnel validation policy:
Compilation, unit tests, packaging and DB2 E2E were not run locally; they require GitHub CI on the current PR head. The previous-head Build run failed during compilation because this port called the Whale-only
ReadonlyConfig.getConfData()API. The follow-up uses Apache'sgetSourceMap()and preserves raw nested configuration values. The previous static-check statement missed this error and has been removed. No passing runtime result is claimed; the current head requires a new CI result.No existing build/deployment artifacts were reused, and no service or Docker container was started.
seatunnel-engine-uiwas excluded because this change does not touch it.The DB2 E2E uses a synthetic ASNCDC catalog; it does not validate the real IBM ASN Capture agent or IBM DDL-history replay. The existing original assertions and DependencyJar driver setup are preserved.
Check list