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