HDDS-12722. EC duplicate replica handling for same index in datanodes#8561
HDDS-12722. EC duplicate replica handling for same index in datanodes#8561sumitagrawl wants to merge 5 commits intoapache:masterfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request addresses the handling of duplicate EC containers with the same replica index by ensuring that the duplicate is deleted. Key changes include:
- Addition of a new test (testEcContainerDuplicateWithSameIndex) in TestContainerReader.java to simulate and verify the duplicate container deletion.
- Modification of the duplicate resolution logic in ContainerReader.java to only delete a container if both have the same replica index.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java | Added a new test to verify behavior when duplicate EC containers with the same replica index exist. |
| hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java | Updated duplicate resolution logic to allow both containers to remain when replica indexes differ and delete the duplicate only when they are identical. |
Comments suppressed due to low confidence (1)
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java:273
- Consider adding a comment clarifying that duplicate containers will only be deleted when both have the same replica index, while distinct replica indexes indicate intentional replication and are therefore both retained.
if (existing.getContainerData().getReplicaIndex() != 0 && existing.getContainerData().getReplicaIndex() != toAdd.getContainerData().getReplicaIndex()) {
...r-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks for the patch @sumitagrawl changes look good to me however i have some doubts on deleting a replica randomly. We should have some guardrails checking block files and their length could be a good check. This should not be that expensive an operation to do. Iterate blockTable and ls on the containerDirectory
| existing.getContainerData().getContainerPath(), | ||
| toAdd.getContainerData().getContainerPath()); | ||
| if (existing.getContainerData().getReplicaIndex() != 0 && | ||
| existing.getContainerData().getReplicaIndex() != toAdd.getContainerData().getReplicaIndex()) { |
There was a problem hiding this comment.
Even if it is equal we have not swapped here right? If 2 replica indices are equal do we want to remove one replica only if the block files are same? Given that this is a delete operation IMO we should be a bit careful
There was a problem hiding this comment.
Its being swapped based on State as down logic after if, for index same,
- if existing is closed, given preference to this
- else if new is closed, given preference to this
- else preference is given to existing which is already loaded.
|
As per discussion in community meeting, delete duplicate container for same index will be on hold till Reconcillation feature is implemented and handles reconcilling of duplicate container on same node. |
|
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days. |
|
Thank you for your contribution. This PR is being closed due to inactivity. If needed, feel free to reopen it. |
What changes were proposed in this pull request?
For EC container, if duplicate container is present, having same index, then it will delete the duplicate container.
Other case for different index, it will be handled as part of HDDS-13174
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-12722
How was this patch tested?