SEZ9 commented on PR #12299: URL: https://github.com/apache/seatunnel/pull/12299#issuecomment-5693943709
## CI status on the current head (`9531cecd4`) Rebased onto `75b60fa14` — the branch content is unchanged, I only replayed it onto a newer `dev` to pick up `[Fix][E2E] Unify testcontainers version to 1.21.4` (#11201) and `[Improve][E2E] Reuse SeaTunnel container for selected test classes` (#11626), on the chance they affected the e2e container failures below. I diffed the patch before and after the replay: byte-identical except for two `@@` hunk offsets in `incompatible-changes.md` (en 308→321, zh 272→283), where other merged PRs had added entries above mine. **What this PR's own changes do in CI — all green:** | | Linux JDK8 | Windows JDK8 | |---|---|---| | `FileUtilsTest` | 14 run, 0 failed | 14 run, 0 failed | | `LogContentReaderTest` | 3 run, 0 failed | 3 run, 0 failed | | `YamlSeaTunnelConfigParserTest` | 4 run, 0 failed | 4 run, 0 failed | `unit-test` is green on all four legs (JDK 8/11 × ubuntu/windows). The Windows legs are the ones I most wanted to see, since two of the review items were specifically about platform dependence — decoding UTF-8 rather than the platform default charset, and emitting a literal `\n` in the truncation notice rather than `%n`. Those now have real cross-platform evidence rather than my reasoning about them. `engine-v2-it`'s `RestApiIT` is also green on both JDKs (22 run, 0 failed, 0 skipped), so the shared `LogContentReader` path did not disturb the existing REST endpoint behaviour. This is also the first real compile of the change — as I said in my previous comment, I have no JDK locally and verified the algorithm by porting it to Python. The JVM run now agrees with that port, including the regression case that returned `""` before this fix. **What is still red, and why I stopped rerunning:** After four attempts the failure set stopped changing: | Job | Failing test | Error | |---|---|---| | `all-connectors-it-2` (8 and 11) | `OpengaussCDCIT.testAddFieldWithRestore` | `ConditionTimeout` at `OpengaussCDCIT.java:476` — `actual iterable was <null> at index [1][4] within 1 minutes` | | `engine-v2-it` (8) | `SplitClusterFaultToleranceIT.testStreamJobCancelResolvesWhenWorkerCrashesBeforeCancelAck` | `ConditionTimeout` at `assertEventuallyCanceled:556` | Neither is reachable from this diff, which only touches log-content reading in the REST layer plus one new config option. Supporting evidence rather than just the assertion: - `OpengaussCDCIT.testAddFieldWithRestore` fails identically on **#12298** as well — an unrelated Zeta PR that touches neither CDC nor the log endpoints — on both JDKs, before *and* after the rebase, across every attempt on both branches. It has never passed once in this fork's runs. - `SplitClusterFaultToleranceIT` and `BackpressureSlowSinkIT` (which failed on the JDK11 leg in an earlier attempt) are both recent additions to `dev`: #12030 on 2026-09-09 and #12107 on 2026-09-10. In earlier attempts the JDK8 and JDK11 legs of `engine-v2-it` failed on *different* tests of this pair, which is the signature of flakiness rather than of a defect this PR introduced. I have stopped issuing reruns: the failure set is now deterministic, and further attempts only consume the fork's runner quota — each full run takes 7–9 hours here, most of it queueing. Happy to rerun on request, or to rebase again once these are addressed on `dev`. @DanielLeens — `9531cecd4` is the head to re-review, and the previous review round's items are all in it. -- 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]
