davidzollo commented on PR #11077:
URL: https://github.com/apache/seatunnel/pull/11077#issuecomment-5509637252
CI diagnosis and status update on the current head.
**Build failure diagnosis (head `4e573be3`):** the apache-side `Build` check
dereferences to the fork's own Actions run (`hesam-oxe/seatunnel` run
`33582179112`). Out of ~90 jobs in that run, exactly one failed:
`all-connectors-it-1 (11, ubuntu-latest)`, with a single test error:
```
HdfsFileIT.testHdfsBinaryUpdateModeContinuousDiscoveryPostSyncDelete
java.lang.RuntimeException: Wait continuous job exit failed.
at HdfsFileIT.cancelContinuousJob(HdfsFileIT.java:651)
Caused by: java.util.concurrent.TimeoutException
at HdfsFileIT.cancelContinuousJob(HdfsFileIT.java:649)
```
`HdfsFileIT.java` is not touched anywhere in this PR's diff (this PR only
changes a *unit* test in the sibling `connector-file-hadoop` module,
`HdfsFileSinkTest.java`), and every other job in the run — including every job
that exercises
`MultiTableSink`/`MultiTableSinkWriter`/`MultiTableSinkCommitter`/`MultiTableSinkAggregatedCommitter`
— passed. This is an unrelated E2E timing flake (a continuous streaming job
not exiting within the HDFS mini-cluster test's cancellation timeout under CI
load), not a regression introduced by the multi-table-sink writer-sharing
changes.
**Action taken:** I don't have admin rights on `hesam-oxe/seatunnel` (only
maintainer-edit push access to this branch), so GitHub's job-level and
run-level rerun (`gh run rerun --job` / `--failed`) are both refused with "Must
have admin rights to Repository." Since a direct rerun isn't available to me, I
pushed an empty commit (`b3435c67e`, "[Chore] Retrigger CI after unrelated
HdfsFileIT flake") to get a fresh `Build` run, the same pattern already used a
few times earlier in this branch's history. A new run is in progress now. No
production or test code changed.
**Review status, independently re-verified against the current source (not
just re-reading prior review text):**
- The two commits unrelated to this PR's stated purpose that a couple of my
recent reviews flagged (a Zeta deployment-hang fix and a Doris E2E assertion
change) are already gone from the branch — confirmed via `git log dev..HEAD`,
they're simply not there anymore.
- @SEZ9's two High-severity carryover findings are both fixed in the code as
it stands right now, and I read the live source directly to confirm both:
- Destination-key collision (`PR11077-F1`): `DestinationKey.equals()` in
`MultiTableSink.java` requires `sink.getClass().equals(that.sink.getClass())`
whenever an identifier is present, and falls back to raw object identity (`sink
== that.sink`) otherwise — two different connector types (or a non-opted-in
sink) can no longer collide onto one writer.
- Snapshot/restore state duplication (`PR11077-F2`):
`MultiTableSinkWriter.snapshotState()` stores a shared writer's state under a
single canonical identifier (`aliasedIdentifiers.get(0)`) only, not fanned out
to every alias — so a checkpoint taken by the current code restores exactly one
copy of state, not N duplicates.
- @SEZ9's review state on GitHub is currently `COMMENTED`, not
`CHANGES_REQUESTED`, so there is no active change-request blocking this PR
right now.
So, as things stand: the multi-table-sink writer-sharing logic itself has no
open correctness issues that I could find, the previously-flagged blockers are
resolved in code, and the only thing left is CI going green on the fresh run
just triggered.
@hesam-oxe — really solid work getting this fix through so many rounds of
deep concurrency/checkpoint-correctness review as a first PR here. Once CI
comes back clean, this should be in good shape for a maintainer to take a final
look and merge.
--
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]
