[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13908183#comment-13908183
]
Feng Honghua commented on HBASE-10516:
--
[~nkeywal], thanks! :-)
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
Affects Versions: 0.98.0, 0.99.0
Reporter: Feng Honghua
Assignee: Feng Honghua
Fix For: 0.99.0
Attachments: HBASE-10516-trunk_v1.patch, HBASE-10516-trunk_v2.patch,
HBASE-10516-trunk_v3.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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13908306#comment-13908306
]
Hudson commented on HBASE-10516:
SUCCESS: Integrated in HBase-TRUNK #4940 (See
[https://builds.apache.org/job/HBase-TRUNK/4940/])
HBASE-10516 Refactor code where Threads.sleep is called within a while/for loop
(Feng Honghua) (nkeywal: rev 1570524)
*
/hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
*
/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
*
/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
*
/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java
*
/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java
*
/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
*
/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
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
Affects Versions: 0.98.0, 0.99.0
Reporter: Feng Honghua
Assignee: Feng Honghua
Fix For: 0.99.0
Attachments: HBASE-10516-trunk_v1.patch, HBASE-10516-trunk_v2.patch,
HBASE-10516-trunk_v3.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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13909144#comment-13909144
]
Hudson commented on HBASE-10516:
SUCCESS: Integrated in HBase-TRUNK-on-Hadoop-1.1 #96 (See
[https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-1.1/96/])
HBASE-10516 Refactor code where Threads.sleep is called within a while/for loop
(Feng Honghua) (nkeywal: rev 1570524)
*
/hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
*
/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/LruBlockCache.java
*
/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
*
/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/DeleteTableHandler.java
*
/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java
*
/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
*
/hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
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
Affects Versions: 0.98.0, 0.99.0
Reporter: Feng Honghua
Assignee: Feng Honghua
Fix For: 0.99.0
Attachments: HBASE-10516-trunk_v1.patch, HBASE-10516-trunk_v2.patch,
HBASE-10516-trunk_v3.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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13907837#comment-13907837
]
Feng Honghua commented on HBASE-10516:
--
[~nkeywal], can this be committed as well? thanks:-)
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,
HBASE-10516-trunk_v3.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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13905077#comment-13905077
]
Feng Honghua commented on HBASE-10516:
--
bq.Nice writeup @nkeywal (and nice commentary Feng Honghua – especially the bit
about resetting interrupt is useless unless it checked in upper layers). We
should extract your wisdom and put on dev list? It is good stuff. I can write
it up in refguide after we are agreed.
Sounds good, [~stack]
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,
HBASE-10516-trunk_v3.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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13905088#comment-13905088
]
Nicolas Liochon commented on HBASE-10516:
-
I'm +1 for v3. I will commit shortly if no one disagree.
Thanks for the patch, Feng. Sorry again the the review in 2 passes, thanks for
taking the comments into account.
I'm going to review the other patches today.
I will write up a small text on interruption for the dev list as well.
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,
HBASE-10516-trunk_v3.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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13905335#comment-13905335
]
Feng Honghua commented on HBASE-10516:
--
bq.Sorry again the the review in 2 passes
No sorry here, your review is really appreciated:-)
bq.I will write up a small text on interruption for the dev list as well.
That's great!
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,
HBASE-10516-trunk_v3.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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13905474#comment-13905474
]
Hadoop QA commented on HBASE-10516:
---
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12629734/HBASE-10516-trunk_v3.patch
against trunk revision .
ATTACHMENT ID: 12629734
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 3 new
or modified tests.
{color:green}+1 hadoop1.0{color}. The patch compiles against the hadoop
1.0 profile.
{color:green}+1 hadoop1.1{color}. The patch compiles against the hadoop
1.1 profile.
{color:green}+1 javadoc{color}. The javadoc tool did not generate any
warning messages.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 lineLengths{color}. The patch does not introduce lines
longer than 100
{color:green}+1 site{color}. The mvn site goal succeeds with this patch.
{color:red}-1 core tests{color}. The patch failed these unit tests:
org.apache.hadoop.hbase.util.TestHBaseFsck
Test results:
https://builds.apache.org/job/PreCommit-HBASE-Build/8742//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8742//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8742//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8742//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8742//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8742//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8742//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8742//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8742//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8742//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output:
https://builds.apache.org/job/PreCommit-HBASE-Build/8742//console
This message is automatically generated.
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,
HBASE-10516-trunk_v3.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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13906596#comment-13906596
]
Feng Honghua commented on HBASE-10516:
--
I rerun the unit tests locally for this patch and all pass.
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,
HBASE-10516-trunk_v3.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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13903973#comment-13903973
]
Feng Honghua commented on HBASE-10516:
--
bq.Seems we should discuss and agree on a final uniform handling for IE before
I can go ahead modifying the patch
Should we treat it case-by-case, or per-class, or uniformly for all occurrences?
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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13903994#comment-13903994
]
Nicolas Liochon commented on HBASE-10516:
-
bq. Should we treat it case-by-case, or per-class, or uniformly for all
occurrences?
There is a general strategy, then the exceptions :-).
I think we agree on the general strategy, let me formulate it to confirm:
- never never never shallow IE
- if possible, stop the current task, if not possible, continue, and manage
the interruption when the non interruptible task has been done
- try to throw a clear exception (IE or IOE), if not restore the status
catch and rethrow a RuntimeException instead is generally bad. It means that
you do not accept to be interrupted. So we should not add such kind of code.
But in such a code
void f(){
partOne(); //shallows exception
partTwo(); // throw Runtime on Exception
}
It's acceptable, imho, to change the part one to make it throws the same
runtime as the part 2.
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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13903998#comment-13903998
]
Feng Honghua commented on HBASE-10516:
--
Thanks [~nkeywal], let me re-examine the patches accordingly, it may involve
some more changes since I only fixed the 'restoring interrupt status within a
while/for loop' bug without changing the handling semantic, there are more work
to do to align the existing IE handling with the general strategy...
Please hold on the review, I'll ping you when I'm done :-)
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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13904012#comment-13904012
]
Nicolas Liochon commented on HBASE-10516:
-
ok :-)
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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13904835#comment-13904835
]
stack commented on HBASE-10516:
---
Nice writeup @nkeywal (and nice commentary [~fenghh] -- especially the bit
about resetting interrupt is useless unless it checked in upper layers). We
should extract your wisdom and put on dev list? It is good stuff. I can write
it up in refguide after we are agreed.
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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13903302#comment-13903302
]
Nicolas Liochon commented on HBASE-10516:
-
sleepBeforeRetry = sure
createDir = why not thowing an InterrupedIOException?
rename = same
deleteDir = same
HRegionServer#waitOnAllRegionsToClose = I suppose it's the best option
For InterrupedIOException, my *feeling* is that we can already receive these
exceptions today (as we're going network disk operations, they are likely to
check the interrupt status already, or may do it in the future).
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
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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13903804#comment-13903804
]
Hadoop QA commented on HBASE-10516:
---
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12629470/HBASE-10516-trunk_v2.patch
against trunk revision .
ATTACHMENT ID: 12629470
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:red}-1 tests included{color}. The patch doesn't appear to include
any new or modified tests.
Please justify why no new tests are needed for this
patch.
Also please list what manual steps were performed to
verify this patch.
{color:green}+1 hadoop1.0{color}. The patch compiles against the hadoop
1.0 profile.
{color:green}+1 hadoop1.1{color}. The patch compiles against the hadoop
1.1 profile.
{color:green}+1 javadoc{color}. The javadoc tool did not generate any
warning messages.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 lineLengths{color}. The patch does not introduce lines
longer than 100
{color:green}+1 site{color}. The mvn site goal succeeds with this patch.
{color:green}+1 core tests{color}. The patch passed unit tests in .
Test results:
https://builds.apache.org/job/PreCommit-HBASE-Build/8730//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8730//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8730//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8730//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8730//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8730//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8730//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8730//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8730//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8730//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output:
https://builds.apache.org/job/PreCommit-HBASE-Build/8730//console
This message is automatically generated.
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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13901213#comment-13901213
]
Nicolas Liochon commented on HBASE-10516:
-
It seems ok to me, but give me some time to review it in details (for today or
Monday: I'm off this week end).
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
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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13901342#comment-13901342
]
Hadoop QA commented on HBASE-10516:
---
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12628943/HBASE-10516-trunk_v1.patch
against trunk revision .
ATTACHMENT ID: 12628943
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:red}-1 tests included{color}. The patch doesn't appear to include
any new or modified tests.
Please justify why no new tests are needed for this
patch.
Also please list what manual steps were performed to
verify this patch.
{color:green}+1 hadoop1.0{color}. The patch compiles against the hadoop
1.0 profile.
{color:green}+1 hadoop1.1{color}. The patch compiles against the hadoop
1.1 profile.
{color:green}+1 javadoc{color}. The javadoc tool did not generate any
warning messages.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 lineLengths{color}. The patch does not introduce lines
longer than 100
{color:red}-1 site{color}. The patch appears to cause mvn site goal to
fail.
{color:green}+1 core tests{color}. The patch passed unit tests in .
Test results:
https://builds.apache.org/job/PreCommit-HBASE-Build/8695//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8695//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8695//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8695//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8695//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8695//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8695//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8695//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8695//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/8695//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
Console output:
https://builds.apache.org/job/PreCommit-HBASE-Build/8695//console
This message is automatically generated.
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
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)
[jira] [Commented] (HBASE-10516) Refactor code where Threads.sleep is called within a while/for loop
[
https://issues.apache.org/jira/browse/HBASE-10516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13901102#comment-13901102
]
stack commented on HBASE-10516:
---
eww. Its ugly. But makes sense. +1 from me. [~nkeywal] You good w/ this?
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
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)
