You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "debiswal (via GitHub)" <gi...@apache.org> on 2023/02/15 11:47:31 UTC

[GitHub] [ozone] debiswal opened a new pull request, #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails

debiswal opened a new pull request, #4278:
URL: https://github.com/apache/ozone/pull/4278

   ## What changes were proposed in this pull request?
   
   Adding HTTP & HTTPS Port for SCM UI 
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-7817
   
   ## Patch Tested 
   Manually
   


-- 
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: issues-unsubscribe@ozone.apache.org

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] debiswal commented on a diff in pull request #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails

Posted by "debiswal (via GitHub)" <gi...@apache.org>.
debiswal commented on code in PR #4278:
URL: https://github.com/apache/ozone/pull/4278#discussion_r1107159792


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java:
##########
@@ -802,7 +802,7 @@ public static final class Port {
      */
     public enum Name {
       STANDALONE, RATIS, REST, REPLICATION, RATIS_ADMIN, RATIS_SERVER,
-      RATIS_DATASTREAM;
+      RATIS_DATASTREAM, HTTP_PORT, HTTPS_PORT;

Review Comment:
   Updated .



-- 
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: issues-unsubscribe@ozone.apache.org

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] debiswal closed pull request #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails

Posted by "debiswal (via GitHub)" <gi...@apache.org>.
debiswal closed pull request #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails
URL: https://github.com/apache/ozone/pull/4278


-- 
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: issues-unsubscribe@ozone.apache.org

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] fapifta commented on pull request #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails

Posted by "fapifta (via GitHub)" <gi...@apache.org>.
fapifta commented on PR #4278:
URL: https://github.com/apache/ozone/pull/4278#issuecomment-1439184020

   @adoroszlai 
   it seems that this problem occurs with the DNs during the test in the phase when the cluster is downgraded after an upgrade.
   The DataNodeIDYaml class reads the DataNodeDetails from the DataNode's yaml file, and it runs into a port that is not recognized by the downgraded version.
   
   This though should mean that the change in HDDS-5480 (#2452) is also backward incompatible, but it seems that the port information so far is not saved in the yaml file, as even if the ports are set in the DataNodeDetails, the DataNodeDetails is not persisted to the yaml file after the ports are set.
   Three things seems to persist the DataNodeDetails:
   1. InitDatanodeState
   2. SetNodeOperationalStateCommandHandler
   3. and the certificate client when a new certificate is persisted to the DN
   
   I am unsure about the initialization phases of DN, but it seems that in a cluster 1. runs before the ports are set, 2. does run only when a node is decommissioned or offlined, and after that when it is recommissioned, while 3. runs also before the ports are set.
   
   In this change though the HTTP and HTTPS ports are set before the InitDatanode state runs, and saved, and that is why we run into the problem. However this way there are a set of events that cause backward incompatibility with all the ports that were added after V0_PORTS (upgrade -> decomm/offline a node -> recomm the node -> downgrade).
   
   I think we might need to solve this similarly as it was solved for the client, but now we need to introduce the handling of unknown ports in the DatanodeIdYaml load logic instead of the client. As this was not discovered earlier the incompatibility remains for the REPLICATION, RATIS_ADMIN, RATIS_SERVER ports in the mentioned scenario, while we can handle it for the RATIS_DATASTREAM port as it was introduced after 1.3.0.
   
   @adoroszlai if you agree with my analisys, then I will set up the corresponding JIRAs to fix and handle these, but would not like to run ahead, and do it without consensus on how to solve the problem, and until that @debiswal we should keep this one open, as we need to find out how we want to handle the compatibility aspect of adding these new ports.


-- 
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: issues-unsubscribe@ozone.apache.org

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] debiswal commented on a diff in pull request #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails

Posted by "debiswal (via GitHub)" <gi...@apache.org>.
debiswal commented on code in PR #4278:
URL: https://github.com/apache/ozone/pull/4278#discussion_r1108168582


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java:
##########
@@ -67,6 +68,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 
+import static org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name.*;

Review Comment:
   Removed .



-- 
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: issues-unsubscribe@ozone.apache.org

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] errose28 commented on pull request #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails

Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #4278:
URL: https://github.com/apache/ozone/pull/4278#issuecomment-1456564013

   Right @adoroszlai. When this change was made on the datanode, it broke downgrade from 1.1.0 to 1.0.0. Since the upgrade framework was still WIP at that time and downgrade not officially supported, we decided not to support downgrade from 1.1.0 to 1.0.0 and allow the change. Now that we support downgrades this will need to be done in a backwards compatible way.


