HDDS-8333. ReplicationManager: Allow partial EC reconstruction if insufficient nodes available

[sodonnel]

What changes were proposed in this pull request?

If an EC container is missing multiple indexes, and there are not enough nodes to recover all indexes, the process currently fails. It could be possible to recover some of the indexes and it will reduce the chances of data loss.

Therefore, if we cannot select enough nodes to recover all indexes, we should try to select enough nodes to recover N - 1 indexes, or N - 2, as recovering some of them is better than nothing.

What is the link to the Apache JIRA

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

How was this patch tested?

New unit tests added and some existing tests modified to handle the change in behavior of throwing an exception on partial success.

[umamaheswararao]

Generally this works. But shouldn't we fix this kind of checks, so we can avoid this loop?

if (chosenNodes.size() < additionalRacksRequired) {
      String reason = "Chosen nodes size from Unique Racks: " + chosenNodes
              .size() + ", but required nodes to choose from Unique Racks: "
              + additionalRacksRequired + " do not match.";
      LOG.warn("Placement policy could not choose the enough nodes from " +
                      "available racks. {} Available racks count: {},"
                      + " Excluded nodes count: {}",
              reason, racks.size(), excludedNodesCount);
      throw new SCMException(reason,
              SCMException.ResultCodes.FAILED_TO_FIND_HEALTHY_NODES);
    }

[sodonnel]

I don't understand what you mean by this comment?

[swamirishi]

We can remove this if check to handle it instead & not have a loop. We can change the definition of the chooseDatanode function in placement policy to return a max number of nodes adhering to the placement policy.

[sodonnel]

I'm not going to start with changing the placement policy now. It is called from too many places, and it requires changing all the placement policies, making it a rather large change. As it stands the placement policy contract is that is returns all the requested nodes or an exception, and I am happy to keep it that way for now. This loop is reused in a few places in this utility function, and therefore it is nicely contained in one place now.

[umamaheswararao]

This loop is reused in a few places in this utility function, and therefore it is nicely contained in one place now.

If this is reused, then right place should be placement policy to fix? Probably have separate API with this contract. Currently it's like throw exception catch it and try again, even though chosen node end up getting same result inside policy.

Anyway, this is committed now. it works functionally. Does it loop even chosenNodes 0 and throw exception out from policy?