You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/03/02 13:26:57 UTC
[GitHub] [ozone] siddhantsangwan opened a new pull request #1982: Hdds 4872. Upgrade UsageInfoSubcommand with options to show most and least used datanodes.
siddhantsangwan opened a new pull request #1982:
URL: https://github.com/apache/ozone/pull/1982
## What changes were proposed in this pull request?
Introduce `--most-used` and `--least-used` options in `UsageInfoSubcommand` to get the usage info of most used and least used datanodes, respectively. `--count` option can be used to specify number of nodes along with `--most-used` or `--least-used`.
All options except `--count` are now exclusive.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4872
## How was this patch tested?
Tested using docker-compose. Screenshots have been attached below.
<img width="798" alt="test1" src="https://user-images.githubusercontent.com/34305492/109654699-827e0e00-7b88-11eb-91f5-eef3fe050d4b.png">
<img width="945" alt="test2" src="https://user-images.githubusercontent.com/34305492/109654728-8ad64900-7b88-11eb-98b5-2c2ac3544a3a.png">
<img width="945" alt="test3" src="https://user-images.githubusercontent.com/34305492/109654772-96c20b00-7b88-11eb-9c75-68f8b45d2c0d.png">
<img width="945" alt="image" src="https://user-images.githubusercontent.com/34305492/109654829-ab9e9e80-7b88-11eb-8ac8-dbab46b94a7a.png">
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] siddhantsangwan commented on a change in pull request #1982: HDDS-4872. Upgrade UsageInfoSubcommand with options to show most and least used datanodes.
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #1982:
URL: https://github.com/apache/ozone/pull/1982#discussion_r589300046
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
##########
@@ -678,13 +680,111 @@ public boolean getReplicationManagerStatus() {
long used = stat.getScmUsed().get();
long remaining = stat.getRemaining().get();
- HddsProtos.DatanodeUsageInfo info = HddsProtos.DatanodeUsageInfo
- .newBuilder()
+ return HddsProtos.DatanodeUsageInfo.newBuilder()
.setCapacity(capacity)
.setUsed(used)
.setRemaining(remaining)
+ .setNode(node.toProto(node.getCurrentVersion()))
.build();
- return info;
+ }
+
+ /**
+ * Get information of the 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
+ * @return List of DatanodeUsageInfo. Each element contains usage info
+ * such as capacity, SCMUsed, and remaining space.
+ * @throws IOException
+ */
+ @Override
+ public List<HddsProtos.DatanodeUsageInfo> getDatanodeUsageInfo(
+ boolean mostUsed, int count) throws IOException {
+
+ // check admin authorisation
+ String remoteUser = getRpcRemoteUsername();
+ try {
+ getScm().checkAdminAccess(remoteUser);
+ } catch (IOException e) {
+ LOG.error("Authorisation failed", e);
+ throw e;
+ }
+
+ PriorityQueue<DatanodeDetails> nodes = null;
+ if (mostUsed) {
+ nodes = getMostUsedDatanodes(count);
+ } else {
+ nodes = getLeastUsedDatanodes(count);
+ }
+
+ List<HddsProtos.DatanodeUsageInfo> infoList = new ArrayList<>();
+ for (DatanodeDetails node : nodes) {
+ infoList.add(getUsageInfoFromDatanodeDetails(node));
+ }
+ return infoList;
+ }
+
+ /**
+ * Get most used datanodes.
+ *
+ * @param count Integer number of datanodes to get. Must be greater than
+ * zero and lesser than total (healthy + stale) number of nodes
+ * @return PriorityQueue of DatanodeDetails
+ * @throws IllegalArgumentException
+ */
+ private PriorityQueue<DatanodeDetails> getMostUsedDatanodes(int count)
+ throws IllegalArgumentException {
+ Map<DatanodeDetails, SCMNodeStat> nodeStatMap = scm.getScmNodeManager()
+ .getNodeStats();
+
+ if (count > nodeStatMap.size()) {
+ throw new IllegalArgumentException(
+ String.format("count must be lesser than %d", nodeStatMap.size()));
+ }
+ PriorityQueue<DatanodeDetails> nodes = new PriorityQueue<>(count,
+ Comparator.comparingLong(node -> nodeStatMap.get(node)
+ .getRemaining().get()).reversed());
Review comment:
Since we only need count number of nodes, I think we could avoid sorting the entire map? Sorting could mean extra work since it will also involve ordering the remaining `size - k` nodes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] siddhantsangwan commented on a change in pull request #1982: HDDS-4872. Upgrade UsageInfoSubcommand with options to show most and least used datanodes.
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #1982:
URL: https://github.com/apache/ozone/pull/1982#discussion_r598437626
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeUsageInfo.java
##########
@@ -0,0 +1,172 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+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.scm.container.placement.metrics.SCMNodeStat;
+
+import java.util.Comparator;
+
+public class DatanodeUsageInfo {
+
+ private DatanodeDetails datanodeDetails;
+ private SCMNodeStat scmNodeStat;
+
+ /**
+ * Constructs a DatanodeUsageInfo with DatanodeDetails and SCMNodeStat.
+ *
+ * @param datanodeDetails DatanodeDetails
+ * @param scmNodeStat SCMNodeStat
+ */
+ public DatanodeUsageInfo(
+ DatanodeDetails datanodeDetails,
+ SCMNodeStat scmNodeStat) {
+ this.datanodeDetails = datanodeDetails;
+ this.scmNodeStat = scmNodeStat;
+ }
+
+ /**
+ * Compares two DatanodeUsageInfo on the basis of used space.
+ *
+ * @param first DatanodeUsageInfo
+ * @param second DatanodeUsageInfo
+ * @return a value greater than 0 if first is more used, a value
+ * lesser than 0 if second is more used, and 0 if both are equally used or
+ * first.equals(second) is true
+ */
+ private static int compare(DatanodeUsageInfo first,
Review comment:
Yes, noted.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lokeshj1703 commented on a change in pull request #1982: HDDS-4872. Upgrade UsageInfoSubcommand with options to show most and least used datanodes.
Posted by GitBox <gi...@apache.org>.
lokeshj1703 commented on a change in pull request #1982:
URL: https://github.com/apache/ozone/pull/1982#discussion_r589188205
##########
File path: hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/UsageInfoSubcommand.java
##########
@@ -41,58 +41,66 @@
versionProvider = HddsVersionProvider.class)
public class UsageInfoSubcommand extends ScmSubcommand {
- @CommandLine.Option(names = {"--ip"}, paramLabel = "IP", description =
- "Show info by datanode ip address")
- private String ipaddress;
+ @CommandLine.ArgGroup(exclusive = true, multiplicity = "1")
+ private ExclusiveArguments exclusiveArguments;
- @CommandLine.Option(names = {"--uuid"}, paramLabel = "UUID", description =
- "Show info by datanode UUID")
- private String uuid;
+ static class ExclusiveArguments {
Review comment:
We can make the class private.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
##########
@@ -678,13 +680,111 @@ public boolean getReplicationManagerStatus() {
long used = stat.getScmUsed().get();
long remaining = stat.getRemaining().get();
- HddsProtos.DatanodeUsageInfo info = HddsProtos.DatanodeUsageInfo
- .newBuilder()
+ return HddsProtos.DatanodeUsageInfo.newBuilder()
.setCapacity(capacity)
.setUsed(used)
.setRemaining(remaining)
+ .setNode(node.toProto(node.getCurrentVersion()))
.build();
- return info;
+ }
+
+ /**
+ * Get information of the 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
+ * @return List of DatanodeUsageInfo. Each element contains usage info
+ * such as capacity, SCMUsed, and remaining space.
+ * @throws IOException
+ */
+ @Override
+ public List<HddsProtos.DatanodeUsageInfo> getDatanodeUsageInfo(
+ boolean mostUsed, int count) throws IOException {
+
+ // check admin authorisation
+ String remoteUser = getRpcRemoteUsername();
+ try {
+ getScm().checkAdminAccess(remoteUser);
+ } catch (IOException e) {
+ LOG.error("Authorisation failed", e);
+ throw e;
+ }
+
+ PriorityQueue<DatanodeDetails> nodes = null;
+ if (mostUsed) {
+ nodes = getMostUsedDatanodes(count);
+ } else {
+ nodes = getLeastUsedDatanodes(count);
+ }
+
+ List<HddsProtos.DatanodeUsageInfo> infoList = new ArrayList<>();
+ for (DatanodeDetails node : nodes) {
+ infoList.add(getUsageInfoFromDatanodeDetails(node));
+ }
+ return infoList;
+ }
+
+ /**
+ * Get most used datanodes.
+ *
+ * @param count Integer number of datanodes to get. Must be greater than
+ * zero and lesser than total (healthy + stale) number of nodes
+ * @return PriorityQueue of DatanodeDetails
+ * @throws IllegalArgumentException
+ */
+ private PriorityQueue<DatanodeDetails> getMostUsedDatanodes(int count)
+ throws IllegalArgumentException {
+ Map<DatanodeDetails, SCMNodeStat> nodeStatMap = scm.getScmNodeManager()
+ .getNodeStats();
+
+ if (count > nodeStatMap.size()) {
+ throw new IllegalArgumentException(
+ String.format("count must be lesser than %d", nodeStatMap.size()));
+ }
+ PriorityQueue<DatanodeDetails> nodes = new PriorityQueue<>(count,
+ Comparator.comparingLong(node -> nodeStatMap.get(node)
+ .getRemaining().get()).reversed());
Review comment:
We could also use sorting by value, please check https://dzone.com/articles/how-to-sort-a-map-by-value-in-java-8.
Also comparator should calculate the percentage free(remaining/capacity) and then return most and least used.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
##########
@@ -678,13 +680,111 @@ public boolean getReplicationManagerStatus() {
long used = stat.getScmUsed().get();
long remaining = stat.getRemaining().get();
- HddsProtos.DatanodeUsageInfo info = HddsProtos.DatanodeUsageInfo
- .newBuilder()
+ return HddsProtos.DatanodeUsageInfo.newBuilder()
.setCapacity(capacity)
.setUsed(used)
.setRemaining(remaining)
+ .setNode(node.toProto(node.getCurrentVersion()))
.build();
- return info;
+ }
+
+ /**
+ * Get information of the 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
+ * @return List of DatanodeUsageInfo. Each element contains usage info
+ * such as capacity, SCMUsed, and remaining space.
+ * @throws IOException
+ */
+ @Override
+ public List<HddsProtos.DatanodeUsageInfo> getDatanodeUsageInfo(
+ boolean mostUsed, int count) throws IOException {
+
+ // check admin authorisation
+ String remoteUser = getRpcRemoteUsername();
+ try {
+ getScm().checkAdminAccess(remoteUser);
+ } catch (IOException e) {
+ LOG.error("Authorisation failed", e);
+ throw e;
+ }
+
+ PriorityQueue<DatanodeDetails> nodes = null;
+ if (mostUsed) {
+ nodes = getMostUsedDatanodes(count);
+ } else {
+ nodes = getLeastUsedDatanodes(count);
+ }
+
+ List<HddsProtos.DatanodeUsageInfo> infoList = new ArrayList<>();
+ for (DatanodeDetails node : nodes) {
+ infoList.add(getUsageInfoFromDatanodeDetails(node));
Review comment:
This would lead to two SCM calls for getting SCMNodeStat. Since we already have stat in getMostUsedDatanodes function, we should use it to return a list of DatanodeUsageInfo from it.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
##########
@@ -678,13 +680,111 @@ public boolean getReplicationManagerStatus() {
long used = stat.getScmUsed().get();
long remaining = stat.getRemaining().get();
- HddsProtos.DatanodeUsageInfo info = HddsProtos.DatanodeUsageInfo
- .newBuilder()
+ return HddsProtos.DatanodeUsageInfo.newBuilder()
.setCapacity(capacity)
.setUsed(used)
.setRemaining(remaining)
+ .setNode(node.toProto(node.getCurrentVersion()))
.build();
- return info;
+ }
+
+ /**
+ * Get information of the 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
+ * @return List of DatanodeUsageInfo. Each element contains usage info
+ * such as capacity, SCMUsed, and remaining space.
+ * @throws IOException
+ */
+ @Override
+ public List<HddsProtos.DatanodeUsageInfo> getDatanodeUsageInfo(
+ boolean mostUsed, int count) throws IOException {
+
+ // check admin authorisation
+ String remoteUser = getRpcRemoteUsername();
+ try {
+ getScm().checkAdminAccess(remoteUser);
+ } catch (IOException e) {
+ LOG.error("Authorisation failed", e);
+ throw e;
+ }
+
+ PriorityQueue<DatanodeDetails> nodes = null;
+ if (mostUsed) {
+ nodes = getMostUsedDatanodes(count);
+ } else {
+ nodes = getLeastUsedDatanodes(count);
Review comment:
Let's have one function for getting most or least used datanodes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] siddhantsangwan commented on a change in pull request #1982: HDDS-4872. Upgrade UsageInfoSubcommand with options to show most and least used datanodes.
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #1982:
URL: https://github.com/apache/ozone/pull/1982#discussion_r589300046
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
##########
@@ -678,13 +680,111 @@ public boolean getReplicationManagerStatus() {
long used = stat.getScmUsed().get();
long remaining = stat.getRemaining().get();
- HddsProtos.DatanodeUsageInfo info = HddsProtos.DatanodeUsageInfo
- .newBuilder()
+ return HddsProtos.DatanodeUsageInfo.newBuilder()
.setCapacity(capacity)
.setUsed(used)
.setRemaining(remaining)
+ .setNode(node.toProto(node.getCurrentVersion()))
.build();
- return info;
+ }
+
+ /**
+ * Get information of the 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
+ * @return List of DatanodeUsageInfo. Each element contains usage info
+ * such as capacity, SCMUsed, and remaining space.
+ * @throws IOException
+ */
+ @Override
+ public List<HddsProtos.DatanodeUsageInfo> getDatanodeUsageInfo(
+ boolean mostUsed, int count) throws IOException {
+
+ // check admin authorisation
+ String remoteUser = getRpcRemoteUsername();
+ try {
+ getScm().checkAdminAccess(remoteUser);
+ } catch (IOException e) {
+ LOG.error("Authorisation failed", e);
+ throw e;
+ }
+
+ PriorityQueue<DatanodeDetails> nodes = null;
+ if (mostUsed) {
+ nodes = getMostUsedDatanodes(count);
+ } else {
+ nodes = getLeastUsedDatanodes(count);
+ }
+
+ List<HddsProtos.DatanodeUsageInfo> infoList = new ArrayList<>();
+ for (DatanodeDetails node : nodes) {
+ infoList.add(getUsageInfoFromDatanodeDetails(node));
+ }
+ return infoList;
+ }
+
+ /**
+ * Get most used datanodes.
+ *
+ * @param count Integer number of datanodes to get. Must be greater than
+ * zero and lesser than total (healthy + stale) number of nodes
+ * @return PriorityQueue of DatanodeDetails
+ * @throws IllegalArgumentException
+ */
+ private PriorityQueue<DatanodeDetails> getMostUsedDatanodes(int count)
+ throws IllegalArgumentException {
+ Map<DatanodeDetails, SCMNodeStat> nodeStatMap = scm.getScmNodeManager()
+ .getNodeStats();
+
+ if (count > nodeStatMap.size()) {
+ throw new IllegalArgumentException(
+ String.format("count must be lesser than %d", nodeStatMap.size()));
+ }
+ PriorityQueue<DatanodeDetails> nodes = new PriorityQueue<>(count,
+ Comparator.comparingLong(node -> nodeStatMap.get(node)
+ .getRemaining().get()).reversed());
Review comment:
Since we only need count number of nodes, I think we could avoid sorting the entire map? Sorting could mean extra work since it will also involve ordering the remaining size - k nodes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lokeshj1703 commented on pull request #1982: HDDS-4872. Upgrade UsageInfoSubcommand with options to show most and least used datanodes.
Posted by GitBox <gi...@apache.org>.
lokeshj1703 commented on pull request #1982:
URL: https://github.com/apache/ozone/pull/1982#issuecomment-805617951
@siddhantsangwan Thanks for the contribution! I have committed the PR to master branch.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] siddhantsangwan commented on a change in pull request #1982: HDDS-4872. Upgrade UsageInfoSubcommand with options to show most and least used datanodes.
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #1982:
URL: https://github.com/apache/ozone/pull/1982#discussion_r598437346
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeUsageInfo.java
##########
@@ -0,0 +1,172 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+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.scm.container.placement.metrics.SCMNodeStat;
+
+import java.util.Comparator;
+
+public class DatanodeUsageInfo {
+
+ private DatanodeDetails datanodeDetails;
+ private SCMNodeStat scmNodeStat;
+
+ /**
+ * Constructs a DatanodeUsageInfo with DatanodeDetails and SCMNodeStat.
+ *
+ * @param datanodeDetails DatanodeDetails
+ * @param scmNodeStat SCMNodeStat
+ */
+ public DatanodeUsageInfo(
+ DatanodeDetails datanodeDetails,
+ SCMNodeStat scmNodeStat) {
+ this.datanodeDetails = datanodeDetails;
+ this.scmNodeStat = scmNodeStat;
+ }
+
+ /**
+ * Compares two DatanodeUsageInfo on the basis of used space.
+ *
+ * @param first DatanodeUsageInfo
+ * @param second DatanodeUsageInfo
+ * @return a value greater than 0 if first is more used, a value
+ * lesser than 0 if second is more used, and 0 if both are equally used or
+ * first.equals(second) is true
+ */
+ private static int compare(DatanodeUsageInfo first,
+ DatanodeUsageInfo second) {
+ if (first.equals(second)) {
+ return 0;
+ }
+ double remaining = first.scmNodeStat.getRemaining().get().doubleValue();
+ double capacity = first.scmNodeStat.getCapacity().get().doubleValue();
+ double ratioRemaining = remaining / capacity;
Review comment:
Thanks for pointing this out. `SCMNodeMetric` already has comparison functions, I could use those.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lokeshj1703 closed pull request #1982: HDDS-4872. Upgrade UsageInfoSubcommand with options to show most and least used datanodes.
Posted by GitBox <gi...@apache.org>.
lokeshj1703 closed pull request #1982:
URL: https://github.com/apache/ozone/pull/1982
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lokeshj1703 commented on a change in pull request #1982: HDDS-4872. Upgrade UsageInfoSubcommand with options to show most and least used datanodes.
Posted by GitBox <gi...@apache.org>.
lokeshj1703 commented on a change in pull request #1982:
URL: https://github.com/apache/ozone/pull/1982#discussion_r589273830
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
##########
@@ -678,13 +680,111 @@ public boolean getReplicationManagerStatus() {
long used = stat.getScmUsed().get();
long remaining = stat.getRemaining().get();
- HddsProtos.DatanodeUsageInfo info = HddsProtos.DatanodeUsageInfo
- .newBuilder()
+ return HddsProtos.DatanodeUsageInfo.newBuilder()
.setCapacity(capacity)
.setUsed(used)
.setRemaining(remaining)
+ .setNode(node.toProto(node.getCurrentVersion()))
.build();
- return info;
+ }
+
+ /**
+ * Get information of the 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
+ * @return List of DatanodeUsageInfo. Each element contains usage info
+ * such as capacity, SCMUsed, and remaining space.
+ * @throws IOException
+ */
+ @Override
+ public List<HddsProtos.DatanodeUsageInfo> getDatanodeUsageInfo(
+ boolean mostUsed, int count) throws IOException {
+
+ // check admin authorisation
+ String remoteUser = getRpcRemoteUsername();
+ try {
+ getScm().checkAdminAccess(remoteUser);
+ } catch (IOException e) {
+ LOG.error("Authorisation failed", e);
+ throw e;
+ }
+
+ PriorityQueue<DatanodeDetails> nodes = null;
+ if (mostUsed) {
+ nodes = getMostUsedDatanodes(count);
+ } else {
+ nodes = getLeastUsedDatanodes(count);
+ }
+
+ List<HddsProtos.DatanodeUsageInfo> infoList = new ArrayList<>();
+ for (DatanodeDetails node : nodes) {
+ infoList.add(getUsageInfoFromDatanodeDetails(node));
Review comment:
DatanodeUsageInfo would have the datanode details so I guess it would be ok to return its list. 2nd option looks good.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] siddhantsangwan commented on a change in pull request #1982: HDDS-4872. Upgrade UsageInfoSubcommand with options to show most and least used datanodes.
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #1982:
URL: https://github.com/apache/ozone/pull/1982#discussion_r589241505
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
##########
@@ -678,13 +680,111 @@ public boolean getReplicationManagerStatus() {
long used = stat.getScmUsed().get();
long remaining = stat.getRemaining().get();
- HddsProtos.DatanodeUsageInfo info = HddsProtos.DatanodeUsageInfo
- .newBuilder()
+ return HddsProtos.DatanodeUsageInfo.newBuilder()
.setCapacity(capacity)
.setUsed(used)
.setRemaining(remaining)
+ .setNode(node.toProto(node.getCurrentVersion()))
.build();
- return info;
+ }
+
+ /**
+ * Get information of the 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
+ * @return List of DatanodeUsageInfo. Each element contains usage info
+ * such as capacity, SCMUsed, and remaining space.
+ * @throws IOException
+ */
+ @Override
+ public List<HddsProtos.DatanodeUsageInfo> getDatanodeUsageInfo(
+ boolean mostUsed, int count) throws IOException {
+
+ // check admin authorisation
+ String remoteUser = getRpcRemoteUsername();
+ try {
+ getScm().checkAdminAccess(remoteUser);
+ } catch (IOException e) {
+ LOG.error("Authorisation failed", e);
+ throw e;
+ }
+
+ PriorityQueue<DatanodeDetails> nodes = null;
+ if (mostUsed) {
+ nodes = getMostUsedDatanodes(count);
+ } else {
+ nodes = getLeastUsedDatanodes(count);
+ }
+
+ List<HddsProtos.DatanodeUsageInfo> infoList = new ArrayList<>();
+ for (DatanodeDetails node : nodes) {
+ infoList.add(getUsageInfoFromDatanodeDetails(node));
Review comment:
When writing `getMostUsedDatanodes` and `getLeastUsedDatanodes`, I intended their purpose to be returning the most used and least used datanodes respectively so that methods higher up in the method call can do whatever they want with that information.
So do you suggest adding another parameter `NodeManager` to `getUsageInfoFromDatanodeDetails` and checking that parameter for null? If it's null, I can call `scm.getNodeManager` there.
OR
Return a list of DatanodeUsageInfo from `getMostUsedDatanodes` (can be refactored to `getMostOrLeastUsedDatanodes`)?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] lokeshj1703 commented on a change in pull request #1982: HDDS-4872. Upgrade UsageInfoSubcommand with options to show most and least used datanodes.
Posted by GitBox <gi...@apache.org>.
lokeshj1703 commented on a change in pull request #1982:
URL: https://github.com/apache/ozone/pull/1982#discussion_r597505555
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeUsageInfo.java
##########
@@ -0,0 +1,172 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+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.scm.container.placement.metrics.SCMNodeStat;
+
+import java.util.Comparator;
+
+public class DatanodeUsageInfo {
+
+ private DatanodeDetails datanodeDetails;
+ private SCMNodeStat scmNodeStat;
+
+ /**
+ * Constructs a DatanodeUsageInfo with DatanodeDetails and SCMNodeStat.
+ *
+ * @param datanodeDetails DatanodeDetails
+ * @param scmNodeStat SCMNodeStat
+ */
+ public DatanodeUsageInfo(
+ DatanodeDetails datanodeDetails,
+ SCMNodeStat scmNodeStat) {
+ this.datanodeDetails = datanodeDetails;
+ this.scmNodeStat = scmNodeStat;
+ }
+
+ /**
+ * Compares two DatanodeUsageInfo on the basis of used space.
+ *
+ * @param first DatanodeUsageInfo
+ * @param second DatanodeUsageInfo
+ * @return a value greater than 0 if first is more used, a value
+ * lesser than 0 if second is more used, and 0 if both are equally used or
+ * first.equals(second) is true
+ */
+ private static int compare(DatanodeUsageInfo first,
+ DatanodeUsageInfo second) {
+ if (first.equals(second)) {
+ return 0;
+ }
+ double remaining = first.scmNodeStat.getRemaining().get().doubleValue();
+ double capacity = first.scmNodeStat.getCapacity().get().doubleValue();
+ double ratioRemaining = remaining / capacity;
Review comment:
Lets move the SCMNodeStat comparison to its own class and use the corresponding fn here.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeUsageInfo.java
##########
@@ -0,0 +1,172 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+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.scm.container.placement.metrics.SCMNodeStat;
+
+import java.util.Comparator;
+
+public class DatanodeUsageInfo {
+
+ private DatanodeDetails datanodeDetails;
+ private SCMNodeStat scmNodeStat;
+
+ /**
+ * Constructs a DatanodeUsageInfo with DatanodeDetails and SCMNodeStat.
+ *
+ * @param datanodeDetails DatanodeDetails
+ * @param scmNodeStat SCMNodeStat
+ */
+ public DatanodeUsageInfo(
+ DatanodeDetails datanodeDetails,
+ SCMNodeStat scmNodeStat) {
+ this.datanodeDetails = datanodeDetails;
+ this.scmNodeStat = scmNodeStat;
+ }
+
+ /**
+ * Compares two DatanodeUsageInfo on the basis of used space.
+ *
+ * @param first DatanodeUsageInfo
+ * @param second DatanodeUsageInfo
+ * @return a value greater than 0 if first is more used, a value
+ * lesser than 0 if second is more used, and 0 if both are equally used or
+ * first.equals(second) is true
+ */
+ private static int compare(DatanodeUsageInfo first,
+ DatanodeUsageInfo second) {
+ if (first.equals(second)) {
+ return 0;
+ }
+ double remaining = first.scmNodeStat.getRemaining().get().doubleValue();
+ double capacity = first.scmNodeStat.getCapacity().get().doubleValue();
+ double ratioRemaining = remaining / capacity;
+
+ double secondRemaining = second.scmNodeStat
+ .getRemaining().get().doubleValue();
+ double secondCapacity = second.scmNodeStat
+ .getCapacity().get().doubleValue();
+ double secondRatioRemaining = secondRemaining / secondCapacity;
+
+ return Double.compare(secondRatioRemaining, ratioRemaining);
+ }
+
+ /**
+ * Sets DatanodeDetails of this DatanodeUsageInfo.
+ *
+ * @param datanodeDetails the DatanodeDetails to use
+ */
+ public void setDatanodeDetails(
+ DatanodeDetails datanodeDetails) {
+ this.datanodeDetails = datanodeDetails;
+ }
+
+ /**
+ * Sets SCMNodeStat of this DatanodeUsageInfo.
+ *
+ * @param scmNodeStat the SCMNodeStat to use.
+ */
+ public void setScmNodeStat(
+ SCMNodeStat scmNodeStat) {
+ this.scmNodeStat = scmNodeStat;
+ }
+
+ /**
+ * Gets DatanodeDetails of this DatanodeUsageInfo.
+ *
+ * @return DatanodeDetails
+ */
+ public DatanodeDetails getDatanodeDetails() {
+ return datanodeDetails;
+ }
+
+ /**
+ * Gets SCMNodeStat of this DatanodeUsageInfo.
+ *
+ * @return SCMNodeStat
+ */
+ public SCMNodeStat getScmNodeStat() {
+ return scmNodeStat;
+ }
+
+ /**
+ * Gets Comparator that compares two DatanodeUsageInfo on the basis of
+ * used space. The comparison function returns a value greater than 0 if
+ * first DatanodeUsageInfo is more used, a value lesser than 0 if second
+ * DatanodeUsageInfo is more used, and 0 if both are equally used or first
+ * .equals(second) is true
+ *
+ * @return Comparator to compare two DatanodeUsageInfo.
+ */
+ public static Comparator<DatanodeUsageInfo> getUsageComparator() {
Review comment:
Let's rename this to getMostUsedByRemainingPercentage or sth similar.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeUsageInfo.java
##########
@@ -0,0 +1,172 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+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.scm.container.placement.metrics.SCMNodeStat;
+
+import java.util.Comparator;
+
+public class DatanodeUsageInfo {
+
+ private DatanodeDetails datanodeDetails;
+ private SCMNodeStat scmNodeStat;
+
+ /**
+ * Constructs a DatanodeUsageInfo with DatanodeDetails and SCMNodeStat.
+ *
+ * @param datanodeDetails DatanodeDetails
+ * @param scmNodeStat SCMNodeStat
+ */
+ public DatanodeUsageInfo(
+ DatanodeDetails datanodeDetails,
+ SCMNodeStat scmNodeStat) {
+ this.datanodeDetails = datanodeDetails;
+ this.scmNodeStat = scmNodeStat;
+ }
+
+ /**
+ * Compares two DatanodeUsageInfo on the basis of used space.
+ *
+ * @param first DatanodeUsageInfo
+ * @param second DatanodeUsageInfo
+ * @return a value greater than 0 if first is more used, a value
+ * lesser than 0 if second is more used, and 0 if both are equally used or
+ * first.equals(second) is true
+ */
+ private static int compare(DatanodeUsageInfo first,
Review comment:
Let's rename to compareByRemainingPercentage or sth similar? Also update the doc to reflect same.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org