zhangshenghang opened a new issue, #12562: URL: https://github.com/apache/seatunnel/issues/12562
Follow-up tracking for the two pre-existing issues (F2 and F5) identified by @SEZ9 during the review of #12381, which the PR deliberately leaves untouched (they are not introduced by the diff, and the reviewers agreed to land the `AWAIT_BUDGET_SECONDS` ceiling widening on its own). ## F2 — ordering race in `rediscoversFileAfterInactiveCursorClosed` The test's choreography is driven purely by wall-clock conditions. `FileCollectReader.poll()` runs `discoverNewFiles` *before* `closeInactiveCursors`, and the two `Awaitility` awaits exit on time alone (`now >= idleDeadlineMs [+ GLOB_SCAN_INTERVAL_MS]`), never on reader state. If the poll that closes the idle cursor lands late (a ~45ms gap with no poll is routine on loaded Windows runners), the subsequent awaits and the pre-append `poll(10)` can all skip discovery (`now - lastDiscoveryMs < 20`), so `"second"` gets appended before the file is re-discovered. The re-discovered cursor then opens with `READ_FROM_BEGINNING=false` and seeks to `Files.size(file)` — past the appended line — and the final `untilAsserted` can never observe it, at any ceiling. **Suggested fix (from the review):** make the choreography state-driven rather than time-driven — e.g. expose a package-private `isTracking(Path)` on `FileCollectReader` and await `!isTracking(logFile)` then `isTracking(logFile)` before appending `"second"`; or replace the wall-clock awaits with an explicit `sleep(CLOSE_INACTIVE_MS + 30); poll(); sleep(GLOB_SCAN_INTERVAL_MS + 5); poll();` sequence, which a stall can delay but cannot reorder. ## F5 — final `untilAsserted` consumes events on every poll, masking the real observation `reader.poll(10)` drains events, so once a poll returns something unexpected (two events, a stale `first`, etc.) every later iteration sees an empty list and the assertion fails with `expected: <1> but was: <0>` until the budget elapses. Awaitility reports the *last* error, so the informative first failure is lost and CI logs are hard to act on. Note the widened ceiling in #12381 makes this masked failure slower to surface (up to 10s instead of 3s) — a deliberate, agreed trade-off, but the diagnostics gap itself remains. **Suggested fix (from the review):** accumulate polled events into a list outside the lambda (`collected.addAll(reader.poll(10))`) and assert on the accumulated collection, or await only until a poll returns non-empty and then assert once outside Awaitility, so the first observed payload ends up in the failure message. ## Related suggestion (non-blocking, deferred with the issue) @SEZ9 also suggested a one-line pointer from the `AWAIT_BUDGET_SECONDS` javadoc to this issue, so the next person who sees a flake understands the ceiling is a margin fix and that the ordering/diagnostics gaps are known and tracked. That can land together with either fix above. Context: SEZ9's review at https://github.com/apache/seatunnel/pull/12381#pullrequestreview-3277564565 and the agreeing thread starting at https://github.com/apache/seatunnel/pull/12381#issuecomment-3278079892 -- 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]
