You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by ad...@apache.org on 2022/03/14 10:56:05 UTC

[ozone] branch master updated: HDDS-6418. Datanode usage info uses wrong version number (#3173)

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

adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git


The following commit(s) were added to refs/heads/master by this push:
     new bb212de  HDDS-6418. Datanode usage info uses wrong version number (#3173)
bb212de is described below

commit bb212de993fdc15b35ecc2bc10adb0abd24309bd
Author: Doroszlai, Attila <64...@users.noreply.github.com>
AuthorDate: Mon Mar 14 11:55:38 2022 +0100

    HDDS-6418. Datanode usage info uses wrong version number (#3173)
---
 .../protocol/StorageContainerLocationProtocol.java | 11 +++++--
 ...inerLocationProtocolClientSideTranslatorPB.java |  5 ++--
 .../hadoop/hdds/scm/node/DatanodeUsageInfo.java    | 15 +++++-----
 ...inerLocationProtocolServerSideTranslatorPB.java |  9 +++---
 .../hdds/scm/server/SCMClientProtocolServer.java   | 12 ++++----
 .../hdds/scm/cli/ContainerOperationClient.java     |  5 ++--
 .../hadoop/ozone/TestContainerOperations.java      | 35 +++++++++++++++++++++-
 7 files changed, 65 insertions(+), 27 deletions(-)

diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java
index 9f78b31..20a8ada 100644
--- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java
@@ -198,8 +198,9 @@ public interface StorageContainerLocationProtocol extends Closeable {
    *  state acts like a wildcard returning all nodes in that state.
    * @param opState The node operational state
    * @param state The node health
-   * @param clientVersion
+   * @param clientVersion Client's version number
    * @return List of Datanodes.
+   * @see org.apache.hadoop.ozone.ClientVersions
    */
   List<HddsProtos.Node> queryNode(HddsProtos.NodeOperationalState opState,
       HddsProtos.NodeState state, HddsProtos.QueryScope queryScope,
@@ -353,24 +354,28 @@ public interface StorageContainerLocationProtocol extends Closeable {
    *
    * @param ipaddress datanode IP address String
    * @param uuid datanode UUID String
+   * @param clientVersion Client's version number
    * @return List of DatanodeUsageInfoProto. Each element contains info such as
    * capacity, SCMused, and remaining space.
    * @throws IOException
+   * @see org.apache.hadoop.ozone.ClientVersions
    */
   List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
-      String ipaddress, String uuid) throws IOException;
+      String ipaddress, String uuid, int clientVersion) throws IOException;
 
   /**
    * Get usage information of most or least used datanodes.
    *
    * @param mostUsed true if most used, false if least used
    * @param count Integer number of nodes to get info for
+   * @param clientVersion Client's version number
    * @return List of DatanodeUsageInfoProto. Each element contains info such as
    * capacity, SCMUsed, and remaining space.
    * @throws IOException
+   * @see org.apache.hadoop.ozone.ClientVersions
    */
   List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
-      boolean mostUsed, int count) throws IOException;
+      boolean mostUsed, int count, int clientVersion) throws IOException;
 
   StatusAndMessages finalizeScmUpgrade(String upgradeClientID)
       throws IOException;
diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java
index a27149d..e1c19f7 100644
--- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java
+++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java
@@ -425,7 +425,6 @@ public final class StorageContainerLocationProtocolClientSideTranslatorPB
    *
    * @param opState The operation state of the node
    * @param nodeState The health of the node
-   * @param clientVersion
    * @return List of Datanodes.
    */
   @Override
@@ -874,7 +873,7 @@ public final class StorageContainerLocationProtocolClientSideTranslatorPB
    */
   @Override
   public List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
-      String ipaddress, String uuid) throws IOException {
+      String ipaddress, String uuid, int clientVersion) throws IOException {
 
     DatanodeUsageInfoRequestProto request =
         DatanodeUsageInfoRequestProto.newBuilder()
@@ -900,7 +899,7 @@ public final class StorageContainerLocationProtocolClientSideTranslatorPB
    */
   @Override
   public List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
-      boolean mostUsed, int count) throws IOException {
+      boolean mostUsed, int count, int clientVersion) throws IOException {
     DatanodeUsageInfoRequestProto request =
         DatanodeUsageInfoRequestProto.newBuilder()
             .setMostUsed(mostUsed)
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeUsageInfo.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeUsageInfo.java
index fd320ea..8de0a41 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeUsageInfo.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeUsageInfo.java
@@ -19,7 +19,7 @@
 package org.apache.hadoop.hdds.scm.node;
 
 import org.apache.hadoop.hdds.protocol.DatanodeDetails;
-import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.DatanodeUsageInfoProto;
 import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeStat;
 
 import java.util.Comparator;
@@ -180,17 +180,16 @@ public class DatanodeUsageInfo {
    *
    * @return Protobuf HddsProtos.DatanodeUsageInfo
    */
-  public HddsProtos.DatanodeUsageInfoProto toProto() {
-    return toProtoBuilder().build();
+  public DatanodeUsageInfoProto toProto(int clientVersion) {
+    return toProtoBuilder(clientVersion).build();
   }
 
-  private HddsProtos.DatanodeUsageInfoProto.Builder toProtoBuilder() {
-    HddsProtos.DatanodeUsageInfoProto.Builder builder =
-        HddsProtos.DatanodeUsageInfoProto.newBuilder();
+  private DatanodeUsageInfoProto.Builder toProtoBuilder(int clientVersion) {
+    DatanodeUsageInfoProto.Builder builder =
+        DatanodeUsageInfoProto.newBuilder();
 
     if (datanodeDetails != null) {
-      builder.setNode(
-          datanodeDetails.toProto(datanodeDetails.getCurrentVersion()));
+      builder.setNode(datanodeDetails.toProto(clientVersion));
     }
     if (scmNodeStat != null) {
       builder.setCapacity(scmNodeStat.getCapacity().get());
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java
index 5d6ee5b..d3f5e8c 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java
@@ -407,7 +407,8 @@ public final class StorageContainerLocationProtocolServerSideTranslatorPB
             .setCmdType(request.getCmdType())
             .setStatus(Status.OK)
             .setDatanodeUsageInfoResponse(getDatanodeUsageInfo(
-                request.getDatanodeUsageInfoRequest()))
+                request.getDatanodeUsageInfoRequest(),
+                request.getVersion()))
             .build();
       case GetContainerCount:
         return ScmContainerLocationResponse.newBuilder()
@@ -864,16 +865,16 @@ public final class StorageContainerLocationProtocolServerSideTranslatorPB
 
   public DatanodeUsageInfoResponseProto getDatanodeUsageInfo(
       StorageContainerLocationProtocolProtos.DatanodeUsageInfoRequestProto
-      request) throws IOException {
+          request, int clientVersion) throws IOException {
     List<HddsProtos.DatanodeUsageInfoProto> infoList;
 
     // get info by ip or uuid
     if (request.hasUuid() || request.hasIpaddress()) {
       infoList = impl.getDatanodeUsageInfo(request.getIpaddress(),
-          request.getUuid());
+          request.getUuid(), clientVersion);
     } else {  // get most or least used nodes
       infoList = impl.getDatanodeUsageInfo(request.getMostUsed(),
-          request.getCount());
+          request.getCount(), clientVersion);
     }
 
     return DatanodeUsageInfoResponseProto.newBuilder()
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
index 7570d98..51daea9 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
@@ -869,7 +869,7 @@ public class SCMClientProtocolServer implements
    */
   @Override
   public List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
-      String ipaddress, String uuid) throws IOException {
+      String ipaddress, String uuid, int clientVersion) throws IOException {
 
     // check admin authorisation
     try {
@@ -894,7 +894,7 @@ public class SCMClientProtocolServer implements
     // get datanode usage info
     List<HddsProtos.DatanodeUsageInfoProto> infoList = new ArrayList<>();
     for (DatanodeDetails node : nodes) {
-      infoList.add(getUsageInfoFromDatanodeDetails(node));
+      infoList.add(getUsageInfoFromDatanodeDetails(node, clientVersion));
     }
 
     return infoList;
@@ -907,7 +907,7 @@ public class SCMClientProtocolServer implements
    * @return Usage info such as capacity, SCMUsed, and remaining space.
    */
   private HddsProtos.DatanodeUsageInfoProto getUsageInfoFromDatanodeDetails(
-      DatanodeDetails node) {
+      DatanodeDetails node, int clientVersion) {
     SCMNodeStat stat = scm.getScmNodeManager().getNodeStat(node).get();
 
     long capacity = stat.getCapacity().get();
@@ -918,7 +918,7 @@ public class SCMClientProtocolServer implements
         .setCapacity(capacity)
         .setUsed(used)
         .setRemaining(remaining)
-        .setNode(node.toProto(node.getCurrentVersion()))
+        .setNode(node.toProto(clientVersion))
         .build();
   }
 
@@ -936,7 +936,7 @@ public class SCMClientProtocolServer implements
    */
   @Override
   public List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
-      boolean mostUsed, int count)
+      boolean mostUsed, int count, int clientVersion)
       throws IOException, IllegalArgumentException {
 
     // check admin authorisation
@@ -963,7 +963,7 @@ public class SCMClientProtocolServer implements
 
     // return count number of DatanodeUsageInfoProto
     return datanodeUsageInfoList.stream()
-        .map(DatanodeUsageInfo::toProto)
+        .map(each -> each.toProto(clientVersion))
         .limit(count)
         .collect(Collectors.toList());
   }
diff --git a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java
index fd7aa04..fd326f8 100644
--- a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java
+++ b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java
@@ -620,7 +620,7 @@ public class ContainerOperationClient implements ScmClient {
   public List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
       String ipaddress, String uuid) throws IOException {
     return storageContainerLocationClient.getDatanodeUsageInfo(ipaddress,
-        uuid);
+        uuid, ClientVersion.CURRENT_VERSION);
   }
 
   /**
@@ -635,7 +635,8 @@ public class ContainerOperationClient implements ScmClient {
   @Override
   public List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
       boolean mostUsed, int count) throws IOException {
-    return storageContainerLocationClient.getDatanodeUsageInfo(mostUsed, count);
+    return storageContainerLocationClient.getDatanodeUsageInfo(mostUsed, count,
+        ClientVersion.CURRENT_VERSION);
   }
 
   @Override
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerOperations.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerOperations.java
index e15d7b1..708bbc0 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerOperations.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerOperations.java
@@ -18,6 +18,7 @@
 package org.apache.hadoop.ozone;
 
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
 import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
 import org.apache.hadoop.hdds.scm.PlacementPolicy;
@@ -34,7 +35,13 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.Rule;
 import org.junit.rules.Timeout;
+
+import java.io.IOException;
+import java.util.List;
+
+import static org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name.REPLICATION;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * This class tests container operations (TODO currently only supports create)
@@ -93,10 +100,36 @@ public class TestContainerOperations {
       storageClient.getPipeline(PipelineID.randomId().getProtobuf());
       Assert.fail("Get Pipeline should fail");
     } catch (Exception e) {
-      Assert.assertTrue(
+      assertTrue(
           SCMHAUtils.unwrapException(e) instanceof PipelineNotFoundException);
     }
 
     Assert.assertFalse(storageClient.listPipelines().isEmpty());
   }
+
+  @Test
+  public void testDatanodeUsageInfoCompatibility() throws IOException {
+    DatanodeDetails dn = cluster.getStorageContainerManager()
+        .getScmNodeManager()
+        .getAllNodes()
+        .get(0);
+    dn.setCurrentVersion(0);
+
+    List<HddsProtos.DatanodeUsageInfoProto> usageInfoList =
+        storageClient.getDatanodeUsageInfo(
+            dn.getIpAddress(), dn.getUuidString());
+
+    for (HddsProtos.DatanodeUsageInfoProto info : usageInfoList) {
+      assertTrue(info.getNode().getPortsList().stream()
+          .anyMatch(port -> REPLICATION.name().equals(port.getName())));
+    }
+
+    usageInfoList =
+        storageClient.getDatanodeUsageInfo(true, 3);
+
+    for (HddsProtos.DatanodeUsageInfoProto info : usageInfoList) {
+      assertTrue(info.getNode().getPortsList().stream()
+          .anyMatch(port -> REPLICATION.name().equals(port.getName())));
+    }
+  }
 }

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org