ayushtkn commented on code in PR #5349:
URL: https://github.com/apache/hadoop/pull/5349#discussion_r1096494121
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java:
##########
@@ -3832,7 +3837,38 @@ public boolean isDatanodeFullyStarted() {
}
return true;
}
-
+
+ /**
+ * Wait for the datanode to be fully started and also connected to active
namenode. This means
+ * wait until the given time duration for all the BP threads to come alive
and all the block
+ * pools to be initialized. Wait until any one of the BP service actor is
connected to active
+ * namenode.
+ *
+ * @param waitTimeMs Wait time in millis for this method to return the
datanode probes. If
+ * datanode stays unhealthy or not connected to any active namenode even
after the given wait
+ * time elapses, it returns false.
+ * @return true - if the data node is fully started and connected to active
namenode within
+ * the given time internal, false otherwise.
+ */
+ public boolean isDatanodeHealthy(long waitTimeMs) {
+ long startTime = monotonicNow();
+ while (monotonicNow() - startTime <= waitTimeMs) {
+ if (isDatanodeFullyStartedAndConnectedToActiveNN()) {
+ return true;
+ }
+ }
+ return false;
+ }
Review Comment:
This waiting is test logic, we should keep it in ``MiniDfsCluster`` only and
can refactor/reuse existing methods as well. Can you trying changing like this:
```
diff --git
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
index 414ab579dd0..ad0f8e8b03e 100644
---
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
+++
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
@@ -3830,39 +3830,12 @@ boolean isRestarting() {
* @return true - if the data node is fully started
*/
public boolean isDatanodeFullyStarted() {
- for (BPOfferService bp : blockPoolManager.getAllNamenodeThreads()) {
- if (!bp.isInitialized() || !bp.isAlive()) {
- return false;
- }
- }
- return true;
- }
-
- /**
- * Wait for the datanode to be fully started and also connected to active
namenode. This means
- * wait until the given time duration for all the BP threads to come
alive and all the block
- * pools to be initialized. Wait until any one of the BP service actor is
connected to active
- * namenode.
- *
- * @param waitTimeMs Wait time in millis for this method to return the
datanode probes. If
- * datanode stays unhealthy or not connected to any active namenode even
after the given wait
- * time elapses, it returns false.
- * @return true - if the data node is fully started and connected to
active namenode within
- * the given time internal, false otherwise.
- */
- public boolean isDatanodeHealthy(long waitTimeMs) {
- long startTime = monotonicNow();
- while (monotonicNow() - startTime <= waitTimeMs) {
- if (isDatanodeFullyStartedAndConnectedToActiveNN()) {
- return true;
- }
- }
- return false;
+ return isDatanodeFullyStarted(false);
}
- private boolean isDatanodeFullyStartedAndConnectedToActiveNN() {
+ public boolean isDatanodeFullyStarted(boolean checkConnectionToActive) {
for (BPOfferService bp : blockPoolManager.getAllNamenodeThreads()) {
- if (!bp.isInitialized() || !bp.isAlive() || bp.getActiveNN() == null)
{
+ if (!bp.isInitialized() || !bp.isAlive() || (checkConnectionToActive
&& bp.getActiveNN() == null)) {
return false;
}
}
diff --git
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java
index dd8bb204382..0576b4a42e1 100644
---
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java
+++
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java
@@ -2529,6 +2529,11 @@ public boolean restartDataNode(DataNodeProperties
dnprop) throws IOException {
return restartDataNode(dnprop, false);
}
+ public void waitDatanodeConnectedToActive(DataNode dn, int timeout)
throws InterruptedException, TimeoutException {
+ GenericTestUtils.waitFor(() -> dn.isDatanodeFullyStarted(true), 100,
timeout,
+ "Datanode is not connected to active even after " + timeout + " ms
of waiting");
+ }
+
public void waitDatanodeFullyStarted(DataNode dn, int timeout)
throws TimeoutException, InterruptedException {
GenericTestUtils.waitFor(dn::isDatanodeFullyStarted, 100, timeout,
diff --git
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMXBean.java
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMXBean.java
index 34fc3558f73..ad4c892b22f 100644
---
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMXBean.java
+++
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMXBean.java
@@ -35,7 +35,6 @@
import org.apache.hadoop.fs.CommonConfigurationKeys;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.ha.HAServiceProtocol;
import org.apache.hadoop.hdfs.DFSConfigKeys;
import org.apache.hadoop.hdfs.DFSTestUtil;
import org.apache.hadoop.hdfs.MiniDFSCluster;
@@ -317,8 +316,7 @@ public void testDataNodeMXBeanLastHeartbeats() throws
Exception {
// Verify and wait until one of the BP service actor identifies
active namenode as active
// and another as standby.
- Assert.assertTrue("Datanode could not be connected to active namenode
in 5s",
- datanode.isDatanodeHealthy(5000));
+ cluster.waitDatanodeConnectedToActive(datanode, 5000);
// Verify that last heartbeat sent to both namenodes in last 5 sec.
assertLastHeartbeatSentTime(datanode, "LastHeartbeat");
```
Can add Javadoc or refactor better, just for the idea sake, Do explore if ``
public void waitActive(int nnIndex) throws IOException {`` can solve the
purpose
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
