[Fix] Add sleep in zero buffer read when no data is flowing#5846
Conversation
Signed-off-by: Hai Yan <oeyh@amazon.com>
oeyh
requested review from
KarstenSchnitter,
chenqi0805,
dinujoh,
dlvenable,
engechas,
graytaylor0,
kkondaka,
san81,
sb2k16 and
srikanthjg
as code owners
| public void testReadFromEmptyBufferCallsThreadSleep() { | ||
| ZeroBuffer<Record<String>> zeroBuffer = createObjectUnderTest(); | ||
|
|
||
| long startTime = System.currentTimeMillis(); |
There was a problem hiding this comment.
Our build are taking a long time and tests with sleeps contribute to this. I think using Mockito's mockStatic on Thread.sleep() is better here. I think some other tests do this.
There was a problem hiding this comment.
Got this error when trying to use mockStatic on Thread.class. Looks like it's not possible.
It is not possible to mock static methods of java.lang.Thread to avoid interfering with class loading what leads to infinite loops
There was a problem hiding this comment.
Updated the test to test Thread.sleep() call indirectly through Thread.currentThread().interrupt() instead.
Signed-off-by: Hai Yan <oeyh@amazon.com>
| try { | ||
| Thread.sleep(DEFAULT_READ_SLEEP_MILLIS); | ||
| } catch (InterruptedException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
Why do we need to throw if sleep is interrupted. It should be silently ignored, right?
There was a problem hiding this comment.
I agree. Otherwise it may break the process loop and shutdown.
Signed-off-by: Hai Yan <oeyh@amazon.com>
| Thread.sleep(DEFAULT_READ_SLEEP_MILLIS); | ||
| } catch (InterruptedException e) { | ||
| // Restore the interrupted status | ||
| Thread.currentThread().interrupt(); |
There was a problem hiding this comment.
I do not think Thread.currentThread().interrupt(); is needed. We do not need to restore the "interrupted" status.
Signed-off-by: Hai Yan <oeyh@amazon.com>
…ch-project#5846) * Add sleep in zero buffer read when no data is flowing Signed-off-by: Hai Yan <oeyh@amazon.com> * Update unit test Signed-off-by: Hai Yan <oeyh@amazon.com> * Catch InterruptedException and not throw Signed-off-by: Hai Yan <oeyh@amazon.com> * Address one more comment Signed-off-by: Hai Yan <oeyh@amazon.com> --------- Signed-off-by: Hai Yan <oeyh@amazon.com> Signed-off-by: Jonah Calvo <caljonah@amazon.com>
4 participants
Description
The ZeroBuffer was causing high CPU usage when no data was flowing through the pipeline. The read() method (called by ProcessWorker#run method) would continuously return immediately when no data is flowing, creating a tight loop that consumed excessive CPU resources.
This PR:
Testing:
ProcessWorker#runno longer takes most of the cpu time.Issues Resolved
N/A
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.