soondenana commented on pull request #9347:
URL: https://github.com/apache/kafka/pull/9347#issuecomment-702938936


   > @soondenana if the goal is to minimize the changes, would it be sufficient 
to change the code to use `Time.sleep(...)` instead of `Thread.sleep(...)`, and 
then change the `SystemTime.sleep(...)` implementation to return immediately if 
the supplied number of milliseconds is <= 0?
   > 
   > Are there performance implications of using `System.nanoTime()` instead of 
`System.currentTimeMillis()`?
   
   Thanks @rhauch for taking a look. The PR started with only fixing negative 
sleep vlaue, but as we started looking at code more, there were multiple issue 
so the "minimize change" idea was dropped. Here are 3 issues with original code:
   
   1. Negative sleep value
   2. Using elapsed time since loop started to decide on next sleep time.
   3. Sleeping even if the `partitionsFor` call would be successful for first 
time
   
   Considering that I decided to rewrite the loop to fix all three issues. 
   
   Good question on performance. It seems like `System.nanoTime()` is slower 
than `System.currentTimeMillis` (one is read from h/w using i/o another one is 
reading from memory), but not sure if that has any implication here for this 
use case. We are waiting for topic to appear in metadata and sleeping for 
seconds. Few milliseconds difference should not matter.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to