sodonnel commented on code in PR #3743:
URL: https://github.com/apache/ozone/pull/3743#discussion_r968284900
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -419,85 +431,33 @@ public long getScmTerm() throws NotLeaderException {
return scmContext.getTermOfLeader();
}
- protected ContainerHealthResult processContainer(ContainerInfo containerInfo,
+ protected void processContainer(ContainerInfo containerInfo,
Queue<ContainerHealthResult.UnderReplicatedHealthResult> underRep,
Queue<ContainerHealthResult.OverReplicatedHealthResult> overRep,
ReplicationManagerReport report) throws ContainerNotFoundException {
ContainerID containerID = containerInfo.containerID();
Set<ContainerReplica> replicas = containerManager.getContainerReplicas(
containerID);
-
- if (containerInfo.getState() == HddsProtos.LifeCycleState.OPEN) {
- if (!isOpenContainerHealthy(containerInfo, replicas)) {
- report.incrementAndSample(
- HealthState.OPEN_UNHEALTHY, containerID);
- eventPublisher.fireEvent(SCMEvents.CLOSE_CONTAINER, containerID);
- return new ContainerHealthResult.UnHealthyResult(containerInfo);
- }
- return new ContainerHealthResult.HealthyResult(containerInfo);
- }
-
- if (containerInfo.getState() == HddsProtos.LifeCycleState.CLOSED) {
- List<ContainerReplica> unhealthyReplicas = replicas.stream()
- .filter(r -> !compareState(containerInfo.getState(), r.getState()))
- .collect(Collectors.toList());
-
- if (unhealthyReplicas.size() > 0) {
- handleUnhealthyReplicas(containerInfo, unhealthyReplicas);
- }
- }
-
List<ContainerReplicaOp> pendingOps =
containerReplicaPendingOps.getPendingOps(containerID);
- ContainerHealthResult health = ecContainerHealthCheck.checkHealth(
- containerInfo, replicas, pendingOps, maintenanceRedundancy);
- // TODO - should the report have a HEALTHY state, rather than just bad
- // states? It would need to be added to legacy RM too.
- if (health.getHealthState()
- == ContainerHealthResult.HealthState.UNDER_REPLICATED) {
- report.incrementAndSample(HealthState.UNDER_REPLICATED, containerID);
- ContainerHealthResult.UnderReplicatedHealthResult underHealth
- = ((ContainerHealthResult.UnderReplicatedHealthResult) health);
- if (underHealth.isUnrecoverable()) {
- // TODO - do we need a new health state for unrecoverable EC?
- report.incrementAndSample(HealthState.MISSING, containerID);
- }
- if (!underHealth.isSufficientlyReplicatedAfterPending() &&
- !underHealth.isUnrecoverable()) {
- underRep.add(underHealth);
- }
- } else if (health.getHealthState()
- == ContainerHealthResult.HealthState.OVER_REPLICATED) {
- report.incrementAndSample(HealthState.OVER_REPLICATED, containerID);
- ContainerHealthResult.OverReplicatedHealthResult overHealth
- = ((ContainerHealthResult.OverReplicatedHealthResult) health);
- if (!overHealth.isSufficientlyReplicatedAfterPending()) {
- overRep.add(overHealth);
- }
- }
- return health;
- }
- /**
- * Handles unhealthy container.
- * A container is inconsistent if any of the replica state doesn't
- * match the container state. We have to take appropriate action
- * based on state of the replica.
- *
- * @param container ContainerInfo
- * @param unhealthyReplicas List of ContainerReplica
- */
- private void handleUnhealthyReplicas(final ContainerInfo container,
- List<ContainerReplica> unhealthyReplicas) {
- Iterator<ContainerReplica> iterator = unhealthyReplicas.iterator();
- while (iterator.hasNext()) {
- final ContainerReplica replica = iterator.next();
- final ContainerReplicaProto.State state = replica.getState();
- if (state == State.OPEN || state == State.CLOSING) {
- sendCloseCommand(container, replica.getDatanodeDetails(), true);
- iterator.remove();
- }
Review Comment:
This method performs some lookups to get the pending and replicas. I don't
think we gain much by moving this around.
I also think that long term, we probably pass a single queue object that
encapsulates the two queues, as most likely 2 queues are too simplistic. I have
a task on my todo list to look into that to see if it makes sense.
--
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]
