[hadoop] 02/02: HDFS-16898. Remove write lock for processCommandFromActor of DataNode to reduce impact on heartbeat (#5330). Contributed by ZhangHB.

[hexiaoqiao]

This is an automated email from the ASF dual-hosted git repository.

hexiaoqiao pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git

commit 3ba058a894b14c13651a52219b72eff48d07f4f4
Author: He Xiaoqiao <hexiaoq...@apache.org>
AuthorDate: Wed Feb 8 11:19:07 2023 +0800

    HDFS-16898. Remove write lock for processCommandFromActor of DataNode to 
reduce impact on heartbeat (#5330). Contributed by ZhangHB.
    
    Reviewed-by: zhangshuyan <zqingc...@gmail.com>
    Reviewed-by: Viraj Jasani <vjas...@apache.org>
    Signed-off-by: He Xiaoqiao <hexiaoq...@apache.org>
---
 .../hdfs/server/datanode/BPOfferService.java       | 31 +++++++++++++---------
 .../hdfs/server/datanode/BPServiceActor.java       |  2 +-
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git 
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPOfferService.java
 
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPOfferService.java
index d660970c725..fdd66cb05d3 100644
--- 
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPOfferService.java
+++ 
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPOfferService.java
@@ -679,15 +679,20 @@ class BPOfferService {
       actor.reRegister();
       return false;
     }
-    writeLock();
+    boolean isActiveActor;
+    InetSocketAddress nnSocketAddress;
+    readLock();
     try {
-      if (actor == bpServiceToActive) {
-        return processCommandFromActive(cmd, actor);
-      } else {
-        return processCommandFromStandby(cmd, actor);
-      }
+      isActiveActor = (actor == bpServiceToActive);
+      nnSocketAddress = actor.getNNSocketAddress();
     } finally {
-      writeUnlock();
+      readUnlock();
+    }
+
+    if (isActiveActor) {
+      return processCommandFromActive(cmd, nnSocketAddress);
+    } else {
+      return processCommandFromStandby(cmd, nnSocketAddress);
     }
   }
 
@@ -715,7 +720,7 @@ class BPOfferService {
    * @throws IOException
    */
   private boolean processCommandFromActive(DatanodeCommand cmd,
-      BPServiceActor actor) throws IOException {
+      InetSocketAddress nnSocketAddress) throws IOException {
     final BlockCommand bcmd = 
       cmd instanceof BlockCommand? (BlockCommand)cmd: null;
     final BlockIdCommand blockIdCmd = 
@@ -768,7 +773,7 @@ class BPOfferService {
       dn.finalizeUpgradeForPool(bp);
       break;
     case DatanodeProtocol.DNA_RECOVERBLOCK:
-      String who = "NameNode at " + actor.getNNSocketAddress();
+      String who = "NameNode at " + nnSocketAddress;
       dn.getBlockRecoveryWorker().recoverBlocks(who,
           ((BlockRecoveryCommand)cmd).getRecoveringBlocks());
       break;
@@ -810,11 +815,11 @@ class BPOfferService {
    * DNA_REGISTER which should be handled earlier itself.
    */
   private boolean processCommandFromStandby(DatanodeCommand cmd,
-      BPServiceActor actor) throws IOException {
+      InetSocketAddress nnSocketAddress) throws IOException {
     switch(cmd.getAction()) {
     case DatanodeProtocol.DNA_ACCESSKEYUPDATE:
       LOG.info("DatanodeCommand action from standby NN {}: 
DNA_ACCESSKEYUPDATE",
-          actor.getNNSocketAddress());
+          nnSocketAddress);
       if (dn.isBlockTokenEnabled) {
         dn.blockPoolTokenSecretManager.addKeys(
             getBlockPoolId(), 
@@ -831,11 +836,11 @@ class BPOfferService {
     case DatanodeProtocol.DNA_UNCACHE:
     case DatanodeProtocol.DNA_ERASURE_CODING_RECONSTRUCTION:
       LOG.warn("Got a command from standby NN {} - ignoring command: {}",
-          actor.getNNSocketAddress(), cmd.getAction());
+          nnSocketAddress, cmd.getAction());
       break;
     default:
       LOG.warn("Unknown DatanodeCommand action: {} from standby NN {}",
-          cmd.getAction(), actor.getNNSocketAddress());
+          cmd.getAction(), nnSocketAddress);
     }
     return true;
   }
diff --git 
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPServiceActor.java
 
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPServiceActor.java
index 35ab6193142..e9f424604b4 100755
--- 
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPServiceActor.java
+++ 
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPServiceActor.java
@@ -1499,7 +1499,7 @@ class BPServiceActor implements Runnable {
           dn.getMetrics().addNumProcessedCommands(processCommandsMs);
         }
         if (processCommandsMs > dnConf.getProcessCommandsThresholdMs()) {
-          LOG.info("Took {} ms to process {} commands from NN",
+          LOG.warn("Took {} ms to process {} commands from NN",
               processCommandsMs, cmds.length);
         }
       }


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-commits-h...@hadoop.apache.org