HDDS-15005. Delete EC replica when container state is DELETING in SCM#10255
HDDS-15005. Delete EC replica when container state is DELETING in SCM#10255sarvekshayr wants to merge 2 commits into
Conversation
sreejasahithi
left a comment
There was a problem hiding this comment.
Thanks @sarvekshayr for the patch
| } | ||
| // HDDS-12421: fall-through to case DELETING | ||
| case DELETING: | ||
| if (replicationType.equals(HddsProtos.ReplicationType.EC) && !replicaIsEmpty) { |
There was a problem hiding this comment.
I'm not sure if (!replicaIsEmpty) is needed here.
@sodonnel , could you take a look of this patch?
There was a problem hiding this comment.
Both EmptyContainerHandler and DeletingContainerHandler will take care of empty replicas cleanup.
There was a problem hiding this comment.
This change makes sense. However it is kind of annoying it is in the ReportHandler rather than the RM deleting handler. There is existing logic in this handler in the DELETING branch of the switch statement for both Ratis and EC, so it makes sense to put in here.
A container transitions to deleting when all its reported replicas are empty. Then it transitions to deleted when all the replicas are gone. If a non empty replica appears after it has transitioned to deleting then it will block the deleting to deleted transition as the RM code will not remove the replica as its non-empty.
What is also interesting is that if the non-empty container appears before the container goes from CLOSED to DELETING, then it would block the container going to DELETING and hence clearing out the other empty replicas. In that case there should be an over replicated index, but I am not sure which RM would remove.
What changes were proposed in this pull request?
When EC container state is
DELETINGin SCM, and any EC non-empty replica reports back to SCM, SCM should delete this replica as this replica will remain in orphan state. This can happen when some old DN is bought back which was containing some replica.What is the link to the Apache JIRA
HDDS-15005
How was this patch tested?
Added unit and integration tests.