tanvipenumudy commented on code in PR #5391:
URL: https://github.com/apache/ozone/pull/5391#discussion_r1483832264


##########
hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto:
##########
@@ -230,6 +233,60 @@ message SortDatanodesResponseProto{
   repeated DatanodeDetailsProto node = 1;
 }
 
+message NodeImpl {
+  required string name = 1;
+  required string location = 2;
+  required uint32 cost = 3;
+}
+
+message CommandCount {
+  required SCMCommandProto.Type command = 1;
+  required uint32 count = 2;
+}
+
+message NodeStatus {
+  required NodeOperationalState operationalState = 1;
+  required NodeState health = 2;
+  required uint64 opStateExpiryEpochSeconds = 3;
+}
+
+message DatanodeInfo {
+  required DatanodeDetailsProto datanodeDetails = 1;
+  required uint64 lastHeartbeatTime = 2;
+  required uint64 lastStatsUpdatedTime = 3;
+  required uint32 failedVolumeCount = 4;
+  repeated StorageReportProto storageReportProto = 5;
+  repeated MetadataStorageReportProto metadataStorageReportProto = 6;
+  required LayoutVersionProto layoutVersionProto = 7;
+  repeated CommandCount commandCount = 8;
+  required NodeStatus nodeStatus = 9;
+}
+
+message NodeType {
+  optional DatanodeDetailsProto datanodeDetails = 1;
+  optional DatanodeInfo datanodeInfo = 2;

Review Comment:
   ![image (13)-2 
(1)](https://github.com/apache/ozone/assets/46785609/418e744e-b00d-404b-9f73-39ee6b61c8e7)
   - I was facing a few challenges while trying to include the parent 
information during serialization/deserialization. Every child object has a 
reference to its parent and similarly the parent has a reference to its child 
(as shown in the image: parent shown in white, child shown in yellow, for 
example). This sort of makes the serialization/deserialization go on infinitely 
deeper. 
   - I was looking at the existing implementation of 
serialization/deserialization of `DatanodeDetails` (which extends `NodeImpl`), 
where we are currently not including the parent information.
   - I went with the second constructor of `NodeImpl` to create the object 
(which does not include the parent). 
   - I am trying to check if there's a better way to approach this, open to any 
suggestions!



-- 
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]

Reply via email to