[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13903913#comment-13903913
]
Nicolas Liochon commented on HBASE-10516:
-----------------------------------------
I'm ok with the new patch. Thanks!
I didn't see the DeleteTableHandler / AssignmentManager / LruBlockCache /
ZooKeeperWatcher changes yesterday.
DeleteTableHandler
=> I think we can just throw an IOE exception here, it will be as if the loop
timeouted. (see the
{code}
throw new IOException("Waited hbase.master.wait.on.region (" +
waitTime + "ms) for region to leave region " +
region.getRegionNameAsString() + " in transitions");
{code}
AssignmentManager
=> It's not really managed today: the IE is transformed in a RuntimeException.
The patch will increase the probability.
I would propose to explicitly throw IE here from the method here. In any case,
it's a risky patch, but throwing an exception seems clearer.
LruBlockCache
We don't need to continue to loop here. We can stop looping at the first
interrupt (and may be we should restore the status?)
ZooKeeperWatcher
We can manage the interrupt as a timeout (so just throwing a Runtime: today we
throw a NPE (!) on timeout, we can do the same or a little bit cleaner).
Sorry for missing this part yesterday.
I will have a look at the other patshes as well. Basically, I think it's better
to manage the exception when we do the patch, because anyway, we need to think
about the impact when we do the default strategy of restoring the status. And
throwing and exception is often better, and requires less code, so...
> Refactor code where Threads.sleep is called within a while/for loop
> -------------------------------------------------------------------
>
> Key: HBASE-10516
> URL: https://issues.apache.org/jira/browse/HBASE-10516
> Project: HBase
> Issue Type: Bug
> Components: Client, master, regionserver
> Reporter: Feng Honghua
> Assignee: Feng Honghua
> Attachments: HBASE-10516-trunk_v1.patch, HBASE-10516-trunk_v2.patch
>
>
> Threads.sleep implementation:
> {code}
> public static void sleep(long millis) {
> try {
> Thread.sleep(millis);
> } catch (InterruptedException e) {
> e.printStackTrace();
> Thread.currentThread().interrupt();
> }
> }
> {code}
> From above implementation, the current thread's interrupt status is
> restored/reset when InterruptedException is caught and handled. If this
> method is called within a while/for loop, if a first InterruptedException is
> thrown during sleep, it will make the Threads.sleep in next loop immediately
> throw InterruptedException without expected sleep. This behavior breaks the
> intention for independent sleep in each loop
> I mentioned above in HBASE-10497 and this jira is created to handle it
> separately per [~nkeywal]'s suggestion
--
This message was sent by Atlassian JIRA
(v6.1.5#6160)
