sodonnel commented on code in PR #3545:
URL: https://github.com/apache/ozone/pull/3545#discussion_r907203555
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -233,24 +229,77 @@ public synchronized void processAll() {
final List<ContainerInfo> containers =
containerManager.getContainers();
ReplicationManagerReport report = new ReplicationManagerReport();
+ List<ContainerHealthResult.UnderReplicatedHealthResult> underReplicated =
+ new ArrayList<>();
+ List<ContainerHealthResult.OverReplicatedHealthResult> overReplicated =
+ new ArrayList<>();
+
for (ContainerInfo c : containers) {
if (!shouldRun()) {
break;
}
- switch (c.getReplicationType()) {
- case EC:
- break;
- default:
+ report.increment(c.getState());
+ if (c.getReplicationType() != EC) {
legacyReplicationManager.processContainer(c, report);
+ continue;
+ }
+ try {
+ processContainer(c, underReplicated, overReplicated, report);
+ // TODO - send any commands contained in the health result
+ } catch (ContainerNotFoundException e) {
+ LOG.error("Container {} not found", c.getContainerID(), e);
}
}
report.setComplete();
+ // TODO - Sort the pending lists by priority and assign to the main queue,
+ // which is yet to be defined.
this.containerReport = report;
LOG.info("Replication Monitor Thread took {} milliseconds for" +
" processing {} containers.", clock.millis() - start,
containers.size());
}
+ protected ContainerHealthResult processContainer(ContainerInfo containerInfo,
+ List<ContainerHealthResult.UnderReplicatedHealthResult> underRep,
+ List<ContainerHealthResult.OverReplicatedHealthResult> overRep,
+ ReplicationManagerReport report) throws ContainerNotFoundException {
+ Set<ContainerReplica> replicas;
+ replicas = containerManager.getContainerReplicas(
+ containerInfo.containerID());
+ List<ContainerReplicaOp> pendingOps =
+ containerReplicaPendingOps.getPendingOps(containerInfo.containerID());
+ ContainerHealthResult health = ecContainerHealthCheck
+ .checkHealth(containerInfo, replicas, pendingOps, 0);
+ // 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, containerInfo.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, containerInfo.containerID());
+ }
+ if (!underHealth.isSufficientlyReplicatedAfterPending() &&
+ !underHealth.isUnrecoverable()) {
+ underRep.add(underHealth);
+ }
+ } else if (health.getHealthState()
Review Comment:
For now I thought it would be easier to have only a single state. The worse
case is under-replication, so ideally we fix that first. When that is fixed,
the container will get processed again and fix the over-replication. So yes,
the if the container is both over and under replicated, under-replicated will
be the result and over will be ignored until it is fixed.
I guess there is an edge case, where the container is both missing
(unrecoverable) and over-replicated. The missing will never get fixed and it
will be stuck like that. I am not sure what the answer is here - probably the
container needs to be removed from the system as it cannot be read anyway.
--
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]
