Revert "HBASE-14802 Replaying server crash recovery procedure after a failover causes incorrect handling of deadservers"
This reverts commit 7c3c9ac9c67cd03f9a915f528d22cb4ed81cb6e8. Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/bb658134 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/bb658134 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/bb658134 Branch: refs/heads/hbase-12439 Commit: bb6581345fd9ecac964e19cea2293477162801ca Parents: 4350632 Author: stack <st...@apache.org> Authored: Sat Nov 14 16:59:35 2015 -0800 Committer: stack <st...@apache.org> Committed: Sat Nov 14 16:59:35 2015 -0800 ---------------------------------------------------------------------- .../apache/hadoop/hbase/master/DeadServer.java | 34 ++++---------------- .../master/procedure/ServerCrashProcedure.java | 12 ------- .../hadoop/hbase/master/TestDeadServer.java | 19 ----------- 3 files changed, 6 insertions(+), 59 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/bb658134/hbase-server/src/main/java/org/apache/hadoop/hbase/master/DeadServer.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/DeadServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/DeadServer.java index c33cdcc..8b16b00 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/DeadServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/DeadServer.java @@ -59,11 +59,6 @@ public class DeadServer { private int numProcessing = 0; /** - * Whether a dead server is being processed currently. - */ - private boolean processing = false; - - /** * A dead server that comes back alive has a different start code. The new start code should be * greater than the old one, but we don't take this into account in this method. * @@ -99,7 +94,9 @@ public class DeadServer { * * @return true if any RS are being processed as dead */ - public synchronized boolean areDeadServersInProgress() { return processing; } + public synchronized boolean areDeadServersInProgress() { + return numProcessing != 0; + } public synchronized Set<ServerName> copyServerNames() { Set<ServerName> clone = new HashSet<ServerName>(deadServers.size()); @@ -112,34 +109,15 @@ public class DeadServer { * @param sn the server name */ public synchronized void add(ServerName sn) { - processing = true; + this.numProcessing++; if (!deadServers.containsKey(sn)){ deadServers.put(sn, EnvironmentEdgeManager.currentTime()); } } - /** - * Notify that we started processing this dead server. - * @param sn ServerName for the dead server. - */ - public synchronized void notifyServer(ServerName sn) { - if (LOG.isDebugEnabled()) { LOG.debug("Started processing " + sn); } - processing = true; - numProcessing++; - } - public synchronized void finish(ServerName sn) { - numProcessing--; - if (LOG.isDebugEnabled()) LOG.debug("Finished " + sn + "; numProcessing=" + numProcessing); - - assert numProcessing >= 0: "Number of dead servers in processing should always be non-negative"; - - if (numProcessing < 0) { - LOG.error("Number of dead servers in processing = " + numProcessing - + ". Something went wrong, this should always be non-negative."); - numProcessing = 0; - } - if (numProcessing == 0) { processing = false; } + if (LOG.isDebugEnabled()) LOG.debug("Finished " + sn + "; numProcessing=" + this.numProcessing); + this.numProcessing--; } public synchronized int size() { http://git-wip-us.apache.org/repos/asf/hbase/blob/bb658134/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java index 5c9f6f4..1e86a23 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java @@ -110,11 +110,6 @@ implements ServerProcedureInterface { private ServerName serverName; /** - * Whether DeadServer knows that we are processing it. - */ - private boolean notifiedDeadServer = false; - - /** * Regions that were on the crashed server. */ private Set<HRegionInfo> regionsOnCrashedServer; @@ -188,13 +183,6 @@ implements ServerProcedureInterface { if (!services.getAssignmentManager().isFailoverCleanupDone()) { throwProcedureYieldException("Waiting on master failover to complete"); } - // HBASE-14802 - // If we have not yet notified that we are processing a dead server, we should do now. - if (!notifiedDeadServer) { - services.getServerManager().getDeadServers().notifyServer(serverName); - notifiedDeadServer = true; - } - try { switch (state) { case SERVER_CRASH_START: http://git-wip-us.apache.org/repos/asf/hbase/blob/bb658134/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestDeadServer.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestDeadServer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestDeadServer.java index 09122a1..40d26f4 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestDeadServer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestDeadServer.java @@ -17,11 +17,7 @@ */ package org.apache.hadoop.hbase.master; -import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.ServerName; -import org.apache.hadoop.hbase.master.procedure.ServerCrashProcedure; -import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; -import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; @@ -47,19 +43,16 @@ public class TestDeadServer { @Test public void testIsDead() { DeadServer ds = new DeadServer(); ds.add(hostname123); - ds.notifyServer(hostname123); assertTrue(ds.areDeadServersInProgress()); ds.finish(hostname123); assertFalse(ds.areDeadServersInProgress()); ds.add(hostname1234); - ds.notifyServer(hostname1234); assertTrue(ds.areDeadServersInProgress()); ds.finish(hostname1234); assertFalse(ds.areDeadServersInProgress()); ds.add(hostname12345); - ds.notifyServer(hostname12345); assertTrue(ds.areDeadServersInProgress()); ds.finish(hostname12345); assertFalse(ds.areDeadServersInProgress()); @@ -82,18 +75,6 @@ public class TestDeadServer { assertFalse(ds.cleanPreviousInstance(deadServerHostComingAlive)); } - @Test(timeout = 15000) - public void testCrashProcedureReplay() throws Exception { - HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); - TEST_UTIL.startMiniCluster(); - HMaster master = TEST_UTIL.getHBaseCluster().getMaster(); - ProcedureExecutor pExecutor = master.getMasterProcedureExecutor(); - ServerCrashProcedure proc = new ServerCrashProcedure(hostname123, false, false); - - ProcedureTestingUtility.submitAndWait(pExecutor, proc); - - assertFalse(master.getServerManager().getDeadServers().areDeadServersInProgress()); - } @Test public void testSortExtract(){