umamaheswararao commented on code in PR #3743:
URL: https://github.com/apache/ozone/pull/3743#discussion_r970276376
##########
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:
Please file the JIRA if you have in TODO list. thanks
In fact, we have two threads precessing currently. I don't see a very strong
reason to have two threads currently when I think about it. The amount of work
that two threads doing is not a really heavy. Probably we should think to merge
them as well. Unless you have some strong points to have two threads. Anyway
that's different topic, I don't want to go into that here in this JIRA.
--
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]
