SEZ9 commented on PR #12568: URL: https://github.com/apache/seatunnel/pull/12568#issuecomment-5945592143
Thanks @SEPURI-SAI-KRISHNA for the detailed CI breakdown on `c35c6d7a0`, that saves a lot of digging. Your triage of run `36728523674` matches what I'd expect: - `SavePointTest.testSavePointButJobGoingToFail` in `seatunnel-engine-server` is untouched by this change, and the comparison against nightly `36025021304` (same `expected: <RUNNING> but was: <FAILED>`, nearly identical timing) is convincing enough that I'm treating it as pre-existing flakiness. The grep for `multi_table_sink_replica` / `must be at least 1` in that job log is a nice extra confirmation that the new guard never fired there. - `TestFilterRowKindIT.testFilterRowKindMultiTable[3]` is already tracked by the issue you linked, so not blocking here. - The HTTP 429 in `all-connectors-it-4 (8)` is pure infrastructure; nothing ran, so there's nothing to evaluate. On the change itself: validating `replicaNum < 1` in the `MultiTableSink` constructor right after the option is read is the right spot, since it's the single constructor and covers both `createWriter` and `restoreWriter`. Including the option key and rejected value in the message is exactly what operators were missing before. The 3 new tests in `MultiTableSinkWriterTest` (31 total, all green on JDK 8) cover the boundary well. Remaining asks before I merge: 1. Please rebase on latest `dev` (or push an empty commit) to re-trigger CI so we get a run where `all-connectors-it-4` actually executes. I don't want to merge on a run where a whole leg aborted before any test ran, even if the cause is unrelated. 2. The review above raised the `queueSize * 2` overflow for very large values as a separate concern. I agree it's out of scope for this PR, but could you confirm in the description that an upper bound is intentionally left out, so nobody later assumes it was overlooked? Once CI comes back with the IT leg completed, I'll approve and merge. <!-- streview-comment:1465 --> -- 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]
