HDDS-4511: Avoiding StaleNodeHandler to take effect in TestDeleteWithSlowFollower.

[GlenGeng-awx]

What changes were proposed in this pull request?

This change is inspired by avoiding StaleNodeHandler to take effect in TestDeleteWithSlowFollower.

Consider the follow logic in RM,

      /*
       * We don't take any action if the container is in OPEN state and
       * the container is healthy. If the container is not healthy, i.e.
       * the replicas are not in OPEN state, send CLOSE_CONTAINER command.
       */
      if (state == LifeCycleState.OPEN) {
        if (!isContainerHealthy(container, replicas)) {
          eventPublisher.fireEvent(SCMEvents.CLOSE_CONTAINER, id);
        }
        return;
      }

  private boolean isContainerHealthy(final ContainerInfo container,
                                     final Set<ContainerReplica> replicas) {
    return !isContainerUnderReplicated(container, replicas) &&
        !isContainerOverReplicated(container, replicas) &&
        replicas.stream().allMatch(
            r -> compareState(container.getState(), r.getState()));
  }

  private boolean isContainerUnderReplicated(final ContainerInfo container,
      final Set<ContainerReplica> replicas) {
    if (container.getState() != LifeCycleState.CLOSED &&
        container.getState() != LifeCycleState.QUASI_CLOSED) {
      return false;
    }
    boolean misReplicated = !getPlacementStatus(
        replicas, container.getReplicationFactor().getNumber())
        .isPolicySatisfied();
    return container.getReplicationFactor().getNumber() >
        getReplicaCount(container.containerID(), replicas) || misReplicated;
  }

Consider this scenario: client triggers SCM to allocate one container by allocating blocks, then it crashes, never writes chunks to DN to trigger the creation of the container, thus no replica report for this container.

Previously, ReplicationManager will close such containers, since it is under replicated. This is a reasonable and legacy handling which is used to prevents StaleNodeHandler to take effect in TestDeleteWithSlowFollower.

This following logic is added by Sammi in her PR HDDS-4023. Delete closed container after all blocks have been deleted

    if (container.getState() != LifeCycleState.CLOSED &&
        container.getState() != LifeCycleState.QUASI_CLOSED) {
      return false;
    }

After talked with @ChenSammi , by design, it just needs to explicitly avoid replicating container in DELETING or DELETED state.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-4511

How was this patch tested?

CI

[linyiqun]

@GlenGeng , not fully get the reason for doing this. If we enable replication for open state containers, how we keep data consistent when container be in replication and meanwhile there is block written in this container.

[bshashikant]

@GlenGeng , is the description of the pull request correct?

[GlenGeng-awx]

@linyiqun @bshashikant Thanks for the review!

Shashi, I found this issue when trying to debug the test case TestDeleteWithSlowFollower for SCM HA, which is written by you. You can find more details in the jira's description.

The key point is, although this case can pass in master, after HDDS-4023. Delete closed container after all blocks have been deleted., there will be log from stale node hander, which is by design not correct.

This case relies on RM to make the container to CLOSING state, so that SCM can move container state to CLOSED, and not hold the delete blocks command.

There are two solutions to avoid the stale node issue, one is shorten the frequency of RM in the case TestDeleteWithSlowFollower, the other is the above changes.

Actually, this fix would work in the version of Oct 20, which is the latest merge from master to HDDS-2823, But seems some change after Oct 20 affect the case TestDeleteWithSlowFollower again.

[GlenGeng-awx]

The description may not be accurate enough, I would update them later. Thanks for pointing out !

[GlenGeng-awx]

@linyiqun Hey yiqun, never mind, RM will only send out replicate container command for closed container.

[linyiqun]

@linyiqun Hey yiqun, never mind, RM will only send out replicate container command for closed container.

Okay, get it.

[GlenGeng-awx]

@ChenSammi Please take a look at this PR. Thanks !

[ChenSammi]

The patch LGTM, +1.

Thanks @GlenGeng for investigating and fix the issue, and @linyiqun @bshashikant for the review.