Fix up ClusterServiceIT #90397
Fix up ClusterServiceIT #90397
Conversation
ClusterServiceIT#testPendingUpdateTask has some unbounded waits, it relies on the clock advancing by at least 1ms which might not happen, and it leaves the cluster service thread blocked on failure which causes knock-on effects. This commit addresses these problems.
DaveCTurner
added
>test
elasticsearchmachine
added
the
Team:Distributed (Obsolete)
|
Pinging @elastic/es-distributed (Team:Distributed) |
| } | ||
| Thread.sleep(100); | ||
| final var startNanoTime = System.nanoTime(); | ||
| while (TimeUnit.MILLISECONDS.convert(System.nanoTime() - startNanoTime, TimeUnit.NANOSECONDS) <= 0) { |
There was a problem hiding this comment.
This looks weird to me. Does this mean that you wait at least for 1ms to have passed? And if not, you then wait for 100ms (which defeats the purpose of the while loop -- except for rare cases where the sleep is interrupted within 1ms)?
There was a problem hiding this comment.
Ah re-reading the PR description seems to clarify this now. Indeed you want to ensure that it waits at least 1ms. Couldn't we better though try-catch that thread.sleep for InterruptedException?
There was a problem hiding this comment.
I encountered a test failure where even the existing 100ms sleep didn't result in System.nanoTime() returning a newer time.
I don't think we need to catch an InterruptedException here, we can just fail the test in that case and let the test runner work out what to do with the interrupt.
There was a problem hiding this comment.
Wow, that seems like a nasty bug. I see, thanks for explaining.
| } | ||
| Thread.sleep(100); | ||
| final var startNanoTime = System.nanoTime(); | ||
| while (TimeUnit.MILLISECONDS.convert(System.nanoTime() - startNanoTime, TimeUnit.NANOSECONDS) <= 0) { |
There was a problem hiding this comment.
Wow, that seems like a nasty bug. I see, thanks for explaining.
|
Ah, I just realised the failure I was chasing was on a branch that uses a much coarser clock for these stats than |
ClusterServiceIT#testPendingUpdateTask has some unbounded waits, it relies on the clock advancing by at least 1ms which might not happen, and it leaves the cluster service thread blocked on failure which causes knock-on effects. This commit addresses these problems.