DanielLeens commented on PR #12081:
URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-5812753961
Thanks @SEZ9 — happy to close the remaining pointers so you can do the final
pass.
**F3 (timed `get()` contract change to `TimeoutException`):** Traced both
production call sites in this same diff (`IMapFileStorage.java`) — there are no
others in the module. `queryExecuteStatus(long, long)` now does `try { return
Boolean.TRUE.equals(requestFuture.get(timeout, TimeUnit.MILLISECONDS)); } catch
(TimeoutException e) { ... } return false;`, and `batchQueryExecuteFailsStatus`
does the same per-entry with the shared `deadlineNanos`. Both were rewritten in
this PR (pre-PR they read `requestFuture.isDone() ||
Boolean.TRUE.equals(requestFuture.get(...))` with no `TimeoutException` catch,
since the old `get(timeout, unit)` just returned `false` on expiry). So the
contract change and its only two callers were updated atomically in this commit
— net externally-observable behavior on timeout is unchanged (still resolves to
`false`/failure for that key), and there's no third caller anywhere in the
codebase that could have depended on the old return-false semantics.
**F5 (Mockito dependency):** `imap-storage-file/pom.xml` adds an explicit
test-scope `org.mockito:mockito-junit-jupiter` dependency, with a comment
noting it's for `HdfsWriterFlushCallCountTest` /
`WALWorkHandlerSurvivabilityTest` and that it's also inherited from the root
POM — added explicitly so the module's test classpath doesn't require reading
the parent to understand.
**F8 (timeout logging level):** Already matches what you're asking for.
`queryExecuteStatus`'s `catch (TimeoutException e)` block logs a single
`log.warn(...)` line with requestId/elapsed/limit and no stack trace, then a
separate `log.debug(...)` carries the full exception only at DEBUG. The
remaining `catch (Exception e)` branch (genuinely unexpected errors) is still
`log.error(...)` with the stack trace, which is correct — only the
expected-timeout path was downgraded.
On the `Error`/dead-worker follow-up: I don't see an existing tracking issue
yet (searched apache/seatunnel issues for `WALWorkHandler`/worker-death,
nothing open). I'll leave opening it to whichever of you gets there first, as
discussed in my last comment — just tag me on it and I'll take a look.
--
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]
