errose28 commented on code in PR #7985:
URL: https://github.com/apache/ozone/pull/7985#discussion_r1976319455
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/MismatchedReplicasHandler.java:
##########
@@ -57,35 +58,28 @@ public MismatchedReplicasHandler(
*/
@Override
public boolean handle(ContainerCheckRequest request) {
- ContainerInfo containerInfo = request.getContainerInfo();
- Set<ContainerReplica> replicas = request.getContainerReplicas();
+ if (request.isReadOnly()) {
+ return false;
+ }
+
+ final ContainerInfo containerInfo = request.getContainerInfo();
+ final Set<ContainerReplica> replicas = request.getContainerReplicas();
+
if (containerInfo.getState() != HddsProtos.LifeCycleState.CLOSED &&
containerInfo.getState() != HddsProtos.LifeCycleState.QUASI_CLOSED) {
// Handler is only relevant for CLOSED or QUASI-CLOSED containers.
return false;
}
- LOG.debug("Checking container {} in MismatchedReplicasHandler",
- containerInfo);
- if (request.isReadOnly()) {
- return false;
- }
- // close replica if needed
- for (ContainerReplica replica : replicas) {
- if (shouldBeClosed(containerInfo, replica)) {
- LOG.debug("Sending close command for mismatched replica {} of " +
- "container {}.", replica, containerInfo);
+ LOG.debug("Checking container {} in MismatchedReplicasHandler",
containerInfo);
- if (containerInfo.getState() == HddsProtos.LifeCycleState.CLOSED) {
- replicationManager.sendCloseContainerReplicaCommand(
- containerInfo, replica.getDatanodeDetails(), true);
- } else if (containerInfo.getState() ==
- HddsProtos.LifeCycleState.QUASI_CLOSED) {
- replicationManager.sendCloseContainerReplicaCommand(
- containerInfo, replica.getDatanodeDetails(), false);
- }
- }
- }
+ final Predicate<ContainerReplica> shouldSendClose = (r) ->
shouldSendClose(containerInfo, r);
+
+ replicas.stream().filter(shouldSendClose).forEach(r -> {
+ LOG.debug("Sending close command for mismatched replica {} of container
{}.", r, containerInfo);
+ replicationManager.sendCloseContainerReplicaCommand(
+ containerInfo, r.getDatanodeDetails(),
shouldForceClose(containerInfo, r));
Review Comment:
We should look at this in context with the [datanode
flow](https://github.com/apache/ozone/blob/39437eae0cbc4de6078ca16f80d5c579d5a93fbf/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/CloseContainerCommandHandler.java#L116).
A non-force close to an open or closing replica will still try to use the
pipeline if it is available. Only if that fails on the DN will it quasi-close
the container. Then we can invoke the force flow from SCM. Since we are
checking the BCSID to decide whether to force close or not, we don't
necessarily need the pipeline though.
I think both approaches give the correct result. However I think it makes
sense to at least try a non-force close in case the pipeline is still there and
let the DN tell us if force is required by moving the replica to quasi-closed.
Otherwise it feels like we skipped a step.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
