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

Feng Honghua commented on HBASE-10497:
--------------------------------------

Thanks [~nkeywal] for the prompt review!
bq.This one is likely wrong...As it's inside a loop, we're likely to loop 
again, then the sleep will be interrupted immediately as we restored the 
interruption status, then we will log again => we will flood the logs
Good catch, my carelessness. To keep the same semantic(in terms of how/when we 
handle the exception), I use below code block for such non-cancelable tasks as 
while/for loops to prevent latter sleep from immediately throwing 
InterruptedException due to the re-interrupt issued by former iteration's 
catch...
{code}
  boolean interrupted = false;
  try {
    while (...) {
      try {
        ...
        Thread.sleep(...);
        ...
      } catch (InterruptedException e) {
        interrupted = true;
      }
    }
  } finally {
    if (interrupted) {
      LOG.warn(...);
      Thread.currentThread().interrupt();
    }
  }
{code}

bq.Because we took care of the interruption already (may be wrongly however), 
so the thread is not interrupted any more: the -1 means: we stop. It's the same 
for the one with the split worker likely.
As you said we do take care of the interruption already, but they are within 
Runnable and Tool respectively, it's still meaningful to re-interrupt current 
thread in order for code higher up the call stack to being able to know about 
the fact of interrupt, right? At least it does no harm to re-interrupt here.

> Add standard handling for swallowed InterruptedException thrown by 
> Thread.sleep under HBase-Client/HBase-Server folders systematically
> --------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HBASE-10497
>                 URL: https://issues.apache.org/jira/browse/HBASE-10497
>             Project: HBase
>          Issue Type: Improvement
>          Components: Client, regionserver
>            Reporter: Feng Honghua
>            Assignee: Feng Honghua
>            Priority: Minor
>         Attachments: HBASE-10497-trunk_v1.patch
>
>
> There are many places where InterruptedException thrown by Thread.sleep are 
> swallowed silently (which are neither declared in the caller method's throws 
> clause nor rethrown immediately) under HBase-Client/HBase-Server folders.
> It'd be better to add standard 'log and call currentThread.interrupt' for 
> such cases.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Reply via email to