You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "xBis7 (via GitHub)" <gi...@apache.org> on 2023/12/04 14:49:24 UTC
[PR] HDDS-9683. Containers belonging to decommissioning or decommissioned nodes, are counted as mis-replicated [ozone]
xBis7 opened a new pull request, #5726:
URL: https://github.com/apache/ozone/pull/5726
## What changes were proposed in this pull request?
When a node is decommissioning, new replicas are being copied to other nodes and once this process has finished, then the node goes into decommission. After the copies are made, the container appears as mis-replicated due to the excessive replicas. These replicas are unavailable and the decommissioned node is expected to be stopped. For that reason, containers that belong to decommissioning or decommissioned nodes, shouldn't be counted as mis-replicated.
Nodes in maintenance, won't be filtered because such nodes are expected to come back and no replica copies are made while entering the state. Mis-replication on a node in maintenance, is the same as having mis-replication on a healthy and active node.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9683
## How was this patch tested?
New unit tests are added. It can also be tested manually with the `ozone-topology` docker env like so
* Edit `docker-config` to enable RackScatter policy (easier to reproduce with RackScatter)
* ```diff
- OZONE-SITE.XML_ozone.scm.container.placement.impl=org.apache.hadoop.hdds.scm.container.placement.algorithms.SCMContainerPlacementRackAware
+ #OZONE-SITE.XML_ozone.scm.container.placement.impl=org.apache.hadoop.hdds.scm.container.placement.algorithms.SCMContainerPlacementRackAware
+
+ OZONE-SITE.XML_ozone.scm.container.placement.impl=org.apache.hadoop.hdds.scm.container.placement.algorithms.SCMContainerPlacementRackScatter
+ OZONE-SITE.XML_ozone.scm.pipeline.placement.impl=org.apache.hadoop.hdds.scm.container.placement.algorithms.SCMContainerPlacementRackScatter
+
+ # For decommission
+ OZONE-SITE.XML_ozone.scm.nodes.scmservice=scm
+ OZONE-SITE.XML_ozone.scm.address.scmservice.scm=scm
+
+ # Expedite the container replication checking
+ OZONE-SITE.XML_hdds.scm.replication.thread.interval=15s
```
* Edit `network-config`
* ```diff
- 10.5.0.6 /rack1
+ 10.5.0.6 /rack2
10.5.0.7 /rack2
- 10.5.0.8 /rack2
+ 10.5.0.8 /rack3
- 10.5.0.9 /rack2
+ 10.5.0.9 /rack3
```
* Create a key with replication Ratis THREE
* `ozone sh key put /vol1/bucket1/key1 /etc/hosts -t=RATIS -r=THREE`
* Find the replicas for container 1 and decommission one of the replica nodes
* ```
bash-4.2$ ozone admin container info 1
get 1 datanode
bash-4.2$ ozone admin scm roles
copy SCM IP
bash-4.2$ ozone admin datanode list
copy datanode IP
bash-4.2$ ozone admin datanode decommission -id=scmservice --scm=172.23.0.2:9894 172.23.0.8/ozone-datanode-2.ozone_default
Started decommissioning datanode(s):
172.23.0.8/ozone-datanode-2.ozone_default
```
* Once the node is decommissioned, check the SCM container report `ozone admin container report` and check Recon container page.
--
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-9683. Containers belonging to decommissioning or decommissioned nodes, are counted as mis-replicated [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #5726:
URL: https://github.com/apache/ozone/pull/5726#issuecomment-1839122085
cc @kerneltime
--
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-9683. Containers belonging to decommissioning or decommissioned nodes, are counted as mis-replicated [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1423724346
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackAware.java:
##########
@@ -563,6 +574,69 @@ public void testvalidateContainerPlacementSingleRackCluster() {
assertEquals(0, stat.misReplicationCount());
}
+ @ParameterizedTest
+ @MethodSource("decommissionNodeStates")
Review Comment:
Nit: can use existing method from `NodeStatus`:
https://github.com/apache/ozone/blob/698e2f4ffc2fd60a6164ffd541e7400d4fa2a727/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStatus.java#L60-L62
as already used in:
https://github.com/apache/ozone/blob/698e2f4ffc2fd60a6164ffd541e7400d4fa2a727/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisContainerReplicaCount.java#L212-L214
(`MethodSource` allows any collection (does not have to use `Stream`), and for single parameter we can use the type directly, no need to wrap i n`Arguments`.)
```suggestion
@MethodSource("org.apache.hadoop.hdds.scm.node.NodeStatus#decommissionStates")
```
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java:
##########
@@ -569,6 +579,67 @@ public void testValidateContainerPlacement(int datanodeCount) {
assertEquals(0, stat.misReplicationCount());
}
+ @ParameterizedTest
+ @MethodSource("decommissionNodeStates")
Review Comment:
```suggestion
@MethodSource("org.apache.hadoop.hdds.scm.node.NodeStatus#decommissionStates")
```
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1431617961
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
@sodonnel Since we are allowing over-replication, Should we also extend the check in `ContainerPlacementStatusDefault#isPolicySatisfied()` to something like this
```java
cnt <= maxReplicasPerRack && maxReplicasPerRack - cnt <= 1);
```
so that we can report mis-replication if there is too much uneven distribution.
In the above example,
> I think that one reason the max-per-rack was brought it, was for EC + Rack Scatter. Say you have 3 racks and EC 3-2. The idea distribution is 2, 2, 1. However 3, 1, 1 is also possible,
3, 1, 1 won't be reported as mis-replication with the new code addition. But if we check that the replica difference between racks isn't more than 1, we can identify such cases.
What do you think? This check might make sense only for EC containers.
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1426582091
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
What I am saying is that we should not filter nodes here, but adjust the max-nodes-per-rack higher if there are more replicas passed in. There is the decommission scenario, maintenance, plus there are scenarios in replication manager where we maintain extra copies of unhealthy replicas too, so you could get 3 x in_service plus 1 or more unhealthy.
If you have 3 replicas, then the expectation is 2 racks and a max of 2 per rack. But what if there are 4 replicas for rep-factor 3. Then you need 2 or 3 racks. Plus at most 3 per rack.
So if you adjust the max per rack higher based on the number of replicas available, then I think it would fix the problem.
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5726:
URL: https://github.com/apache/ozone/pull/5726#issuecomment-1874483250
Thanks @xBis7 for the patch, @kerneltime, @sodonnel 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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1439606872
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
3, 1, 1 should still be reported as mis-replicated for EC-3-2, as there should be 5 replicas. There is no excess / over replication in this example.
If we had 4, 1, 1 - this has an excess of 1. It also has an idea max-per-rack of 2, but we adjust it 1 higher due to the excess to 3, and hence it should be reported as mis-replicated. However 3, 2, 1 would not be.
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1425775267
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
> Again here, I wonder about maintenance replicas. It would be possible for there to be extra copies of maintenance replicas in the cluster, like with decommission, and until the node goes offline, there would be too many replicas.
Are you saying that we should occasionally filter maintenance replicas as well? We skip them, since we expect a maintenance node to not be stopped.
> But as the container is effectively over-replicated, we have to adjust the max nodes per rack upwards by the delta between the actual replica count and expected replicas.
>
> So if we add to that max per rack Math.max(0, dns.size() - replicas) = 1, then we get 3 allowed per rack, which means the policy would not be violated any more.
I don't understand this part, can you explain it more? Where can I find it?
In your example, `(3xin_service 1xdecommissioning)` the container isn't over-replicated for SCM. The replica on the decommissioned node isn't counted. But the placement policy will say that the container is mis-replicated. As long as the SCM considers the container properly replicated, it makes sense to me to filter any replicas on offline nodes, in the policy as well.
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1431617961
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
@sodonnel Since we are allowing over-replication, I think we should also extend the check in `ContainerPlacementStatusDefault#isPolicySatisfied()` to something like this
```java
cnt <= maxReplicasPerRack && maxReplicasPerRack - cnt <= 1);
```
so that we can report mis-replication if there is too much uneven distribution.
In the above example,
> I think that one reason the max-per-rack was brought it, was for EC + Rack Scatter. Say you have 3 racks and EC 3-2. The idea distribution is 2, 2, 1. However 3, 1, 1 is also possible,
3, 1, 1 won't be reported as mis-replication with the new code addition. But if we check that the replica difference between racks isn't more than 1, we can identify such cases.
What do you think?
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1431359472
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
> Since over-replication shouldn't be considered as mis-replication, we should adjust the max replicas per node every time there are excessive replicas, right?
Yea, I think so. We shouldn't need to consider decommission or maintenance in here. If some caller wanted to exclude decom / maint for some reason, it can filter the replicas before it passes them into the method.
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1424125045
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
Again here, I wonder about maintenance replicas. It would be possible for there to be extra copies of maintenance replicas in the cluster, like with decommission, and until the node goes offline, there would be too many replicas.
The problem here, is not the number of racks, but that we are violating the "max nodes per rack". The reason is that we pass in the expected number of nodes (repFactor) and the list of replicas, and the list of replicas is greater than the expected number.
In this case, we pass in 4 replicas (3xin_service 1xdecommissioning) and say we expect 3 (repFactor).
Then the calculation of max nodes per rack says:
```
protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) {
return numReplicas / numberOfRacks
+ Math.min(numReplicas % numberOfRacks, 1);
}
```
So if we have 3 replicas, we get:
```
3 / 2 + min( 3 % 2)=1, 1), so 1 + 1 = 2.
```
But as the container is effectively over-replicated, we have to adjust the max nodes per rack upwards by the delta between the actual replica count and expected replicas.
So if we add to that max per rack Math.max(0, dns.size() - replicas) = 1, then we get 3 allowed per rack, which means the policy would not be violated any more.
Do you think that change would fix it in the general case for any type of excess replica count?
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1431383058
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
ok, thanks. I'll add some tests and push the changes.
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1431339230
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
@sodonnel Thanks for the explanation. I agree. I'll add all that as a comment because other people might have the same questions and confusion.
Since over-replication shouldn't be considered as mis-replication, we should adjust the max replicas per node every time there are excessive replicas, right? We don't need to check for decommission or maintenance to perform the operation.
I'm referring to something like this
```java
int maxReplicasPerRack = getMaxReplicasPerRack(replicas,
Math.min(requiredRacks, numRacks));
boolean offlineNodeReplicas = dns.stream()
.anyMatch(d -> d.isMaintenance() || d.isDecommissioned());
if (offlineNodeReplicas) {
// Adjust max replicas per rack for excessive replicas belonging to offline nodes.
maxReplicasPerRack += Math.max(0, dns.size() - replicas);
}
```
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1431339230
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
@sodonnel Thanks for the explanation. I agree. I'll add all that as a comment because other people might have the same questions and confusion.
Since over-replication shouldn't be considered as mis-replication, we should adjust the max replicas per node every time there are excessive replicas, right? We don't need to check for decommission or maintenance to perform the operation.
I'm referring to something like this
```java
int maxReplicasPerRack = getMaxReplicasPerRack(replicas,
Math.min(requiredRacks, numRacks));
boolean offlineNodeReplicas = dns.stream()
.anyMatch(d -> d.isMaintenance() || d.isDecommissioned());
if (offlineNodeReplicas) {
// Adjust max replicas per rack for excessive replicas belonging to offline nodes.
maxReplicasPerRack += Math.max(0, dns.size() - replicas);
}
```
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1431617961
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
@sodonnel Since we are allowing over-replication, should we also extend the check in `ContainerPlacementStatusDefault#isPolicySatisfied()` to something like this
```java
cnt <= maxReplicasPerRack && maxReplicasPerRack - cnt <= 1);
```
so that we can report mis-replication if there is too much uneven distribution?
In the above example,
> I think that one reason the max-per-rack was brought it, was for EC + Rack Scatter. Say you have 3 racks and EC 3-2. The idea distribution is 2, 2, 1. However 3, 1, 1 is also possible,
3, 1, 1 won't be reported as mis-replication with the new code addition. But if we check that the replica difference between racks isn't more than 1, we can identify such cases.
What do you think? This check might make sense only for EC containers.
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1430672703
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
The new replication manager does not report on mis-replication when a container is over or under-replicated. It expects those conditions to be fixed first.
We have to consider what does mis-replication really mean - the original intention was to indicate that the rack tolerence of the container isn't high enough. If you have enough racks, but too many nodes on some racks due to over-replication, then the container isn't really mis-replicated in my opinion.
I think that one reason the max-per-rack was brought it, was for EC + Rack Scatter. Say you have 3 racks and EC 3-2. The idea distribution is 2, 2, 1. However 3, 1, 1 is also possible, but its not good, as if you lose the rack with 3, the EC data is no longer readable. In that case, the container would be rightly mis-replicated.
When you have over-replication, I think it is fine to report the container as over-replicated, and so long as the minimum racks is met, then it is not mis-replicated. After over-replication has been fixed, then mis-replication can come into play.
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1423820378
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackAware.java:
##########
@@ -563,6 +574,69 @@ public void testvalidateContainerPlacementSingleRackCluster() {
assertEquals(0, stat.misReplicationCount());
}
+ @ParameterizedTest
+ @MethodSource("decommissionNodeStates")
Review Comment:
> (MethodSource allows any collection (does not have to use Stream), and for single parameter we can use the type directly, no need to wrap i nArguments.)
Thanks @adoroszlai for the info. I didn't know that.
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #5726:
URL: https://github.com/apache/ozone/pull/5726
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1425775267
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
> Again here, I wonder about maintenance replicas. It would be possible for there to be extra copies of maintenance replicas in the cluster, like with decommission, and until the node goes offline, there would be too many replicas.
Are you saying that we should occasionally filter maintenance replicas as well? We skip them, since we expect a maintenance node to not be stopped.
> But as the container is effectively over-replicated, we have to adjust the max nodes per rack upwards by the delta between the actual replica count and expected replicas.
>
> So if we add to that max per rack Math.max(0, dns.size() - replicas) = 1, then we get 3 allowed per rack, which means the policy would not be violated any more.
I don't understand this part, can you explain it more? Where can I find it in the code?
In your example, `(3xin_service 1xdecommissioning)` the container isn't over-replicated for SCM. The replica on the decommissioned node isn't counted. But the placement policy will say that the container is mis-replicated. As long as the SCM considers the container properly replicated, it makes sense to me to filter any replicas on offline nodes, in the policy as well.
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1426593449
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
> So if you adjust the max per rack higher based on the number of replicas available, then I think it would fix the problem.
ok, I get it. Let me look into that. Thanks!
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1427902450
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
@sodonnel I've been looking into your suggestion. With that change, we will be ignoring mis-replication for every over-replicated container.
Let's say we have this scenario
* RackAware policy
* 7 datanodes, 3 are IN_SERVICE, 4 are offline
* 2 of the offline nodes get back IN_SERVICE
* Now we have, 5 nodes IN_SERVICE, 2 offline
* 5 available replicas and we only need 3, so the container is over-replicated
Initially
```
/rack0/node0 -> in_service - used
/rack0/node1 -> in_service - used
/rack0/node2 -> offline
/rack0/node3 -> offline
/rack0/node4 -> offline
/rack1/node5 -> in_service - used
/rack1/node6 -> offline
```
After 2 offline coming back
```
/rack0/node0 -> in_service - used
/rack0/node1 -> in_service - used
/rack0/node2 -> offline -> in_service - used
/rack0/node3 -> offline
/rack0/node4 -> offline
/rack1/node5 -> in_service - used
/rack1/node6 -> offline -> in_service - used
```
The placement will be [3, 2] where we need [2, 1]. The container is mis-replicated but because we adjust the maxReplica number by adding the replicaDelta, we get that the policy is satisfied.
`maxReplicaPerRack = 2`
`dns.size() = 7`
`replicas = 3`
`Math.max(0, dns.size() - replicas) = Math.max(0, 4) = 4` regardless of whether the excessive replicas are healthy or not.
`maxReplicaPerRack = 6`
Even if we add a check to increase the maxReplica number, only if there are decommission or maintenance nodes, it won't make a difference. We can't distinguish between replicas on offline nodes and replicas on online nodes.
This approach will effectively ignore `mis-replication` when caused by `over-replication`.
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1431617961
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
@sodonnel Since we are allowing over-replication, I think we should also extend the check in `ContainerPlacementStatusDefault#isPolicySatisfied()` to something like this
```java
cnt <= maxReplicasPerRack && maxReplicasPerRack - cnt <= 1);
```
so that we can report mis-replication if there is too much uneven distribution.
In the above example,
> I think that one reason the max-per-rack was brought it, was for EC + Rack Scatter. Say you have 3 racks and EC 3-2. The idea distribution is 2, 2, 1. However 3, 1, 1 is also possible,
3, 1, 1 won't be reported as mis-replication with the new code addition. But if we check that the replica difference between racks isn't more than 1, we can identify such cases.
What do you think? This check might make sense only for EC containers.
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1426672027
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
@sodonnel I was thinking that we need to stop reporting mis-replication in case of excessive replicas due to offline nodes. Are you saying that we should include unhealthy replicas as well? Stop reporting mis-replication if we have proper replication and the excessive number of replicas are unavailable, either due to offline nodes or being unhealthy?
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
xBis7 commented on PR #5726:
URL: https://github.com/apache/ozone/pull/5726#issuecomment-1851804504
@adoroszlai Thanks for the review. I've addressed your comments. Can you take a look?
--
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-9683. Containers present on decommission nodes are reported as mis-replicated [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #5726:
URL: https://github.com/apache/ozone/pull/5726#discussion_r1424125045
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java:
##########
@@ -445,6 +445,7 @@ public ContainerPlacementStatus validateContainerPlacement(
}
}
List<Integer> currentRackCount = new ArrayList<>(dns.stream()
+ .filter(d -> !(d.isDecommissioned()))
Review Comment:
Again here, I wonder about maintenance replicas. It would be possible for there to be extra copies of maintenance replicas in the cluster, like with decommission, and until the node goes offline, there would be too many replicas.
The problem here, is not the number of racks, but that we are violating the "max nodes per rack". The reason is that we pass in the expected number of nodes (repFactor) and the list of replicas, and the list of replicas is greater than the expected number.
In this case, we pass in 4 replicas (3xin_service 1xdecommissioning) and say we expect 3 (repFactor).
Then the calculation of max nodes per rack says:
```
protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) {
return numReplicas / numberOfRacks
+ Math.min(numReplicas % numberOfRacks, 1);
}
```
So if we have 3 replicas, we get:
```
3 / 2 + min(( 3 % 2)=1, 1), so 1 + 1 = 2.
```
But as the container is effectively over-replicated, we have to adjust the max nodes per rack upwards by the delta between the actual replica count and expected replicas.
So if we add to that max per rack Math.max(0, dns.size() - replicas) = 1, then we get 3 allowed per rack, which means the policy would not be violated any more.
Do you think that change would fix it in the general case for any type of excess replica count?
--
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