Fix TestNodeStateManager flakiness#28055
Conversation
|
Nice work, Awesome! It also means, that a few blind fixes that we did in the past (like extending timeouts), were probably not needed. |
| () -> nodeStateManager.getServerState().equals(DRAINED)) | ||
| .until(() -> nodeStateManager.getServerState().equals(DRAINING)); | ||
| AtomicBoolean everDrained = new AtomicBoolean(false); | ||
| assertEventually( |
There was a problem hiding this comment.
I'll take a second look at this condition. I do not remember what was my intention. But probably it does not need to be complex.
There was a problem hiding this comment.
During means: value must be maintained CONTINUOUSLY for the period given to "during" method.
So I think your code skips that part. But as I mentioned before, we probably do not need to verify such state.
There was a problem hiding this comment.
how should i have written this?
There was a problem hiding this comment.
it is actually ok. Sorry for the fuss.
There was a problem hiding this comment.
discussed offline, it's apparently fine as is
|
I think we could achieve the same by changing from the default config in Awaitility LGTM! |
`TestNodeStateManager.testDrain` was flaky when run together with `TestHashJoinOperator`. `TestHashJoinOperator` creates executor and schedules some tasks. Some of these tasks schedule further tasks. Some tasks fail with `RejectedExecutionException`. `TestNodeStateManager` used (a shaded version of) awaitility library. This library, by default, installs global uncaught exception handler and was intercepting failures occurring in threads spawned by `TestHashJoinOperator` and propagating them to unsuspecting `TestNodeStateManager`.
findepi
force-pushed
the
findepi/fix-testnodestatemanager-flakiness-2805e7
branch
from
ab7ac82 to
6a0602a
Compare
yes, but
|
3 participants
TestNodeStateManager.testDrainwas flaky when run together withTestHashJoinOperator.TestHashJoinOperatorcreates executor and schedules some tasks. Some of these tasks schedule further tasks. Some tasks fail withRejectedExecutionException.TestNodeStateManagerused (a shaded version of) awaitility library. This library, by default, installs global uncaught exception handler and was intercepting failures occurring in threads spawned byTestHashJoinOperatorand propagating them to unsuspectingTestNodeStateManager.TestNodeStateManager#25705awaitilityis being deny-listed in Disallow use of awaitility library airlift/airbase#1070