[PR] Reject unresolved data sources before creating CDC jobs [shardingsphere]

["terrymanu (via GitHub)" <gi...@apache.org>]

terrymanu opened a new pull request, #39572:
URL: https://github.com/apache/shardingsphere/pull/39572

   (no comment)


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Reject unresolved data sources before creating CDC jobs [shardingsphere]

["terrymanu (via GitHub)" <gi...@apache.org>]

terrymanu commented on PR #39572:
URL: https://github.com/apache/shardingsphere/pull/39572#issuecomment-5384215591

   ### Result
   
   **Review Result: Mergeable**
   
   The change rejects CDC job creation when a routed data node references a data source absent from the job's data source configuration, before any governance-center mutation. It follows the maintained `MigrationJobAPI` validation precedent, propagates to the CDC client through the existing `PipelineSQLException` mapping, and is covered by focused tests.
   
   ### Evidence
   
   - Root cause and fix placement: `kernel/data-pipeline/scenario/cdc/core/src/main/java/org/apache/shardingsphere/data/pipeline/cdc/api/CDCJobAPI.java:137` calls `checkDataSources` before `getJobFacade().getJob().create(...)` and `getConfiguration().persist(...)`. Without the check, an incremental job failed in `initIncrementalPosition` via `ShardingSpherePipelineDataSourceConfiguration.getActualDataSourceConfiguration` (Guava `checkNotNull` on the missing name) only after the job was already registered, and a full job was created broken and failed later at prepare/start — both leaving orphaned registry state.
   - Divergence path is real: `kernel/data-pipeline/scenario/cdc/core/src/main/java/org/apache/shardingsphere/data/pipeline/cdc/handler/CDCBackendHandler.java:120-121` builds `tableAndDataNodesMap` from rule routing, while the job data sources are built from the database's registered storage units; a rule referencing an unresolved resource produces data nodes absent from `getStorageUnits()`.
   - Null-safety traced: `CDCJobConfiguration.dataSourceConfig` is the concrete `ShardingSpherePipelineDataSourceConfiguration` whose `rootConfig` is a final field populated by the create-time YAML round trip; `YamlCDCJobConfigurationSwapper.swapToObject` never returns a null data-node list, and empty lists are already rejected earlier by the `jobShardingCount` check at `CDCJobAPI.java:132`. `jobShardingDataNodes` is derived from the same map as `tablesFirstDataNodes` and covers a superset of its data nodes.
   - Precedent: `kernel/data-pipeline/scenario/migration/core/src/main/java/org/apache/shardingsphere/data/pipeline/scenario/migration/api/MigrationJobAPI.java:137` validates data source names with the same `ShardingSpherePreconditions.checkContainsKey` + `PipelineInvalidParameterException` pattern.
   - External contract: `PipelineInvalidParameterException` extends `PipelineSQLException` with `XOpenSQLState.INVALID_PARAMETER_VALUE`; `proxy/frontend/core/src/main/java/org/apache/shardingsphere/proxy/frontend/netty/CDCChannelInboundHandler.java:182-184` catches it and wraps it into a CDC protocol error response, so the client receives a proper rejection.
   - Tests: `kernel/data-pipeline/scenario/cdc/core/src/test/java/org/apache/shardingsphere/data/pipeline/cdc/api/CDCJobAPITest.java:216-231` builds a real `ShardingSpherePipelineDataSourceConfiguration` with data nodes on `bar_ds` (present) and `foo_readwrite_ds` (absent), asserts the exception message names the offending data source, and verifies no job create/persist. `CDCJobAPITest.java:198-214` pins that an already-existing job still skips validation and mutation. Existing scenarios delegate through the new `createJobConfiguration(List, boolean)` overload with resolvable data sources.
   - Local verification: `./mvnw -pl kernel/data-pipeline/scenario/cdc/core -DskipITs -Dspotless.skip=true -Dtest=CDCJobAPITest -Dsurefire.failIfNoSpecifiedTests=false test` — exit 0, `Tests run: 10, Failures: 0, Errors: 0`.
   
   ### Coverage
   
   - Reviewed head `aadef5f2dc396c1582aa997ac62d43ff46400cae` (re-verified unchanged after review; 1 commit), base `master` at `eeb072be8f4`; local `HEAD` equals the public head and the local triple-dot file list matches the GitHub changed-file list exactly (2 files). PR has an empty description, no linked issue, and no review comments.
   - Behavior clusters: (A) pre-creation data source validation in `CDCJobAPI.create` — fully traced end to end including callers, exception mapping, and old-scenario regression; (B) test updates in `CDCJobAPITest` — validated against real production behavior. No churn-only files.
   - Discovery lenses completed (root cause/behavior, blast radius/contracts, tests/runtime/operations); candidates examined and disproven: NPE risk on `getRootConfig`, empty/null data-node lists, uncovered `tablesFirstDataNodes`, check skipped for existing jobs, and exception escaping the CDC protocol. Zero findings passed the Finding Proof Gate.
   - Unresolved gaps: none. Note: the local working tree contains an uncommitted, user-owned refactor of `checkDataSources` (nested loops instead of streams, semantically identical); the focused test run compiled that version — the committed head version is behaviorally equivalent.
   - GitHub access route: `GITHUB_TOKEN` via authenticated REST API (value not exposed). CI was not reviewed; this result is code-scope only.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Reject unresolved data sources before creating CDC jobs [shardingsphere]

["terrymanu (via GitHub)" <gi...@apache.org>]

terrymanu merged PR #39572:
URL: https://github.com/apache/shardingsphere/pull/39572


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org