-- 
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: issues-unsubscribe@ozone.apache.org

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] adoroszlai commented on a diff in pull request #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4278:
URL: https://github.com/apache/ozone/pull/4278#discussion_r1107038004


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java:
##########
@@ -802,7 +802,7 @@ public static final class Port {
      */
     public enum Name {
       STANDALONE, RATIS, REST, REPLICATION, RATIS_ADMIN, RATIS_SERVER,
-      RATIS_DATASTREAM;
+      RATIS_DATASTREAM, HTTP_PORT, HTTPS_PORT;

Review Comment:
   Please rename to `HTTP` and `HTTPS`.  None of the other names include "_PORT".



-- 
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: issues-unsubscribe@ozone.apache.org

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] adoroszlai commented on a diff in pull request #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4278:
URL: https://github.com/apache/ozone/pull/4278#discussion_r1107169146


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java:
##########
@@ -67,6 +68,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 
+import static org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name.*;

Review Comment:
   ```
   hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java
    71: Using the '.*' form of import should be avoided - org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name.*.
   ```
   
   https://github.com/debiswal/ozone/actions/runs/4184593284/jobs/7250464577#step:6:435



-- 
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: issues-unsubscribe@ozone.apache.org

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] adoroszlai commented on pull request #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4278:
URL: https://github.com/apache/ozone/pull/4278#issuecomment-1432957982

   @fapifta Upgrade acceptance test is failing due to:
   
   ```
   dn1_1    | 2023-02-16 09:02:55,670 [main] ERROR ozone.HddsDatanodeService: Exception in HddsDatanodeService.
   dn1_1    | java.lang.IllegalArgumentException: No enum constant org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name.HTTP
   dn1_1    | 	at java.base/java.lang.Enum.valueOf(Enum.java:240)
   dn1_1    | 	at org.apache.hadoop.hdds.protocol.DatanodeDetails$Port$Name.valueOf(DatanodeDetails.java:785)
   dn1_1    | 	at org.apache.hadoop.ozone.container.common.helpers.DatanodeIdYaml.readDatanodeIdFile(DatanodeIdYaml.java:99)
   dn1_1    | 	at org.apache.hadoop.ozone.container.common.helpers.ContainerUtils.readDatanodeDetailsFrom(ContainerUtils.java:171)
   dn1_1    | 	at org.apache.hadoop.ozone.HddsDatanodeService.initializeDatanodeDetails(HddsDatanodeService.java:467)
   dn1_1    | 	at org.apache.hadoop.ozone.HddsDatanodeService.start(HddsDatanodeService.java:221)
   dn1_1    | 	at org.apache.hadoop.ozone.HddsDatanodeService.start(HddsDatanodeService.java:207)
   dn1_1    | 	at org.apache.hadoop.ozone.HddsDatanodeService.call(HddsDatanodeService.java:175)
   dn1_1    | 	at org.apache.hadoop.ozone.HddsDatanodeService.call(HddsDatanodeService.java:92)
   ```
   
   So this is a backwards incompatible change.


-- 
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: issues-unsubscribe@ozone.apache.org

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] debiswal commented on pull request #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails

Posted by "debiswal (via GitHub)" <gi...@apache.org>.
debiswal commented on PR #4278:
URL: https://github.com/apache/ozone/pull/4278#issuecomment-1457959673

   closing this PR due to incompatibility in downgrade/upgrade  system


-- 
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: issues-unsubscribe@ozone.apache.org

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] debiswal commented on a diff in pull request #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails

Posted by "debiswal (via GitHub)" <gi...@apache.org>.
debiswal commented on code in PR #4278:
URL: https://github.com/apache/ozone/pull/4278#discussion_r1108168582


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java:
##########
@@ -67,6 +68,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 
+import static org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name.*;

Review Comment:
   it was unintentional though, IDE has imported  anyway have  Removed it .



-- 
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: issues-unsubscribe@ozone.apache.org

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] adoroszlai commented on pull request #4278: HDDS-7817. Add HTTP and HTTPS ports to DatanodeDetails

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4278:
URL: https://github.com/apache/ozone/pull/4278#issuecomment-1456416298

   > As this was not discovered earlier the incompatibility remains for the REPLICATION, RATIS_ADMIN, RATIS_SERVER ports in the mentioned scenario, 
   
   Those were introduced in 1.1.0.  I don't think we support downgrade to any version before that.
   
   CC @errose28 


-- 
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: issues-unsubscribe@ozone.apache.org

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