You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by st...@apache.org on 2019/07/17 12:53:30 UTC
[hadoop] branch branch-3.2 updated: HADOOP-16341.
ShutDownHookManager: Regressed performance on Hook removals after
HADOOP-15679
This is an automated email from the ASF dual-hosted git repository.
stevel pushed a commit to branch branch-3.2
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/branch-3.2 by this push:
new 76d2cd2 HADOOP-16341. ShutDownHookManager: Regressed performance on Hook removals after HADOOP-15679
76d2cd2 is described below
commit 76d2cd228336dde207e47c9a659ce48bcb9b4e8f
Author: Gopal V <go...@cloudera.com>
AuthorDate: Wed Jul 17 13:52:58 2019 +0100
HADOOP-16341. ShutDownHookManager: Regressed performance on Hook removals after HADOOP-15679
Contributed by Gopal V and Atilla Magyar.
Change-Id: I066d5eece332a1673594de0f9b484443f95530ec
---
.../apache/hadoop/util/ShutdownHookManager.java | 21 ++++++++++------
.../hadoop/util/TestShutdownHookManager.java | 29 ++++++++++++++--------
2 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ShutdownHookManager.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ShutdownHookManager.java
index 2ca8e55..3b8d577 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ShutdownHookManager.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ShutdownHookManager.java
@@ -92,7 +92,7 @@ public final class ShutdownHookManager {
return;
}
long started = System.currentTimeMillis();
- int timeoutCount = executeShutdown();
+ int timeoutCount = MGR.executeShutdown();
long ended = System.currentTimeMillis();
LOG.debug(String.format(
"Completed shutdown in %.3f seconds; Timeouts: %d",
@@ -116,9 +116,9 @@ public final class ShutdownHookManager {
*/
@InterfaceAudience.Private
@VisibleForTesting
- static int executeShutdown() {
+ int executeShutdown() {
int timeouts = 0;
- for (HookEntry entry: MGR.getShutdownHooksInOrder()) {
+ for (HookEntry entry: getShutdownHooksInOrder()) {
Future<?> future = EXECUTOR.submit(entry.getHook());
try {
future.get(entry.getTimeout(), entry.getTimeUnit());
@@ -254,7 +254,9 @@ public final class ShutdownHookManager {
private AtomicBoolean shutdownInProgress = new AtomicBoolean(false);
//private to constructor to ensure singularity
- private ShutdownHookManager() {
+ @VisibleForTesting
+ @InterfaceAudience.Private
+ ShutdownHookManager() {
}
/**
@@ -267,8 +269,8 @@ public final class ShutdownHookManager {
@VisibleForTesting
List<HookEntry> getShutdownHooksInOrder() {
List<HookEntry> list;
- synchronized (MGR.hooks) {
- list = new ArrayList<>(MGR.hooks);
+ synchronized (hooks) {
+ list = new ArrayList<>(hooks);
}
Collections.sort(list, new Comparator<HookEntry>() {
@@ -342,7 +344,9 @@ public final class ShutdownHookManager {
throw new IllegalStateException("Shutdown in progress, cannot remove a " +
"shutdownHook");
}
- return hooks.remove(new HookEntry(shutdownHook, 0));
+ // hooks are only == by runnable
+ return hooks.remove(new HookEntry(shutdownHook, 0, TIMEOUT_MINIMUM,
+ TIME_UNIT_DEFAULT));
}
/**
@@ -354,7 +358,8 @@ public final class ShutdownHookManager {
@InterfaceAudience.Public
@InterfaceStability.Stable
public boolean hasShutdownHook(Runnable shutdownHook) {
- return hooks.contains(new HookEntry(shutdownHook, 0));
+ return hooks.contains(new HookEntry(shutdownHook, 0, TIMEOUT_MINIMUM,
+ TIME_UNIT_DEFAULT));
}
/**
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShutdownHookManager.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShutdownHookManager.java
index 03fa903..59430e8 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShutdownHookManager.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShutdownHookManager.java
@@ -21,7 +21,6 @@ import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
-import org.junit.After;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -43,13 +42,10 @@ public class TestShutdownHookManager {
LoggerFactory.getLogger(TestShutdownHookManager.class.getName());
/**
- * remove all the shutdown hooks so that they never get invoked later
- * on in this test process.
+ * A new instance of ShutdownHookManager to ensure parallel tests
+ * don't have shared context.
*/
- @After
- public void clearShutdownHooks() {
- ShutdownHookManager.get().clearShutdownHooks();
- }
+ private final ShutdownHookManager mgr = new ShutdownHookManager();
/**
* Verify hook registration, then execute the hook callback stage
@@ -58,7 +54,6 @@ public class TestShutdownHookManager {
*/
@Test
public void shutdownHookManager() {
- ShutdownHookManager mgr = ShutdownHookManager.get();
assertNotNull("No ShutdownHookManager", mgr);
assertEquals(0, mgr.getShutdownHooksInOrder().size());
Hook hook1 = new Hook("hook1", 0, false);
@@ -119,7 +114,7 @@ public class TestShutdownHookManager {
// now execute the hook shutdown sequence
INVOCATION_COUNT.set(0);
LOG.info("invoking executeShutdown()");
- int timeouts = ShutdownHookManager.executeShutdown();
+ int timeouts = mgr.executeShutdown();
LOG.info("Shutdown completed");
assertEquals("Number of timed out hooks", 1, timeouts);
@@ -193,7 +188,6 @@ public class TestShutdownHookManager {
*/
@Test
public void testDuplicateRegistration() throws Throwable {
- ShutdownHookManager mgr = ShutdownHookManager.get();
Hook hook = new Hook("hook1", 0, false);
// add the hook
@@ -222,6 +216,21 @@ public class TestShutdownHookManager {
}
+ @Test
+ public void testShutdownRemove() throws Throwable {
+ assertNotNull("No ShutdownHookManager", mgr);
+ assertEquals(0, mgr.getShutdownHooksInOrder().size());
+ Hook hook1 = new Hook("hook1", 0, false);
+ Hook hook2 = new Hook("hook2", 0, false);
+ mgr.addShutdownHook(hook1, 9); // create Hook1 with priority 9
+ assertTrue("No hook1", mgr.hasShutdownHook(hook1)); // hook1 lookup works
+ assertEquals(1, mgr.getShutdownHooksInOrder().size()); // 1 hook
+ assertFalse("Delete hook2 should not be allowed",
+ mgr.removeShutdownHook(hook2));
+ assertTrue("Can't delete hook1", mgr.removeShutdownHook(hook1));
+ assertEquals(0, mgr.getShutdownHooksInOrder().size());
+ }
+
private static final AtomicInteger INVOCATION_COUNT = new AtomicInteger();
/**
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org