You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/08/17 06:59:30 UTC
[GitHub] [ozone] umamaheswararao commented on a diff in pull request #3667: HDDS-7092. EC Offline Recovery with simultaneous Over Replication
umamaheswararao commented on code in PR #3667:
URL: https://github.com/apache/ozone/pull/3667#discussion_r947503736
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECOverReplicationHandler.java:
##########
@@ -74,23 +75,26 @@ public Map<DatanodeDetails, SCMCommand<?>> processAndCreateCommands(
Set<ContainerReplica> replicas, List<ContainerReplicaOp> pendingOps,
ContainerHealthResult result, int remainingMaintenanceRedundancy) {
ContainerInfo container = result.getContainerInfo();
- ContainerHealthResult currentUnderRepRes = ecContainerHealthCheck
+ Optional<ContainerHealthResult> currentUnderRepRes = ecContainerHealthCheck
.checkHealth(container, replicas, pendingOps,
- remainingMaintenanceRedundancy);
+ remainingMaintenanceRedundancy,
+ Optional.of(ContainerHealthResult.HealthState.OVER_REPLICATED));
LOG.debug("Handling over-replicated EC container: {}", container);
//sanity check
- if (currentUnderRepRes.getHealthState() !=
- ContainerHealthResult.HealthState.OVER_REPLICATED) {
+ if (currentUnderRepRes.map(containerHealthResult ->
+ containerHealthResult.getHealthState()
+ != ContainerHealthResult.HealthState.OVER_REPLICATED)
+ .orElse(true)) {
LOG.info("The container {} state changed and it's not in over"
- + " replication any more. Current state is: {}",
- container.getContainerID(), currentUnderRepRes);
+ + " replication any more. Current state is: {}",
Review Comment:
Are there tabs here?
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerReplicaCount.java:
##########
@@ -342,6 +342,10 @@ public List<Integer> overReplicatedIndexes(boolean includePendingDelete) {
return indexes;
}
Review Comment:
right now you are using it only for size? why it is returning set. Is there any other plans to use? Just healthy index count may work?
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECContainerHealthCheck.java:
##########
@@ -67,20 +83,23 @@ public ContainerHealthResult checkHealth(ContainerInfo container,
dueToDecommission = false;
remainingRedundancy = repConfig.getParity() - missingIndexes.size();
}
- return new ContainerHealthResult.UnderReplicatedHealthResult(
- container, remainingRedundancy, dueToDecommission,
- replicaCount.isSufficientlyReplicated(true),
- replicaCount.isUnrecoverable());
+ containerHealthResults.add(
+ new ContainerHealthResult.UnderReplicatedHealthResult(
+ container, remainingRedundancy, dueToDecommission,
+ replicaCount.isSufficientlyReplicated(true),
+ replicaCount.isUnrecoverable()));
}
if (replicaCount.isOverReplicated(false)) {
List<Integer> overRepIndexes = replicaCount.overReplicatedIndexes(false);
- return new ContainerHealthResult
+ containerHealthResults.add(new ContainerHealthResult
.OverReplicatedHealthResult(container, overRepIndexes.size(),
- !replicaCount.isOverReplicated(true));
+ !replicaCount.isOverReplicated(true)));
}
- // No issues detected, so return healthy.
- return new ContainerHealthResult.HealthyResult(container);
+ // If No issues detected, return healthy.
Review Comment:
Nit typo: No -> no
--
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