hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Nicolas Liochon (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
Date Tue, 18 Feb 2014 09:57:20 GMT

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

Mime
View raw message