You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "sarvekshayr (via GitHub)" <gi...@apache.org> on 2025/02/18 12:08:04 UTC

[PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

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

   ## What changes were proposed in this pull request?
   #7803 renamed config keys previously prefixed with `dfs` to `hdds`. This PR updates the corresponding Java constants in `DFSConfigKeysLegacy`, moves the configs out and removes the deprecated class.
   
   ## What is the link to the Apache JIRA
   [HDDS-12172](https://issues.apache.org/jira/browse/HDDS-12172)
   
   ## How was this patch tested?
   CI: https://github.com/sarvekshayr/ozone/actions/runs/13388920661
   


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

Re: [PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

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

   > Where is this information in the release?
   
   It's in the source release: `hadoop-hdds/common/src/main/resources/ozone-default.xml`.
   
   Interestingly this file is not present in the binary tarball.


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

Re: [PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

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

   > Adding config keys to ozone-default.xml makes them visible for users of Ozone, provides description and tags
   
   Where is this information in the release? I just checked the 1.4.1 tarball and `/ec/hadoop/ozone-site.xml` is empty.


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

Re: [PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

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

   Thanks @sarvekshayr for the patch, @errose28 for the review.


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

Re: [PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

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


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java:
##########
@@ -595,6 +595,34 @@ public final class OzoneConfigKeys {
       OZONE_CLIENT_BUCKET_REPLICATION_CONFIG_REFRESH_PERIOD_DEFAULT_MS =
       300 * 1000;
 
+  public static final String HDDS_DATANODE_DNS_INTERFACE_KEY =

Review Comment:
   Our config organization isn't great, but these should probably be moved to:
   - `HddsConfigKeys` if used by datanode and SCM
   - `DatanodeConfiguration` if used only by datanode
   - `ScmConfigKeys` if used only by SCM



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

Re: [PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

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

   On second thought, to avoid feature scope of this task, I think we should:
   - Keep the exclusion in `TestOzoneConfigurationFields` as done in the PR, because adding to `ozone-default.xml` requires documenting them, which is a different type of task.
   - Move constants to `HddsConfigKeys` (`NET_TOPOLOGY_NODE_SWITCH_MAPPING_IMPL_KEY` to `ScmConfigKeys`), because `DatanodeConfiguration` is a different kind of object, not just a collection of constants for keys and default values.  Adding `@Config` items also requires description, similar to `ozone-default.xml`.
   - Document in follow-up task, adding to `ozone-default.xml` or `DatanodeConfiguration`.


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

Re: [PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

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

   > I wonder if any of the config keys should be added to ozone-default.xml, instead of marking as "not in XML" in TestOzoneConfigurationFields.
   
   Can you remind me what the significance of putting configs in `ozone-default.xml` vs defining the default values in the code is?


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

Re: [PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

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

   > > I wonder if any of the config keys should be added to ozone-default.xml, instead of marking as "not in XML" in TestOzoneConfigurationFields.
   > 
   > Can you remind me what the significance of putting configs in `ozone-default.xml` vs defining the default values in the code is?
   
   Adding config keys to `ozone-default.xml` makes them visible for users of Ozone, provides description and tags.  Adding them as exception in `TestOzoneConfigurationFields` means they are undocumented.
   
   `TestOzoneConfigurationFields` verifies that all keys are present in both XML and constants.


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

Re: [PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

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


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java:
##########
@@ -595,6 +595,34 @@ public final class OzoneConfigKeys {
       OZONE_CLIENT_BUCKET_REPLICATION_CONFIG_REFRESH_PERIOD_DEFAULT_MS =
       300 * 1000;
 
+  public static final String HDDS_DATANODE_DNS_INTERFACE_KEY =

Review Comment:
   Our existing config organization isn't great, but these should probably be moved to:
   - `HddsConfigKeys` if used by datanode and SCM
   - `DatanodeConfiguration` if used only by datanode
   - `ScmConfigKeys` if used only by SCM



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

Re: [PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #7922:
URL: https://github.com/apache/ozone/pull/7922


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

Re: [PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

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

   For this PR we can keep using ozone-default.xml as we have been. We should probably add the major configs like datanode kerberos principal to ozone-default.xml. Later we can investigate how to better expose default configs to users, ideally without devs having to put them in two places.


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

Re: [PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

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

   > ideally without devs having to put them in two places.
   
   That's one of the benefits of `@Config` annotated objects.


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

Re: [PR] HDDS-12172. Rename Java constants of DFSConfigKeysLegacy keys [ozone]

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


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java:
##########
@@ -629,6 +629,10 @@ public final class ScmConfigKeys {
       "ozone.scm.ha.dbtransactionbuffer.flush.interval";
   public static final long
       OZONE_SCM_HA_DBTRANSACTIONBUFFER_FLUSH_INTERVAL_DEFAULT = 600 * 1000L;
+
+  public static final String NET_TOPOLOGY_NODE_SWITCH_MAPPING_IMPL_KEY =
+          "net.topology.node.switch.mapping.impl";

Review Comment:
   ```suggestion
         "net.topology.node.switch.mapping.impl";
   ```



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsConfigKeys.java:
##########
@@ -405,4 +405,32 @@ private HddsConfigKeys() {
 
   public static final String OZONE_DATANODE_IO_METRICS_PERCENTILES_INTERVALS_SECONDS_KEY =
       "ozone.volume.io.percentiles.intervals.seconds";
+
+  public static final String HDDS_DATANODE_DNS_INTERFACE_KEY =
+          "hdds.datanode.dns.interface";
+  public static final String HDDS_DATANODE_DNS_NAMESERVER_KEY =
+          "hdds.datanode.dns.nameserver";
+  public static final String HDDS_DATANODE_HOST_NAME_KEY =
+          "hdds.datanode.hostname";
+  public static final String HDDS_DATANODE_DATA_DIR_KEY =
+          "hdds.datanode.data.dir";
+  public static final String HDDS_DATANODE_USE_DN_HOSTNAME =
+          "hdds.datanode.use.datanode.hostname";
+  public static final boolean HDDS_DATANODE_USE_DN_HOSTNAME_DEFAULT = false;
+
+  public static final String HDDS_XFRAME_OPTION_ENABLED = "hdds.xframe.enabled";
+  public static final boolean HDDS_XFRAME_OPTION_ENABLED_DEFAULT = true;
+  public static final String HDDS_XFRAME_OPTION_VALUE = "hdds.xframe.value";
+  public static final String HDDS_XFRAME_OPTION_VALUE_DEFAULT = "SAMEORIGIN";
+
+  public static final String HDDS_METRICS_SESSION_ID_KEY =
+          "hdds.metrics.session-id";
+
+  public static final String HDDS_DATANODE_KERBEROS_PRINCIPAL_KEY =
+          "hdds.datanode.kerberos.principal";
+  public static final String HDDS_DATANODE_KERBEROS_KEYTAB_FILE_KEY =
+          "hdds.datanode.kerberos.keytab.file";
+  public static final String HDDS_METRICS_PERCENTILES_INTERVALS_KEY =
+          "hdds.metrics.percentiles.intervals";

Review Comment:
   ```suggestion
     public static final String HDDS_DATANODE_DNS_INTERFACE_KEY =
         "hdds.datanode.dns.interface";
     public static final String HDDS_DATANODE_DNS_NAMESERVER_KEY =
         "hdds.datanode.dns.nameserver";
     public static final String HDDS_DATANODE_HOST_NAME_KEY =
         "hdds.datanode.hostname";
     public static final String HDDS_DATANODE_DATA_DIR_KEY =
         "hdds.datanode.data.dir";
     public static final String HDDS_DATANODE_USE_DN_HOSTNAME =
         "hdds.datanode.use.datanode.hostname";
     public static final boolean HDDS_DATANODE_USE_DN_HOSTNAME_DEFAULT = false;
   
     public static final String HDDS_XFRAME_OPTION_ENABLED = "hdds.xframe.enabled";
     public static final boolean HDDS_XFRAME_OPTION_ENABLED_DEFAULT = true;
     public static final String HDDS_XFRAME_OPTION_VALUE = "hdds.xframe.value";
     public static final String HDDS_XFRAME_OPTION_VALUE_DEFAULT = "SAMEORIGIN";
   
     public static final String HDDS_METRICS_SESSION_ID_KEY =
         "hdds.metrics.session-id";
   
     public static final String HDDS_DATANODE_KERBEROS_PRINCIPAL_KEY =
         "hdds.datanode.kerberos.principal";
     public static final String HDDS_DATANODE_KERBEROS_KEYTAB_FILE_KEY =
         "hdds.datanode.kerberos.keytab.file";
     public static final String HDDS_METRICS_PERCENTILES_INTERVALS_KEY =
         "hdds.metrics.percentiles.intervals";
   ```



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