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 2022/07/29 11:41:32 UTC
[GitHub] [ozone] sodonnel opened a new pull request, #3640: HDDS-7069. EC: ReplicationManager - Track nodes already used when handing under replication
sodonnel opened a new pull request, #3640:
URL: https://github.com/apache/ozone/pull/3640
## What changes were proposed in this pull request?
If a container has a missing replica, and some decommission replicas, then it will have both a reconstruction and replication command scheduled when it is processed for under replication.
However, the target node picked for the reconstruction is not excluded when picking nodes for the replication, so it would be possible for both commands to target the same node with different indexes.
We need to track all nodes selected when processing under replication to ensure they are not used twice.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7069
## How was this patch tested?
New unit test
--
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] sodonnel commented on a diff in pull request #3640: HDDS-7069. EC: ReplicationManager - Track nodes already used when handing under replication
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3640:
URL: https://github.com/apache/ozone/pull/3640#discussion_r934402045
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECUnderReplicationHandler.java:
##########
@@ -186,15 +191,17 @@ public Map<DatanodeDetails, SCMCommand<?>> processAndCreateCommands(
Set<Integer> decomIndexes = replicaCount.decommissioningOnlyIndexes(true);
if (decomIndexes.size() > 0) {
final List<DatanodeDetails> selectedDatanodes =
- getTargetDatanodes(replicas, container, decomIndexes.size());
+ getTargetDatanodes(excludedNodes, container, decomIndexes.size());
+ excludedNodes.addAll(selectedDatanodes);
Review Comment:
There are two reasons I added it here - (1) to keep it consistent with the earlier code and (2) because we need to extend this class for maintenance mode, and we will need to add it here then anyway, and its something that could easily get missed. Therefore I think its best to keep it as it 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
[GitHub] [ozone] umamaheswararao commented on a diff in pull request #3640: HDDS-7069. EC: ReplicationManager - Track nodes already used when handing under replication
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3640:
URL: https://github.com/apache/ozone/pull/3640#discussion_r934053182
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECUnderReplicationHandler.java:
##########
@@ -186,15 +191,17 @@ public Map<DatanodeDetails, SCMCommand<?>> processAndCreateCommands(
Set<Integer> decomIndexes = replicaCount.decommissioningOnlyIndexes(true);
if (decomIndexes.size() > 0) {
final List<DatanodeDetails> selectedDatanodes =
- getTargetDatanodes(replicas, container, decomIndexes.size());
+ getTargetDatanodes(excludedNodes, container, decomIndexes.size());
+ excludedNodes.addAll(selectedDatanodes);
Review Comment:
Nit: Is this adding is useful only for logging? After decommission indexes processing, we don't use that exclude list anymore except the logging? If so, we could add just before logging?
--
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] umamaheswararao merged pull request #3640: HDDS-7069. EC: ReplicationManager - Track nodes already used when handing under replication
Posted by GitBox <gi...@apache.org>.
umamaheswararao merged PR #3640:
URL: https://github.com/apache/ozone/pull/3640
--
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] umamaheswararao commented on a diff in pull request #3640: HDDS-7069. EC: ReplicationManager - Track nodes already used when handing under replication
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3640:
URL: https://github.com/apache/ozone/pull/3640#discussion_r934565557
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECUnderReplicationHandler.java:
##########
@@ -186,15 +191,17 @@ public Map<DatanodeDetails, SCMCommand<?>> processAndCreateCommands(
Set<Integer> decomIndexes = replicaCount.decommissioningOnlyIndexes(true);
if (decomIndexes.size() > 0) {
final List<DatanodeDetails> selectedDatanodes =
- getTargetDatanodes(replicas, container, decomIndexes.size());
+ getTargetDatanodes(excludedNodes, container, decomIndexes.size());
+ excludedNodes.addAll(selectedDatanodes);
Review Comment:
ok got it. Thanks for the reasoning.
--
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