SEPURI-SAI-KRISHNA commented on PR #11721: URL: https://github.com/apache/seatunnel/pull/11721#issuecomment-5327840001
Thanks for re-reviewing at `7cfabc8` rather than assuming the merge was a no-op — you were right not to reuse the blob-hash shortcut this round, since `dev` did independently touch `MultiTableSinkWriter.java` via the Dead Letter Queue work (#10306). On your **Issue 2 (CI status)** — the run has since produced a result, and I pulled the log rather than guessing at it. **It is not a test failure.** `unit-test (8, ubuntu-latest)`, job `95601487349`, run `32100301798`. The whole reactor has exactly one `FAILURE` line: ``` [INFO] SeaTunnel : Connectors V2 : Clickhouse ............. FAILURE [ 1.351 s] ... [ERROR] Failed to execute goal on project connector-clickhouse: Could not resolve dependencies for project org.apache.seatunnel:connector-clickhouse:jar:3.0.0-SNAPSHOT: Could not transfer artifact com.clickhouse:clickhouse-client:jar:0.3.2-patch11 from/to central (https://repo.maven.apache.org/maven2): transfer failed for https://repo.maven.apache.org/maven2/com/clickhouse/clickhouse-client/0.3.2-patch11/clickhouse-client-0.3.2-patch11.jar: Connection reset -> [Help 1] ``` A Maven Central `Connection reset` during dependency download, 1.35s into a module this PR does not touch. There is not a single `Failures: [1-9]` or `Errors: [1-9]` anywhere in the 18,544-line log. More usefully for this PR specifically — the build got well past `seatunnel-api` before it died, so the change here **did** get exercised on JDK 8: ``` [INFO] Running org.apache.seatunnel.api.sink.multitablesink.MultiTableSinkWriterTest [INFO] Tests run: 28, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.09 s - in org.apache.seatunnel.api.sink.multitablesink.MultiTableSinkWriterTest ``` 28 is the full post-merge count: `dev`'s 26 (including the DLQ tests and `testMultiTableStateDeserializesPreFailedTablesCheckpoint`) plus the two added here. So `testMinValuePrimaryKeyHashRoutesToValidQueue` and `testOrdinaryNegativeHashRoutesToMaskedIndex` both ran and passed on JDK 8 / ubuntu, against the merged `write()` that the DLQ PR restructured. That is the signal your Issue 2 was asking for, and it is green; the job's red status comes from a module three quarters of the way further down the reactor. The other three `unit-test` legs show `cancelled` for the reason you identified on #11724 — matrix fail-fast, not three more problems. **On the fail-fast masking itself.** I said on #11724 that I would file this, and I now have the concrete evidence rather than a suspicion. `.github/workflows/backend.yml:480-483`: ```yaml unit-test: strategy: matrix: java: [ '8', '11' ] os: [ 'ubuntu-latest', 'windows-latest' ] ``` No `fail-fast: false` — while `benchmark-test`, 26 lines below at `:511-512`, sets it explicitly. So the omission reads as an oversight rather than a deliberate choice, and it is a one-line fix. Today's two runs are a clean illustration of the cost: on this PR one infrastructure flake in `connector-clickhouse` cancelled three legs that had every reason to be green, and on #11724 one Windows Hazelcast bootstrap flake cancelled the other three. In both cases the platform-specific information a 2x2 matrix exists to produce was thrown away because of a failure that had nothing to do with the platform. I will open that as a separate infra issue rather than widen this PR. **On your Issue 1 (#11725).** Still open, still unchanged since 2026-08-10, still `CHANGES_REQUESTED` with a red build. I agree that closing it as superseded is the right outcome and equally that neither of us should be the one to make that call — it needs a committer. Flagging it here so it is on the record at merge time. Requesting a rerun of the failed legs to get a clean board. -- 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]
