[
https://issues.apache.org/jira/browse/HDFS-16547?focusedWorklogId=792380&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-792380
]
ASF GitHub Bot logged work on HDFS-16547:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 18/Jul/22 22:45
Start Date: 18/Jul/22 22:45
Worklog Time Spent: 10m
Work Description: xkrogen commented on code in PR #4201:
URL: https://github.com/apache/hadoop/pull/4201#discussion_r923907353
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSHAAdmin.java:
##########
@@ -262,8 +262,13 @@ private int transitionToObserver(final CommandLine cmd)
if (!checkManualStateManagementOK(target)) {
return -1;
}
- HAServiceProtocol proto = target.getProxy(getConf(), 0);
- HAServiceProtocolHelper.transitionToObserver(proto, createReqInfo());
+ try {
+ HAServiceProtocol proto = target.getProxy(getConf(), 0);
+ HAServiceProtocolHelper.transitionToObserver(proto, createReqInfo());
+ } catch (IOException e) {
+ errOut.println("TransitionToObserver failed! " + e.getMessage());
Review Comment:
minor nit: `TransitionToObserver` -> `transitionToObserver` for consistency
with other prints e.g. L254
Also `getMessage()` -> `getLocalizedMessage()`
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSHAAdminMiniCluster.java:
##########
@@ -161,7 +164,28 @@ public void testObserverIllegalTransition() throws
Exception {
assertEquals(-1, runTool("-transitionToActive", "nn1"));
assertFalse(nnode1.isActiveState());
}
-
+
+ /**
+ * Tests that a Namenode in safe mode should not be transfer to observer.
+ */
+ @Test
+ public void testObserverTransitionInSafeMode() throws Exception {
+ NameNodeAdapter.enterSafeMode(cluster.getNameNode(0), false);
+ DFSHAAdmin tool = new DFSHAAdmin();
+ tool.setConf(conf);
+ System.setIn(new ByteArrayInputStream("yes\n".getBytes()));
+ int result = tool.run(
+ new String[]{"-transitionToObserver", "-forcemanual", "nn1"});
Review Comment:
minor nit: indentation should be 4 spaces
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestHASafeMode.java:
##########
@@ -977,4 +977,31 @@ public void testTransitionToActiveWhenSafeMode() throws
Exception {
() -> miniCluster.transitionToActive(0));
}
}
+
+ /**
+ * Test transition to observer when namenode in safemode.
+ *
+ * @throws IOException
Review Comment:
this is inaccurate, it throws `Exception`
but this annotation isn't really necessary IMO? it's just a test
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestHASafeMode.java:
##########
@@ -977,4 +977,31 @@ public void testTransitionToActiveWhenSafeMode() throws
Exception {
() -> miniCluster.transitionToActive(0));
}
}
+
+ /**
+ * Test transition to observer when namenode in safemode.
+ *
+ * @throws IOException
+ */
+ @Test
+ public void testTransitionToObserverWhenSafeMode() throws Exception {
+ Configuration config = new Configuration();
+ config.setBoolean(DFS_HA_NN_NOT_BECOME_ACTIVE_IN_SAFEMODE, true);
+ try (MiniDFSCluster miniCluster = new MiniDFSCluster.Builder(config,
+ new File(GenericTestUtils.getRandomizedTempPath()))
+ .nnTopology(MiniDFSNNTopology.simpleHATopology())
+ .numDataNodes(1)
+ .build()) {
+ miniCluster.waitActive();
+ miniCluster.transitionToStandby(0);
+ miniCluster.transitionToStandby(1);
+ NameNode namenode0 = miniCluster.getNameNode(0);
+ NameNode namenode1 = miniCluster.getNameNode(1);
+ NameNodeAdapter.enterSafeMode(namenode0, false);
+ NameNodeAdapter.enterSafeMode(namenode1, false);
+ LambdaTestUtils.intercept(ServiceFailedException.class,
+ "NameNode still not leave safemode",
+ () -> miniCluster.transitionToObserver(0));
Review Comment:
minor nit: indentation should be 4 spaces
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSHAAdminMiniCluster.java:
##########
@@ -161,7 +164,28 @@ public void testObserverIllegalTransition() throws
Exception {
assertEquals(-1, runTool("-transitionToActive", "nn1"));
assertFalse(nnode1.isActiveState());
}
-
+
+ /**
+ * Tests that a Namenode in safe mode should not be transfer to observer.
+ */
+ @Test
+ public void testObserverTransitionInSafeMode() throws Exception {
+ NameNodeAdapter.enterSafeMode(cluster.getNameNode(0), false);
+ DFSHAAdmin tool = new DFSHAAdmin();
+ tool.setConf(conf);
+ System.setIn(new ByteArrayInputStream("yes\n".getBytes()));
+ int result = tool.run(
+ new String[]{"-transitionToObserver", "-forcemanual", "nn1"});
+ assertEquals("State transition returned: " + result, -1, result);
+
+ NameNodeAdapter.leaveSafeMode(cluster.getNameNode(0));
+ System.setIn(new ByteArrayInputStream("yes\n".getBytes()));
+ int result1 = tool.run(
+ new String[]{"-transitionToObserver", "-forcemanual", "nn1"});
Review Comment:
minor nit: indentation should be 4 spaces
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DFSHAAdmin.java:
##########
@@ -262,8 +262,13 @@ private int transitionToObserver(final CommandLine cmd)
if (!checkManualStateManagementOK(target)) {
return -1;
}
- HAServiceProtocol proto = target.getProxy(getConf(), 0);
- HAServiceProtocolHelper.transitionToObserver(proto, createReqInfo());
+ try {
+ HAServiceProtocol proto = target.getProxy(getConf(), 0);
+ HAServiceProtocolHelper.transitionToObserver(proto, createReqInfo());
+ } catch (IOException e) {
Review Comment:
Maybe only catch `ServiceFailedException` here since we only print the
message. If an arbitrary `IOException` occurs, we probably want the additional
context provided by the full stack trace.
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestHASafeMode.java:
##########
@@ -977,4 +977,31 @@ public void testTransitionToActiveWhenSafeMode() throws
Exception {
() -> miniCluster.transitionToActive(0));
}
}
+
+ /**
+ * Test transition to observer when namenode in safemode.
+ *
+ * @throws IOException
+ */
+ @Test
+ public void testTransitionToObserverWhenSafeMode() throws Exception {
+ Configuration config = new Configuration();
+ config.setBoolean(DFS_HA_NN_NOT_BECOME_ACTIVE_IN_SAFEMODE, true);
+ try (MiniDFSCluster miniCluster = new MiniDFSCluster.Builder(config,
+ new File(GenericTestUtils.getRandomizedTempPath()))
+ .nnTopology(MiniDFSNNTopology.simpleHATopology())
+ .numDataNodes(1)
+ .build()) {
Review Comment:
minor nit: indentation should be 4 spaces
Issue Time Tracking
-------------------
Worklog Id: (was: 792380)
Time Spent: 1h 40m (was: 1.5h)
> [SBN read] Namenode in safe mode should not be transfered to observer state
> ---------------------------------------------------------------------------
>
> Key: HDFS-16547
> URL: https://issues.apache.org/jira/browse/HDFS-16547
> Project: Hadoop HDFS
> Issue Type: Improvement
> Reporter: Tao Li
> Assignee: Tao Li
> Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> Currently, when a Namenode is in safemode(under starting or enter safemode
> manually), we can transfer this Namenode to Observer by command. This
> Observer node may receive many requests and then throw a SafemodeException,
> this causes unnecessary failover on the client.
> So Namenode in safe mode should not be transfer to observer state.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
