raunaqmorarka commented on PR #18540:
URL: https://github.com/apache/iceberg/pull/18540#issuecomment-6284660166
> `awaitBlocked` works because `FutureTask.get()` happens to park with the
future as the blocker. That's an OpenJDK implementation detail, not specified
anywhere, and three of the new tests depend on it. If it ever changes they'd
all just spin for 10s and time out. Could a `CountDownLatch` do the same job?
Switched to a latch. The test executor's futures count down a latch when
get() is called on a future that is not done, and the three tests wait on that.
> Also `assertThat(elapsedMillis).isLessThan(rounds * 10L)` is wall-clock.
The margin is wide so it's probably fine, but it's the first test I'd suspect
if this ever goes flaky on CI.
The timing test takes 32 to 41 ms locally against the 1000 ms limit. Polling
can't pass it, since each of the 100 rounds sleeps at least 10 ms.
> Minor: is `@SuppressWarnings("checkstyle:CyclomaticComplexity")` still
needed now the poll loop is gone?
The suppression is still needed. Without it checkstyle reports a cyclomatic
complexity of 14, and the limit is 12.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
