hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Feng Honghua (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 11:41:21 GMT

    [ https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13903965#comment-13903965

Feng Honghua commented on HBASE-10516:

bq. 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.
Seems we should discuss and agree on a final uniform handling for IE before I can go ahead
modifying the patch, since there are some different kinds of handling for it in existing code:-):
# restore interrupt status (but almost all of this kind handling have the problem of restoring
within a while/for loop)
# catch and rethrow it after transforming to InterruptedIOException
# catch and rethrow a RuntimeException instead

And we may need to change some of the upper code which eventually receives the IE if we choose
to rethrow instead of catch-and-restore.

bq.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
First thank very much for your review, I know it's always time-consuming and unhappy to review
an 'ugly' patch(as [~stack] said:-)), so your review is really appreciated! Second I agree
with you on that 'we need to think about the impact when we do the default strategy of restoring
the status', by restoring interrupt status we implicitly assume code somewhere higher up on
the call stack cares and is aware of whether current thread is ever interrupted, and checks
the interrupt status and takes some action accordingly, otherwise restoring interrupt status
is totally meaningless. Restoring interrupt status is just the first half part of a cooperative

> 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

View raw message