DanielLeens commented on PR #12391: URL: https://github.com/apache/seatunnel/pull/12391#issuecomment-5846383487
@SeaSand1024 @SEZ9 one correction to my last comment before we call this ready to merge. I went back through fork run `35870424836` job by job instead of relying on job names alone (it's the same run that confirmed `all-connectors-it-2` green), and the apache-side `Build` check is still reporting `FAILURE` on this head for a reason beyond what we've covered so far. Two more jobs in that same run are red and neither of us had flagged them yet: - `all-connectors-it-5 (8, ubuntu-latest)`: https://github.com/SeaSand1024/seatunnel/actions/runs/35870424836/job/107213895383 — `OceanBaseCDCCompatibilityIT.testOceanBaseCdcWrapperRuntimeE2e` (2 errors), failing with `ProgramInvocationException: The main method caused an error: Flink job executed failed`. - `transform-v2-it-part-1 (11, ubuntu-latest)`: https://github.com/SeaSand1024/seatunnel/actions/runs/35870424836/job/107213895413 — `TestFilterRowKindIT.testFilterRowKindMultiTable`, `expected: <0> but was: <1>`. The second one I can close out the same way we closed the Opengauss job: it's a known, already-filed flake. apache/seatunnel#12116 describes exactly this test failing intermittently only on Flink legs, because `AssertSinkWriter` evaluates static JVM-wide row counters per subtask `close()`; it's already reproduced the same wrong-row-count symptom on two unrelated PRs (#11458, #11077). Nothing in this PR's diff or in the Zeta engine touches `AssertSinkWriter`, so I read this as the same category of pre-existing noise. The OceanBase one I can't close the same way yet. I didn't find a pre-filed issue for this exact test/symptom, and I only have this one occurrence to go on. Both failing tests run against the Flink translation layer rather than the Zeta engine `SeaTunnelTask.close()` path this PR touches, so mechanically I don't see how this diff could cause either — but "mechanically unrelated" isn't the same as "confirmed pre-existing," so I'd rather not wave it away without another data point. @SeaSand1024, could you re-run just `all-connectors-it-5 (8, ubuntu-latest)` and post the result? Green on a re-run would put it in the same bucket as the other testcontainers-level flakes we already tracked down; a repeat failure with the same error would mean we should look at it properly before merging. For completeness: `engine-v2-it (8, ubuntu-latest)` in this same run is still `SplitClusterFaultToleranceIT.testStreamJobCancelResolvesWhenWorkerCrashesBeforeCancelAck`, the same #12353/#12311 symptom we've already attributed — nothing new there, just re-confirmed on this exact head. So: the code side is still ready, and most of the CI picture still stands, but I'd like to hold off on "ready to merge" specifically until the `all-connectors-it-5` re-run is in — one more real data point is worth the short wait. === END REPLY === -- 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]
