[
https://issues.apache.org/jira/browse/HBASE-6476?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13470516#comment-13470516
]
Chris Trezzo commented on HBASE-6476:
-------------------------------------
A few curious things (if anyone has any thoughts on these that would be great):
1. Check out the Test history:
https://builds.apache.org/job/HBase-TRUNK/3392/testReport/junit/org.apache.hadoop.hbase.util/TestThreads/testSleepWithoutInterrupt/history/
The System.currentTimeMillis() change might not have caused the test failure
(it has failed twice for the same reason since the patch was reverted). In
addition, there is something wonky going on because the test duration for all
the failed runs is 1~2ms. The error message for the failed runs stated that the
test timed out after 6000ms, which is the timeout set in the test tag.
2. The method sleepWithoutInterrupt is only called from test code, except
during thrift server shutdown. Look at
TBoundedThreadPoolServer.shutdownServer(). Anyone have any thoughts why we do
the extra wait which calls sleepWithoutInterrupt? My thoughts are that we could
remove the extra wait since we are already calling awaitTermination on the
executor service in a loop above. Either that, or we could just replace the
call with another call to awaitTermination and keep the second wait loop. This
would limit sleepWithoutInterrupt calls to just test code.
3. Another tricky part with the EdgeManager is dealing with small tests that
run in parallel within the same JVM. If test1 uses a non-default
EnvironmentEdge, and test2 is relying on the DefaultEdge and doesn't set it
explicitly, test2 would unknowingly be using whatever EnvironmentEdge test1
set. This would create flapping tests that might be tricky to debug.
Thoughts?
Thanks,
Chris
> Replace all occurrances of System.currentTimeMillis() with EnvironmentEdge
> equivalent
> -------------------------------------------------------------------------------------
>
> Key: HBASE-6476
> URL: https://issues.apache.org/jira/browse/HBASE-6476
> Project: HBase
> Issue Type: Bug
> Reporter: Lars Hofhansl
> Assignee: Chris Trezzo
> Priority: Minor
> Fix For: 0.96.0
>
> Attachments: 6476.txt, 6476-v2.txt, 6476-v2.txt, 6476v3.txt
>
>
> There are still some areas where System.currentTimeMillis() is used in HBase.
> In order to make all parts of the code base testable and (potentially) to be
> able to configure HBase's notion of time, this should be generally be
> replaced with EnvironmentEdgeManager.currentTimeMillis().
> How hard would it be to add a maven task that checks for that, so we do not
> introduce System.currentTimeMillis back in the future?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
