[GitHub] [hbase] apurtell edited a comment on pull request #3302: HBASE-25911 Replace calls to System.currentTimeMillis with EnvironmentEdgeManager.currentTime

2021-05-28 Thread GitBox


apurtell edited a comment on pull request #3302:
URL: https://github.com/apache/hbase/pull/3302#issuecomment-850569022


   > But sometimes we use System.currentTimeMillis just for calculating elapsed 
time, not for getting system clock, I'm not sure if this is also our goal here, 
to also replace these usages.
   
   @Apache9 This is not the goal.
   
   > We introduced EnvironmentEdgeManager a long time ago as a way to inject 
alternate clocks (gettimeofday() aka System.currentTimeMillis()) for unit 
tests. In order for this to be effective, all callers that would otherwise use 
System.currentTimeMillis() must call EnvironmentEdgeManager.currentTime() 
instead, except obviously the implementors of EnvironmentEdge.
   >
   > It's common for contributors to be unaware of this practice and reviewers 
might not catch it.
   
   The goal is to fix the code is out of practice with 
EnvironmentEdgeManager#currentTime. 
   
   We can also do something about nanotime, but under another JIRA, please. 
Should we file a JIRA to discuss what to do, if anything, about nanotime?


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




[GitHub] [hbase] apurtell edited a comment on pull request #3302: HBASE-25911 Replace calls to System.currentTimeMillis with EnvironmentEdgeManager.currentTime

2021-05-27 Thread GitBox


apurtell edited a comment on pull request #3302:
URL: https://github.com/apache/hbase/pull/3302#issuecomment-849872794


   > What about System.nanoTime?
   
   @Apache9 System.nanotime is not used to get the system clock. I don't mind 
bringing it into EnvironmentEdge but I don't care about it here. It is out of 
scope for all of this work. 
   
   Feel free to file a follow up JIRA, to add it to EnvironmentEdge in the 
future, if you would like.


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