errose28 commented on code in PR #3920:
URL: https://github.com/apache/ozone/pull/3920#discussion_r1052912492
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/LegacyReplicationManager.java:
##########
@@ -2009,5 +1929,256 @@ private void compleleteMoveFutureWithResult(ContainerID
cid, MoveResult mr) {
inflightMoveFuture.remove(cid);
}
}
-}
+ /* HELPER METHODS FOR UNHEALTHY OVER AND UNDER REPLICATED CONTAINERS */
+
+ private void handleOverReplicatedAllUnhealthy(ContainerInfo container,
+ List<ContainerReplica> replicas, int excess) {
+ List<ContainerReplica> deleteCandidates =
+ getUnhealthyDeletionCandidates(container, replicas);
+
+ // Only unhealthy replicas which cannot be closed will remain eligible
+ // for deletion, since this method is deleting unhealthy containers only.
+ closeReplicasIfPossible(container, deleteCandidates);
+ if (deleteCandidates.isEmpty()) {
+ return;
+ }
+
+ if (container.getState() == LifeCycleState.CLOSED) {
+ // Prefer to delete unhealthy replicas with lower BCS IDs.
+ deleteExcessLowestBcsIDs(container, deleteCandidates, excess);
+ } else {
+ // Container is not yet closed.
+ // We only need to save the unhealthy replicas if they
+ // represent unique origin node IDs. If recovering these replicas is
+ // possible in the future they could be used to close the container.
+ Set<UUID> originNodeIDs = replicas.stream()
+ .map(ContainerReplica::getOriginDatanodeId)
+ .collect(Collectors.toSet());
+ deleteExcessWithNonUniqueOriginNodeIDs(container,
+ originNodeIDs, deleteCandidates, excess);
+ }
+ }
+
+ private void handleUnderReplicatedAllUnhealthy(ContainerInfo container,
+ List<ContainerReplica> replicas, ContainerPlacementStatus
placementStatus,
+ int additionalReplicasNeeded) {
+
+ int numCloseCmdsSent = closeReplicasIfPossible(container, replicas);
+ // Only replicate unhealthy containers if none of the unhealthy replicas
+ // could be closed. If we sent a close command to an unhealthy replica,
+ // we should wait for that to complete and replicate it when it becomes
+ // healthy on a future iteration.
+ if (numCloseCmdsSent == 0) {
+ // TODO Datanodes currently shuffle sources, so we cannot prioritize
+ // some replicas based on BCSID or origin node ID.
+ replicateAnyWithTopology(container,
+ getReplicationSources(container, replicas), placementStatus,
+ additionalReplicasNeeded);
+ }
+ }
+
+ private int closeReplicasIfPossible(ContainerInfo container,
+ List<ContainerReplica> replicas) {
+ int numCloseCmdsSent = 0;
+ Iterator<ContainerReplica> iterator = replicas.iterator();
+ while (iterator.hasNext()) {
+ final ContainerReplica replica = iterator.next();
+ final State state = replica.getState();
+ if (state == State.OPEN || state == State.CLOSING) {
+ sendCloseCommand(container, replica.getDatanodeDetails(), false);
+ numCloseCmdsSent++;
+ iterator.remove();
+ } else if (state == State.QUASI_CLOSED) {
+ // Send force close command if the BCSID matches
+ if (container.getSequenceId() == replica.getSequenceId()) {
+ sendCloseCommand(container, replica.getDatanodeDetails(), true);
+ numCloseCmdsSent++;
+ iterator.remove();
+ }
+ }
+ }
+
+ return numCloseCmdsSent;
+ }
+
+ /* HELPER METHODS FOR ALL OVER AND UNDER REPLICATED CONTAINERS */
+
+ private void deleteExcess(ContainerInfo container,
+ List<ContainerReplica> deleteCandidates, int excess) {
+ // Replica which are maintenance or decommissioned are not eligible to
+ // be removed, as they do not count toward over-replication and they
+ // also may not be available
+ deleteCandidates.removeIf(r ->
+ r.getDatanodeDetails().getPersistedOpState() !=
+ NodeOperationalState.IN_SERVICE);
+
+ deleteCandidates.stream().limit(excess).forEach(r ->
+ sendDeleteCommand(container, r.getDatanodeDetails(), true));
+ }
+
+ /**
+ * remove execess replicas if needed, replicationFactor and placement policy
+ * will be take into consideration.
+ *
+ * @param excess the excess number after subtracting replicationFactor
+ * @param container ContainerInfo
+ * @param eligibleReplicas An list of replicas, which may have excess
replicas
+ */
+ private void deleteExcessWithTopology(int excess,
+ final ContainerInfo container,
+ final List<ContainerReplica> eligibleReplicas) {
+ // After removing all unhealthy replicas, if the container is still over
+ // replicated then we need to check if it is already mis-replicated.
+ // If it is, we do no harm by removing excess replicas. However, if it is
+ // not mis-replicated, then we can only remove replicas if they don't
+ // make the container become mis-replicated.
+ if (excess > 0) {
+ Set<ContainerReplica> eligibleSet = new HashSet<>(eligibleReplicas);
+ final int replicationFactor =
+ container.getReplicationConfig().getRequiredNodes();
+ ContainerPlacementStatus ps =
+ getPlacementStatus(eligibleSet, replicationFactor);
+
+ for (ContainerReplica r : eligibleReplicas) {
+ if (excess <= 0) {
+ break;
+ }
+ // First remove the replica we are working on from the set, and then
+ // check if the set is now mis-replicated.
+ eligibleSet.remove(r);
+ ContainerPlacementStatus nowPS =
+ getPlacementStatus(eligibleSet, replicationFactor);
+ if (isPlacementStatusActuallyEqual(ps, nowPS)) {
+ // Remove the replica if the container was already unsatisfied
+ // and losing this replica keep actual placement count unchanged.
+ // OR if losing this replica still keep satisfied
+ sendDeleteCommand(container, r.getDatanodeDetails(), true);
+ excess -= 1;
+ continue;
+ }
+ // If we decided not to remove this replica, put it back into the set
+ eligibleSet.add(r);
+ }
+ if (excess > 0) {
+ LOG.info("The container {} is over replicated with {} excess " +
+ "replica. The excess replicas cannot be removed without " +
+ "violating the placement policy", container, excess);
+ }
+ }
+ }
+
+ private void deleteExcessWithNonUniqueOriginNodeIDs(ContainerInfo container,
+ Set<UUID> existingOriginNodeIDs, List<ContainerReplica> deleteCandidates,
+ int excess) {
Review Comment:
Good catch. I cleaned up the `deleteExcessWithNonUniqueOriginNodeIDs` method
and added a simplified version of this test
(`testQuasiClosedContainerWithUniqueUnhealthyReplica`) as part of
[913eca9](https://github.com/apache/ozone/pull/3920/commits/913eca9d79f59e182472ef3ea248c058411c6c5c)
--
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]
