kerneltime commented on code in PR #5391:
URL: https://github.com/apache/ozone/pull/5391#discussion_r1486686794
##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -497,3 +498,26 @@ message CompactionLogEntryProto {
repeated CompactionFileInfoProto outputFileIntoList = 4;
optional string compactionReason = 5;
}
+
+message NodeImpl {
+ required string name = 1;
+ required string location = 2;
+ required uint32 cost = 3;
+ required uint32 level = 4;
+}
+
+message NodeInterface {
Review Comment:
Is there a better name that we can use?
##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -497,3 +498,26 @@ message CompactionLogEntryProto {
repeated CompactionFileInfoProto outputFileIntoList = 4;
optional string compactionReason = 5;
}
+
+message NodeImpl {
Review Comment:
`NodeImpl` is the implementation class, this is essentially the
NodeTopology, we can rename this to `NodeTopology` or just `Node`.
##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -497,3 +498,26 @@ message CompactionLogEntryProto {
repeated CompactionFileInfoProto outputFileIntoList = 4;
optional string compactionReason = 5;
}
+
+message NodeImpl {
+ required string name = 1;
+ required string location = 2;
+ required uint32 cost = 3;
+ required uint32 level = 4;
+}
+
+message NodeInterface {
+ optional DatanodeDetailsProto datanodeDetails = 1;
+ optional InnerNode innerNode = 3;
+}
+
+message ChildrenMap {
+ required string networkName = 1;
+ required NodeInterface nodeType = 2;
+}
+
+message InnerNode {
+ required NodeImpl nodeImpl = 1;
+ required uint32 numOfLeaves = 2;
+ repeated ChildrenMap childrenMap = 3;
Review Comment:
We should mark these as optional, this will make future changes a bit easier
and leave the compatibility to the higher level applications.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -1847,24 +1849,49 @@ void sortDatanodes(String clientMachine, OmKeyInfo...
keyInfos) {
}
}
- private List<DatanodeDetails> sortDatanodes(String clientMachine,
+ public List<DatanodeDetails> sortDatanodes(String clientMachine,
List<DatanodeDetails> nodes, OmKeyInfo keyInfo, List<String> nodeList) {
- List<DatanodeDetails> sortedNodes = null;
- try {
- sortedNodes = scmClient.getBlockClient()
- .sortDatanodes(nodeList, clientMachine);
- if (LOG.isDebugEnabled()) {
- LOG.debug("Sorted datanodes {} for client {}, result: {}", nodes,
- clientMachine, sortedNodes);
+ DatanodeDetails client = null;
+ List<DatanodeDetails> possibleClients =
+ getClientNodesByAddress(clientMachine, nodes);
+ if (possibleClients.size() > 0) {
+ client = possibleClients.get(0);
+ } else {
+ /**
+ * In case of read from a non-datanode host, return
+ * {@link ScmBlockLocationProtocol#sortDatanodes(List<String>, String)}.
+ */
+ try {
+ return scmClient.getBlockClient()
+ .sortDatanodes(nodeList, clientMachine);
+ } catch (IOException e) {
+ LOG.warn("Unable to sort datanodes based on distance to client, "
Review Comment:
A client outside of the Datanode set of nodes can flood the OM log. Tracking
a metric for number of requests failed would be better (along with the latency
introduced to this method). Best to keep the logging level to `debug`
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -1847,24 +1849,49 @@ void sortDatanodes(String clientMachine, OmKeyInfo...
keyInfos) {
}
}
- private List<DatanodeDetails> sortDatanodes(String clientMachine,
+ public List<DatanodeDetails> sortDatanodes(String clientMachine,
List<DatanodeDetails> nodes, OmKeyInfo keyInfo, List<String> nodeList) {
- List<DatanodeDetails> sortedNodes = null;
- try {
- sortedNodes = scmClient.getBlockClient()
- .sortDatanodes(nodeList, clientMachine);
- if (LOG.isDebugEnabled()) {
- LOG.debug("Sorted datanodes {} for client {}, result: {}", nodes,
- clientMachine, sortedNodes);
+ DatanodeDetails client = null;
+ List<DatanodeDetails> possibleClients =
+ getClientNodesByAddress(clientMachine, nodes);
+ if (possibleClients.size() > 0) {
+ client = possibleClients.get(0);
+ } else {
+ /**
+ * In case of read from a non-datanode host, return
Review Comment:
What's the capability within SCM that cannot be replicated within Ozone for
clients not on 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.
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]
