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 2020/09/14 06:21:01 UTC

[GitHub] [hadoop-ozone] prashantpogde opened a new pull request #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

prashantpogde opened a new pull request #1421:
URL: https://github.com/apache/hadoop-ozone/pull/1421


   ## What changes were proposed in this pull request?
   
   Add current HDDS layout version to DataNode heartbeat/registration
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-4174
   
   ## How was this patch tested?
   Successful Build
   


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on a change in pull request #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1421:
URL: https://github.com/apache/hadoop-ozone/pull/1421#discussion_r487716504



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerDatanodeHeartbeatProtocol.proto
##########
@@ -390,6 +404,14 @@ message ClosePipelineCommandProto {
   required int64 cmdId = 2;
 }
 
+/**
+ * This command asks the DataNode to finalize a new HDDS layout version.
+ */
+message FinalizeNewLayoutVersionCommandProto {
+  required bool finalizeNewHDDSLayoutVersion = 1 [default = false];

Review comment:
       What is the use case the command where this flag=false. Is it a no-op request?




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] amaliujia commented on a change in pull request #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1421:
URL: https://github.com/apache/hadoop-ozone/pull/1421#discussion_r488049394



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerDatanodeHeartbeatProtocol.proto
##########
@@ -303,6 +315,8 @@ message SCMCommandProto {
   optional ReplicateContainerCommandProto replicateContainerCommandProto = 6;
   optional CreatePipelineCommandProto createPipelineCommandProto = 7;
   optional ClosePipelineCommandProto closePipelineCommandProto = 8;
+  optional FinalizeNewLayoutVersionCommandProto
+  finalizeNewLayoutVersionCommandProto = 9;

Review comment:
       Why not use a single line: `optional FinalizeNewLayoutVersionCommandProto=9`?




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] prashantpogde commented on a change in pull request #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1421:
URL: https://github.com/apache/hadoop-ozone/pull/1421#discussion_r488049733



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerDatanodeHeartbeatProtocol.proto
##########
@@ -390,6 +404,14 @@ message ClosePipelineCommandProto {
   required int64 cmdId = 2;
 }
 
+/**
+ * This command asks the DataNode to finalize a new HDDS layout version.
+ */
+message FinalizeNewLayoutVersionCommandProto {
+  required bool finalizeNewHDDSLayoutVersion = 1 [default = false];

Review comment:
       yes, that would be a no op request.  
   




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1421:
URL: https://github.com/apache/hadoop-ozone/pull/1421#discussion_r487719553



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerDatanodeHeartbeatProtocol.proto
##########
@@ -92,6 +101,7 @@ message SCMRegisterRequestProto {
   required NodeReportProto nodeReport = 2;
   required ContainerReportsProto containerReport = 3;
   required PipelineReportsProto pipelineReports = 4;
+  optional HDDSLayoutVersionProto dataNodeLayoutVersion = 5;

Review comment:
       Why not this be a required field? when Datanode firstly register on SCM, it should have its own HDDS layout version.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] avijayanhwx merged pull request #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

Posted by GitBox <gi...@apache.org>.
avijayanhwx merged pull request #1421:
URL: https://github.com/apache/hadoop-ozone/pull/1421


   


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on a change in pull request #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1421:
URL: https://github.com/apache/hadoop-ozone/pull/1421#discussion_r491953506



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerDatanodeHeartbeatProtocol.proto
##########
@@ -390,6 +404,14 @@ message ClosePipelineCommandProto {
   required int64 cmdId = 2;
 }
 
+/**
+ * This command asks the DataNode to finalize a new HDDS layout version.
+ */
+message FinalizeNewLayoutVersionCommandProto {
+  required bool finalizeNewHDDSLayoutVersion = 1 [default = false];

Review comment:
       Is there any use case for this noop request? Will it be used by somebody? Why do we need this field?




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] prashantpogde commented on a change in pull request #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1421:
URL: https://github.com/apache/hadoop-ozone/pull/1421#discussion_r488052107



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerDatanodeHeartbeatProtocol.proto
##########
@@ -303,6 +315,8 @@ message SCMCommandProto {
   optional ReplicateContainerCommandProto replicateContainerCommandProto = 6;
   optional CreatePipelineCommandProto createPipelineCommandProto = 7;
   optional ClosePipelineCommandProto closePipelineCommandProto = 8;
+  optional FinalizeNewLayoutVersionCommandProto
+  finalizeNewLayoutVersionCommandProto = 9;

Review comment:
       > 80 columns




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] amaliujia commented on a change in pull request #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1421:
URL: https://github.com/apache/hadoop-ozone/pull/1421#discussion_r488054797



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerDatanodeHeartbeatProtocol.proto
##########
@@ -303,6 +315,8 @@ message SCMCommandProto {
   optional ReplicateContainerCommandProto replicateContainerCommandProto = 6;
   optional CreatePipelineCommandProto createPipelineCommandProto = 7;
   optional ClosePipelineCommandProto closePipelineCommandProto = 8;
+  optional FinalizeNewLayoutVersionCommandProto
+  finalizeNewLayoutVersionCommandProto = 9;

Review comment:
       Ah I see. Didn't know Ozone also enforce that in proto files (very common that message name is very long).




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] prashantpogde commented on a change in pull request #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1421:
URL: https://github.com/apache/hadoop-ozone/pull/1421#discussion_r488166360



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerDatanodeHeartbeatProtocol.proto
##########
@@ -92,6 +101,7 @@ message SCMRegisterRequestProto {
   required NodeReportProto nodeReport = 2;
   required ContainerReportsProto containerReport = 3;
   required PipelineReportsProto pipelineReports = 4;
+  optional HDDSLayoutVersionProto dataNodeLayoutVersion = 5;

Review comment:
       All new fields added should be optional for backward compatibility.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] prashantpogde commented on a change in pull request #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1421:
URL: https://github.com/apache/hadoop-ozone/pull/1421#discussion_r488051287



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerDatanodeHeartbeatProtocol.proto
##########
@@ -92,6 +101,7 @@ message SCMRegisterRequestProto {
   required NodeReportProto nodeReport = 2;
   required ContainerReportsProto containerReport = 3;
   required PipelineReportsProto pipelineReports = 4;
+  optional HDDSLayoutVersionProto dataNodeLayoutVersion = 5;

Review comment:
       Datanode need not pass the HDDSLaoutVersionProto information during registration. 




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on a change in pull request #1421: HDDS-4174. Add current HDDS layout version to Datanode heartbeat/registration

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1421:
URL: https://github.com/apache/hadoop-ozone/pull/1421#discussion_r491953506



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerDatanodeHeartbeatProtocol.proto
##########
@@ -390,6 +404,14 @@ message ClosePipelineCommandProto {
   required int64 cmdId = 2;
 }
 
+/**
+ * This command asks the DataNode to finalize a new HDDS layout version.
+ */
+message FinalizeNewLayoutVersionCommandProto {
+  required bool finalizeNewHDDSLayoutVersion = 1 [default = false];

Review comment:
       Is there any use case for this noop request? Will it be used by somebody? Why do we need this field?




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org