You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "peterxcli (via GitHub)" <gi...@apache.org> on 2025/03/03 15:42:14 UTC
[PR] HDDS-12458. Refactor DataNodeSafeModeRule to use NodeManager [ozone]
peterxcli opened a new pull request, #7998:
URL: https://github.com/apache/ozone/pull/7998
## What changes were proposed in this pull request?
Currently the DataNodeSafeModeRule is processing report from Datanodes to validate the rule, we can use the information in NodeManager to validate instead.
We will remove the report processing logic from SafeMode rules in future.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-12458
## How was this patch tested?
- Added `TestDataNodeSafeModeRule`.
CI:
https://github.com/peterxcli/ozone/actions/runs/13550557639
--
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-12458. Refactor DataNodeSafeModeRule to use NodeManager [ozone]
Posted by "aryangupta1998 (via GitHub)" <gi...@apache.org>.
aryangupta1998 commented on PR #7998:
URL: https://github.com/apache/ozone/pull/7998#issuecomment-2779516047
> The validateBasedOnReportProcessing check added to validate is for future report logic removal more convenient.
I understand. For the very same reason, I thought of adding the validateBasedOnReportProcessing check in the process(), as it will be removed in the future.
However, the changes look good, just fix the CI check.
--
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-12458. Refactor DataNodeSafeModeRule to use NodeManager [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7998:
URL: https://github.com/apache/ozone/pull/7998#issuecomment-2785375212
Thanks @peterxcli for the patch, @aryangupta1998 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-12458. Refactor DataNodeSafeModeRule to use NodeManager [ozone]
Posted by "aryangupta1998 (via GitHub)" <gi...@apache.org>.
aryangupta1998 commented on PR #7998:
URL: https://github.com/apache/ozone/pull/7998#issuecomment-2778803601
Thanks for the patch @peterxcli, can we modify the process() of DataNodeSafeModeRule something as,
```
protected void process(NodeRegistrationContainerReport reportsProto) {
if (validateBasedOnReportProcessing()) {
registeredDnSet.add(reportsProto.getDatanodeDetails().getUuid());
registeredDns = registeredDnSet.size();
} else {
registeredDns = nodeManager.getNodes(NodeStatus.inServiceHealthy()).size();
}
if (scmInSafeMode()) {
SCMSafeModeManager.getLogger().info(
"SCM in safe mode. {} DataNodes registered, {} required.",
registeredDns, requiredDns);
}
}
```
because in future we maynot be needing the datanode reports to process the information!
--
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-12458. Refactor DataNodeSafeModeRule to use NodeManager [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #7998:
URL: https://github.com/apache/ozone/pull/7998#issuecomment-2779095311
Thanks @aryangupta1998 for the review.
> because in future we maynot be needing the datanode reports to process the information!
Do we still the need the `process` function after fully remove the report logic from safemodes? If not, i think we may not need to add that if condition.
--
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-12458. Refactor DataNodeSafeModeRule to use NodeManager [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #7998:
URL: https://github.com/apache/ozone/pull/7998
--
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-12458. Refactor DataNodeSafeModeRule to use NodeManager [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #7998:
URL: https://github.com/apache/ozone/pull/7998#issuecomment-2782950077
@aryangupta1998 Do you think this patch is ready to go?
--
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-12458. Refactor DataNodeSafeModeRule to use NodeManager [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #7998:
URL: https://github.com/apache/ozone/pull/7998#issuecomment-2779542450
> However, the changes look good, just fix the CI check.
It seems it is a flaky test, could you help me to rerun that test?
latest passed CI: https://github.com/peterxcli/ozone/actions/runs/14235955706
--
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-12458. Refactor DataNodeSafeModeRule to use NodeManager [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #7998:
URL: https://github.com/apache/ozone/pull/7998#issuecomment-2785383206
Thanks @aryangupta1998 for the review, @adoroszlai for merging.
--
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