DanielLeens commented on PR #11206:
URL: https://github.com/apache/seatunnel/pull/11206#issuecomment-6624346107
Thanks @SEZ9. I checked each point against `c3d11fc6475`. One of them (the
F2 unit test) needs a code change. The others are already on this head or need
a clarification only.
**F1.** The comment that explains why the two names must differ is at
`AbstractMysqlCDCITBase.java:1313-1316`, and it stays.
**F3, option names and defaults.** These are two separate options, both
defined in `MySqlIncrementalSourceOptions.java:61-82`:
- `scan.binlog.newly-added-table.enabled` (default **false**) is the new
runtime feature in this PR: CREATE TABLE seen while reading binlog. The runtime
path is off unless the user enables it.
- `scan.newly-added-table.enabled` (default **true**) is evaluated only on
checkpoint/savepoint restore. It decides whether tables that newly match the
pattern join the snapshot phase. `true` is not a new behavior. In the released
`v3.0.0`, `IncrementalSource#restore` adds `Sets.difference(capturedTables,
checkpointCapturedTables)` to the remaining tables unconditionally
(`IncrementalSource.java:435-438` at `v3.0.0`). This PR only makes that
behavior switchable (`IncrementalSource.java:435-437` on this head). A `false`
default would silently change restore behavior for existing jobs, so I'm
keeping `true`.
`docs/en/connectors/source/MySQL-CDC.md:195-196,233-235` (zh mirrored) state
both names and defaults the same way as the option definitions.
**F6, old checkpoint without runtime tables.**
`MultiTableSinkWriterTest#testMultiTableStateDeserializesPreFailedTablesCheckpoint`
(`:1323-1340`) deserializes a pre-PR `MultiTableState` byte snapshot that has
only `states`. It asserts empty `states`, `failedTables` and
`runtimeCreatedTables`. On the source side, restore still goes through the
unchanged `IncrementalSource#restore` path above. To be precise: this is a
state-compatibility unit test, not an E2E restore from a savepoint written by a
previous release.
**F7, JDBC sink docs.** These are already there:
`docs/en/connectors/sink/Jdbc.md:63` (zh `:63`) requires `is_exactly_once =
false` and `generate_sink_sql = true`, with no custom `query`, for
runtime-created tables.
**F8, digit-leading names.** The allowlist you quoted is the old one.
Production is `[A-Za-z0-9_$]*[A-Za-z_$][A-Za-z0-9_$]*`
(`MySqlCatalogTableUtils.java:47-48`), which accepts `2024_orders` and rejects
all-numeric names. `docs/en/connectors/source/MySQL-CDC.md:241` (zh `:240`)
states this explicitly.
**F2, sink that has not opted in.** The contract is on
`SupportMultiTableSinkWriter#supportsNewlyCreatedTable`
(`SupportMultiTableSinkWriter.java:37-46`): a runtime `CreateTableEvent` is
never silently dropped, and the job fails if the sink has not opted in.
`MultiTableSinkWriter#hasSourceMatchedWriter` enforces it before any writer is
created or a row is routed: it throws `IOException("Sink %s does not support
runtime newly created table %s")` (`MultiTableSinkWriter.java:512-519`,
template path `:520-531`). The gap is the test. The existing tests cover a
factory-returned writer that rejects capability
(`testRuntimeRegistrationRetryClosesUninstalledWriter`), but not a sink that
never opted in. **Owed:** one `MultiTableSinkWriterTest` case where
`supportsNewlyCreatedTable()` is false, asserting the exact `IOException`
naming the table, that the factory is never invoked, and that no row reaches a
writer.
**F4, CREATE scoping.**
- Source: a runtime CREATE becomes a `CatalogTable` only if the table passes
the configured database/table pattern filter
(`MySqlIncrementalSource.java:293-302`, `dataCollectionFilter().isIncluded`).
CREATE records are accepted only when `scan.binlog.newly-added-table.enabled =
true` (`MySqlSchemaChangeResolver.java:65-72`).
- Sink: `hasSourceMatchedWriter` (`MultiTableSinkWriter.java:502-507`)
rejects an event whose path does not match its catalog table before the factory
runs (`testRuntimeCreateRejectsMismatchedIdentityBeforeFactory`, `:172`). The
physical target is resolved from the configured sink template; source DDL text
is never executed on the target.
**F5, writer creation throws.**
- `FAIL_FAST`: the `IOException` propagates after one attempt and fails the
task.
- `CONTINUE_OTHER_TABLES`: creation is retried up to the table retry budget
before any row of that table is processed. If the retries are exhausted, the
table is quarantined, recorded in `MultiTableState.failedTables`, and none of
its rows reach a writer, while the other tables keep writing.
`testRuntimeRegistrationFailurePolicy` (`:214-280`) covers both policies.
`testRuntimeTableRegistrationRetriesWriterCreation` (`:324-367`) covers
transient failure and then success, and checks that a replayed CREATE reuses
the registered writer.
Follow-up commit: the F2 unit test above, test only. No production change is
planned for F3–F8.
--
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]
