You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "navinko (via GitHub)" <gi...@apache.org> on 2026/09/19 09:25:56 UTC
[PR] HDDS-16386. NodeDecommissionManager should read maintenance replica config from ReplicationManagerConfiguration [ozone]
navinko opened a new pull request, #11271:
URL: https://github.com/apache/ozone/pull/11271
## What changes were proposed in this pull request?
NodeDecommissionManager was reading the two maintenance settings as keys with hardcoded default values:
```
setMaintenanceConfigs(config.getInt("hdds.scm.replication.maintenance.replica.minimum", 2),
config.getInt("hdds.scm.replication.maintenance.remaining.redundancy", 1));
```
Those keys and defaults (2 and 1) are already defined in ReplicationManager.ReplicationManagerConfiguration, so they were duplicated in two places. Since the constructor already receives the ReplicationManager instance, this PR reads them from rm.getConfig(), making ReplicationManagerConfiguration the single source of truth.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16386
## How was this patch tested?
#Added new unit test -
TestNodeDecommissionManager#testMaintenanceConfigsReadFromReplicationManagerConfiguration — verifies the values comes from ReplicationManagerConfiguration.
#Updated existing unit test -
TestDeletedBlockLog to have getConfig stub, otherwise passing it as null causing NPE which results test failure.
Successful CI - https://github.com/navinko/ozone/actions/runs/35424869985
--
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-16386. NodeDecommissionManager should read maintenance replica config from ReplicationManagerConfiguration [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl merged PR #11271:
URL: https://github.com/apache/ozone/pull/11271
--
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-16386. NodeDecommissionManager should read maintenance replica config from ReplicationManagerConfiguration [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #11271:
URL: https://github.com/apache/ozone/pull/11271#issuecomment-6284889644
Thanks @navinko for the patch. Thanks @chihsuan @rich7420 for the reviews.
--
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-16386. NodeDecommissionManager should read maintenance replica config from ReplicationManagerConfiguration [ozone]
Posted by "navinko (via GitHub)" <gi...@apache.org>.
navinko commented on PR #11271:
URL: https://github.com/apache/ozone/pull/11271#issuecomment-5995553736
> LGTM
Thanks @rich7420 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-16386. NodeDecommissionManager should read maintenance replica config from ReplicationManagerConfiguration [ozone]
Posted by "chihsuan (via GitHub)" <gi...@apache.org>.
chihsuan commented on code in PR #11271:
URL: https://github.com/apache/ozone/pull/11271#discussion_r4063068002
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java:
##########
@@ -281,8 +281,11 @@ public NodeDecommissionManager(OzoneConfiguration config, NodeManager nm, Contai
ScmConfigKeys.OZONE_SCM_DATANODE_ADMIN_MONITOR_INTERVAL_DEFAULT,
TimeUnit.MILLISECONDS);
- setMaintenanceConfigs(config.getInt("hdds.scm.replication.maintenance.replica.minimum", 2),
- config.getInt("hdds.scm.replication.maintenance.remaining.redundancy", 1));
+ // Read the maintenance replica config from ReplicationManagerConfiguration,
+ // which is the single source of truth for these values.
Review Comment:
nit: Could we drop this comment? The code already says where the values come from.
```suggestion
```
--
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-16386. NodeDecommissionManager should read maintenance replica config from ReplicationManagerConfiguration [ozone]
Posted by "navinko (via GitHub)" <gi...@apache.org>.
navinko commented on PR #11271:
URL: https://github.com/apache/ozone/pull/11271#issuecomment-6323173682
Thanks @chihsuan @rich7420 for the reviews and @smengcl for the merge.
--
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-16386. NodeDecommissionManager should read maintenance replica config from ReplicationManagerConfiguration [ozone]
Posted by "navinko (via GitHub)" <gi...@apache.org>.
navinko commented on code in PR #11271:
URL: https://github.com/apache/ozone/pull/11271#discussion_r4063661912
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java:
##########
@@ -281,8 +281,11 @@ public NodeDecommissionManager(OzoneConfiguration config, NodeManager nm, Contai
ScmConfigKeys.OZONE_SCM_DATANODE_ADMIN_MONITOR_INTERVAL_DEFAULT,
TimeUnit.MILLISECONDS);
- setMaintenanceConfigs(config.getInt("hdds.scm.replication.maintenance.replica.minimum", 2),
- config.getInt("hdds.scm.replication.maintenance.remaining.redundancy", 1));
+ // Read the maintenance replica config from ReplicationManagerConfiguration,
+ // which is the single source of truth for these values.
Review Comment:
Thank you! Removed the comment.
--
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-16386. NodeDecommissionManager should read maintenance replica config from ReplicationManagerConfiguration [ozone]
Posted by "navinko (via GitHub)" <gi...@apache.org>.
navinko commented on PR #11271:
URL: https://github.com/apache/ozone/pull/11271#issuecomment-5763056705
> Thanks for the cleanup! @navinko LGTM +1
>
> Just curious, since both keys are reconfigurable and only ReplicationManager picks up a reconfig today, could `checkIfMaintenancePossible` read `rmConf` directly instead of copying the values? Happy to see that as a follow-up too. Left a small nit inline.
Thanks @chihsuan for the review.
Yes, it's doable . I'll do the rmConf read as the follow-up.
--
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