From 61388b41e7a1ad5125f5026dded4b5c054d55c4b Mon Sep 17 00:00:00 2001 From: Kihwal Lee Date: Tue, 13 Oct 2015 10:46:20 -0500 Subject: [PATCH] HDFS-8656. Preserve compatibility of ClientProtocol#rollingUpgrade after finalization. (cherry picked from commit 60b858bfa65e0feb665e1a84784a3d45e9091c66) Conflicts: hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestRollingUpgrade.java --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 + .../hadoop/hdfs/protocol/ClientProtocol.java | 5 +- .../hdfs/server/namenode/FSNamesystem.java | 8 ++- .../hdfs/server/namenode/NameNodeMXBean.java | 5 +- .../hadoop/hdfs/TestRollingUpgrade.java | 66 +++++++++++++++---- .../server/namenode/TestNameNodeMXBean.java | 5 +- 6 files changed, 71 insertions(+), 21 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index cb116e33cad..1df7f02a0f4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -31,6 +31,9 @@ Release 2.7.2 - UNRELEASED HDFS-8806. Inconsistent metrics: number of missing blocks with replication factor 1 not properly cleared. (Zhe Zhang via aajisaka) + HDFS-8656. Preserve compatibility of ClientProtocol#rollingUpgrade after + finalization. (wang) + HDFS-8852. HDFS architecture documentation of version 2.x is outdated about append write support. (Ajith S via aajisaka) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java index d4fe9038e3d..222971c7702 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java @@ -847,8 +847,9 @@ public interface ClientProtocol { /** * Rolling upgrade operations. - * @param action either query, start or finailze. - * @return rolling upgrade information. + * @param action either query, prepare or finalize. + * @return rolling upgrade information. On query, if no upgrade is in + * progress, returns null. */ @Idempotent public RollingUpgradeInfo rollingUpgrade(RollingUpgradeAction action) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 8c033037bbf..a2e35eb9819 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -7485,10 +7485,12 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, checkOperation(OperationCategory.READ); readLock(); try { - if (rollingUpgradeInfo != null) { - boolean hasRollbackImage = this.getFSImage().hasRollbackFSImage(); - rollingUpgradeInfo.setCreatedRollbackImages(hasRollbackImage); + if (!isRollingUpgrade()) { + return null; } + Preconditions.checkNotNull(rollingUpgradeInfo); + boolean hasRollbackImage = this.getFSImage().hasRollbackFSImage(); + rollingUpgradeInfo.setCreatedRollbackImages(hasRollbackImage); return rollingUpgradeInfo; } finally { readUnlock(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeMXBean.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeMXBean.java index 3c317fda7d9..0e4d44594d2 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeMXBean.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeMXBean.java @@ -81,9 +81,10 @@ public interface NameNodeMXBean { public boolean isUpgradeFinalized(); /** - * Gets the RollingUpgrade information + * Gets the RollingUpgrade information. * - * @return Rolling upgrade information + * @return Rolling upgrade information if an upgrade is in progress. Else + * (e.g. if there is no upgrade or the upgrade is finalized), returns null. */ public RollingUpgradeInfo.Bean getRollingUpgradeStatus(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestRollingUpgrade.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestRollingUpgrade.java index cd5ce77458c..67127c36b2d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestRollingUpgrade.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestRollingUpgrade.java @@ -19,6 +19,16 @@ package org.apache.hadoop.hdfs; import java.io.File; import java.io.IOException; +import java.lang.management.ManagementFactory; + +import javax.management.AttributeNotFoundException; +import javax.management.InstanceNotFoundException; +import javax.management.MBeanException; +import javax.management.MBeanServer; +import javax.management.MalformedObjectNameException; +import javax.management.ObjectName; +import javax.management.ReflectionException; +import javax.management.openmbean.CompositeDataSupport; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -44,6 +54,9 @@ import org.apache.hadoop.io.IOUtils; import org.junit.Assert; import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; /** * This class tests rolling upgrade. @@ -54,7 +67,7 @@ public class TestRollingUpgrade { public static void runCmd(DFSAdmin dfsadmin, boolean success, String... args) throws Exception { if (success) { - Assert.assertEquals(0, dfsadmin.run(args)); + assertEquals(0, dfsadmin.run(args)); } else { Assert.assertTrue(dfsadmin.run(args) != 0); } @@ -84,6 +97,7 @@ public class TestRollingUpgrade { //illegal argument "abc" to rollingUpgrade option runCmd(dfsadmin, false, "-rollingUpgrade", "abc"); + checkMxBeanIsNull(); //query rolling upgrade runCmd(dfsadmin, true, "-rollingUpgrade"); @@ -94,11 +108,16 @@ public class TestRollingUpgrade { //query rolling upgrade runCmd(dfsadmin, true, "-rollingUpgrade", "query"); + checkMxBean(); dfs.mkdirs(bar); //finalize rolling upgrade runCmd(dfsadmin, true, "-rollingUpgrade", "finalize"); + // RollingUpgradeInfo should be null after finalization, both via + // Java API and in JMX + assertNull(dfs.rollingUpgrade(RollingUpgradeAction.QUERY)); + checkMxBeanIsNull(); dfs.mkdirs(baz); @@ -195,8 +214,8 @@ public class TestRollingUpgrade { LOG.info("START\n" + info1); //query rolling upgrade - Assert.assertEquals(info1, dfs.rollingUpgrade(RollingUpgradeAction.QUERY)); - + assertEquals(info1, dfs.rollingUpgrade(RollingUpgradeAction.QUERY)); + dfs.mkdirs(bar); cluster.shutdown(); } @@ -216,13 +235,13 @@ public class TestRollingUpgrade { Assert.assertFalse(dfs2.exists(baz)); //query rolling upgrade in cluster2 - Assert.assertEquals(info1, dfs2.rollingUpgrade(RollingUpgradeAction.QUERY)); + assertEquals(info1, dfs2.rollingUpgrade(RollingUpgradeAction.QUERY)); dfs2.mkdirs(baz); LOG.info("RESTART cluster 2"); cluster2.restartNameNode(); - Assert.assertEquals(info1, dfs2.rollingUpgrade(RollingUpgradeAction.QUERY)); + assertEquals(info1, dfs2.rollingUpgrade(RollingUpgradeAction.QUERY)); Assert.assertTrue(dfs2.exists(foo)); Assert.assertTrue(dfs2.exists(bar)); Assert.assertTrue(dfs2.exists(baz)); @@ -236,7 +255,7 @@ public class TestRollingUpgrade { LOG.info("RESTART cluster 2 again"); cluster2.restartNameNode(); - Assert.assertEquals(info1, dfs2.rollingUpgrade(RollingUpgradeAction.QUERY)); + assertEquals(info1, dfs2.rollingUpgrade(RollingUpgradeAction.QUERY)); Assert.assertTrue(dfs2.exists(foo)); Assert.assertTrue(dfs2.exists(bar)); Assert.assertTrue(dfs2.exists(baz)); @@ -257,9 +276,31 @@ public class TestRollingUpgrade { } } + private static CompositeDataSupport getBean() + throws MalformedObjectNameException, MBeanException, + AttributeNotFoundException, InstanceNotFoundException, + ReflectionException { + MBeanServer mbs = ManagementFactory.getPlatformMBeanServer(); + ObjectName mxbeanName = + new ObjectName("Hadoop:service=NameNode,name=NameNodeInfo"); + return (CompositeDataSupport)mbs.getAttribute(mxbeanName, + "RollingUpgradeStatus"); + } + + private static void checkMxBeanIsNull() throws Exception { + CompositeDataSupport ruBean = getBean(); + assertNull(ruBean); + } + + private static void checkMxBean() throws Exception { + CompositeDataSupport ruBean = getBean(); + assertNotEquals(0l, ruBean.get("startTime")); + assertEquals(0l, ruBean.get("finalizeTime")); + } + @Test - public void testRollback() throws IOException { - // start a cluster + public void testRollback() throws Exception { + // start a cluster final Configuration conf = new HdfsConfiguration(); MiniDFSCluster cluster = null; try { @@ -277,10 +318,13 @@ public class TestRollingUpgrade { out.write(data, 0, data.length); out.close(); + checkMxBeanIsNull(); startRollingUpgrade(foo, bar, file, data, cluster); + checkMxBean(); cluster.getFileSystem().rollEdits(); cluster.getFileSystem().rollEdits(); rollbackRollingUpgrade(foo, bar, file, data, cluster); + checkMxBeanIsNull(); startRollingUpgrade(foo, bar, file, data, cluster); cluster.getFileSystem().rollEdits(); @@ -353,18 +397,18 @@ public class TestRollingUpgrade { // check the datanode final String dnAddr = dn.getDatanodeId().getIpcAddr(false); final String[] args1 = {"-getDatanodeInfo", dnAddr}; - Assert.assertEquals(0, dfsadmin.run(args1)); + runCmd(dfsadmin, true, args1); // issue shutdown to the datanode. final String[] args2 = {"-shutdownDatanode", dnAddr, "upgrade" }; - Assert.assertEquals(0, dfsadmin.run(args2)); + runCmd(dfsadmin, true, args2); // the datanode should be down. Thread.sleep(2000); Assert.assertFalse("DataNode should exit", dn.isDatanodeUp()); // ping should fail. - Assert.assertEquals(-1, dfsadmin.run(args1)); + assertEquals(-1, dfsadmin.run(args1)); } finally { if (cluster != null) cluster.shutdown(); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeMXBean.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeMXBean.java index c649621db73..da7e0568334 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeMXBean.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestNameNodeMXBean.java @@ -27,7 +27,6 @@ import org.apache.hadoop.hdfs.DFSTestUtil; import org.apache.hadoop.hdfs.MiniDFSCluster; import org.apache.hadoop.hdfs.server.datanode.DataNode; import org.apache.hadoop.hdfs.server.namenode.top.TopConf; -import org.apache.hadoop.hdfs.server.namenode.top.window.RollingWindowManager; import org.apache.hadoop.io.nativeio.NativeIO; import org.apache.hadoop.io.nativeio.NativeIO.POSIX.NoMlockCacheManipulator; import org.apache.hadoop.util.VersionInfo; @@ -45,8 +44,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; -import static org.apache.hadoop.hdfs.server.namenode.top.window.RollingWindowManager.Op; -import static org.apache.hadoop.hdfs.server.namenode.top.window.RollingWindowManager.TopWindow; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -196,6 +193,8 @@ public class TestNameNodeMXBean { assertEquals(NativeIO.POSIX.getCacheManipulator().getMemlockLimit() * cluster.getDataNodes().size(), mbs.getAttribute(mxbeanName, "CacheCapacity")); + assertNull("RollingUpgradeInfo should be null when there is no rolling" + + " upgrade", mbs.getAttribute(mxbeanName, "RollingUpgradeStatus")); } finally { if (cluster != null) { for (URI dir : cluster.getNameDirs(0)) {