You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "devmadhuu (via GitHub)" <gi...@apache.org> on 2026/04/30 06:40:38 UTC
[PR] HDDS-15135. DN replication/reconstruction should retry another volume… [ozone]
devmadhuu opened a new pull request, #10161:
URL: https://github.com/apache/ozone/pull/10161
This change prevents DN replication/import from failing the whole target DN when the volume selected by the volume choosing policy already contains a stale container directory.
**Key changes:**
- Retry container creation on another candidate volume when KeyValueContainer#create hits an existing container directory on the selected volume.
- Add retry-aware import path in ContainerImporter for pull and push replication.
- Keep retry scoped to CONTAINER_ALREADY_EXISTS; other import failures still fail normally.
- Handling committed space reservation accounting for both initial and retry-selected volumes.
- Add unit coverage for stale selected-volume container dirs in create/import paths.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15135
## How was this patch tested?
This patch was tested using some new unit tests and some existing ones:
**Tests:**
- Added unit coverage for retrying another candidate volume when the selected volume already has the target container directory.
- Added unit coverage for retry-aware container import when the initially selected volume has a stale container directory.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #10161:
URL: https://github.com/apache/ozone/pull/10161#discussion_r3189942267
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/replication/TestContainerImporter.java:
##########
@@ -234,6 +237,53 @@ public void testImportContainerResetsLastScanTime() throws Exception {
assertEquals(Optional.empty(), containerData.lastDataScanTime());
}
+ @Test
+ public void testImportWithVolumeRetrySkipsSelectedVolumeWithContainerDir()
Review Comment:
Updated.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #10161: HDDS-15135. Import container to another volume if already exists in selected one
URL: https://github.com/apache/ozone/pull/10161
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10161:
URL: https://github.com/apache/ozone/pull/10161#issuecomment-4930778129
Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #10161:
URL: https://github.com/apache/ozone/pull/10161#issuecomment-4386043605
> Hi @devmadhuu , do we see container replication eventually failed in real case due to this? I'm not sure if we need this as it will create more same container replicas on single datanode cases, and is current duplicate replica deletion logic ready to handle this? If we are sure the existing container directory is stale, can we just delete it and continue the import?
@ChenSammi thanks for your review. This was identified as part of some use case discussed. We don't have good conflict resolution for Ratis or EC when a DN ends up with multiple copies, but we can end up in this situation due to volume failures as well. We need to prioritize moving to a safer state, which means allowing replication to pass. We do not want to end up in a situation where containers are perpetually under-replicated because no target nodes are valid.
Since replication failure is not the only way DNs can end up with duplicate replicas, we should allow replication and just pick a different volume. Better duplicate replica handling on startup may require internal reconciliation within the DN but that can be done later.
cc: @errose28
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10161:
URL: https://github.com/apache/ozone/pull/10161#issuecomment-4585255424
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.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #10161:
URL: https://github.com/apache/ozone/pull/10161#discussion_r3189943544
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ContainerImporter.java:
##########
@@ -90,53 +94,170 @@ public boolean isAllowedContainerImport(long containerID) {
public void importContainer(long containerID, Path tarFilePath,
HddsVolume targetVolume, CopyContainerCompression compression)
throws IOException {
+ markContainerImportInProgress(containerID, tarFilePath);
+
+ try {
+ checkContainerCanBeImported(containerID);
+ doImportContainer(containerID, tarFilePath, targetVolume, compression);
+ } finally {
+ importContainerProgress.remove(containerID);
+ deleteFileQuietely(tarFilePath);
Review Comment:
done.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ContainerImporter.java:
##########
@@ -90,53 +94,170 @@ public boolean isAllowedContainerImport(long containerID) {
public void importContainer(long containerID, Path tarFilePath,
HddsVolume targetVolume, CopyContainerCompression compression)
throws IOException {
+ markContainerImportInProgress(containerID, tarFilePath);
+
+ try {
+ checkContainerCanBeImported(containerID);
+ doImportContainer(containerID, tarFilePath, targetVolume, compression);
+ } finally {
+ importContainerProgress.remove(containerID);
+ deleteFileQuietely(tarFilePath);
+ }
+ }
+
+ public void importContainerWithVolumeRetry(long containerID, Path tarFilePath,
Review Comment:
done
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10161:
URL: https://github.com/apache/ozone/pull/10161#issuecomment-5299358836
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.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #10161:
URL: https://github.com/apache/ozone/pull/10161#issuecomment-5055257253
@aswinshakil @ptlrs can you help review this PR ?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #10161:
URL: https://github.com/apache/ozone/pull/10161#discussion_r3189918339
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java:
##########
@@ -260,6 +262,43 @@ protected void createContainerMetaData(File containerMetaDataPath,
assertEquals(2, callCount.get());
}
Review Comment:
@ArafatKhan2198 this comment doesn't hold right in this test class. We do mock `controller.importContainer` in `ContainerImporter#importContainerWithVolumeRetry `which belongs to `TestContainerImporter` class.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #10161: HDDS-15135. Import container to another volume if already exists in selected one
URL: https://github.com/apache/ozone/pull/10161
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #10161:
URL: https://github.com/apache/ozone/pull/10161#discussion_r3186644306
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ContainerImporter.java:
##########
@@ -90,53 +94,170 @@ public boolean isAllowedContainerImport(long containerID) {
public void importContainer(long containerID, Path tarFilePath,
HddsVolume targetVolume, CopyContainerCompression compression)
throws IOException {
+ markContainerImportInProgress(containerID, tarFilePath);
+
+ try {
+ checkContainerCanBeImported(containerID);
+ doImportContainer(containerID, tarFilePath, targetVolume, compression);
+ } finally {
+ importContainerProgress.remove(containerID);
+ deleteFileQuietely(tarFilePath);
Review Comment:
We already have a utility method for it - FileUtils.deleteQuietly
could we use it?
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java:
##########
@@ -260,6 +262,43 @@ protected void createContainerMetaData(File containerMetaDataPath,
assertEquals(2, callCount.get());
}
Review Comment:
Suggestion - DO you think the test coverage should verify non-retriable failures are not retried
The PR explicitly says retry is scoped only to CONTAINER_ALREADY_EXISTS. That behavior should have a test.
For example, mock controller.importContainer(...) to throw a normal IOException or a StorageContainerException with a different result code. Then verify only the initially selected volume was attempted.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ContainerImporter.java:
##########
@@ -90,53 +94,170 @@ public boolean isAllowedContainerImport(long containerID) {
public void importContainer(long containerID, Path tarFilePath,
HddsVolume targetVolume, CopyContainerCompression compression)
throws IOException {
+ markContainerImportInProgress(containerID, tarFilePath);
+
+ try {
+ checkContainerCanBeImported(containerID);
+ doImportContainer(containerID, tarFilePath, targetVolume, compression);
+ } finally {
+ importContainerProgress.remove(containerID);
+ deleteFileQuietely(tarFilePath);
+ }
+ }
+
+ public void importContainerWithVolumeRetry(long containerID, Path tarFilePath,
Review Comment:
We can add JavaDoc to clarify committed-space ownership.
```
/**
* Imports a container and retries on alternate volumes only when the selected
* volume already has the container directory.
*
* The caller is responsible for releasing committed bytes on
* initialTargetVolume. This method releases committed bytes only for
* retry-selected volumes.
*/
```
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/replication/TestContainerImporter.java:
##########
@@ -234,6 +237,53 @@ public void testImportContainerResetsLastScanTime() throws Exception {
assertEquals(Optional.empty(), containerData.lastDataScanTime());
}
+ @Test
+ public void testImportWithVolumeRetrySkipsSelectedVolumeWithContainerDir()
Review Comment:
The new tests cover the successful retry case, which is good. But I would also add a negative test where every candidate volume already has the stale container directory.
Expected behavior:
Import/create tries all candidate volumes.
It fails with CONTAINER_ALREADY_EXISTS.
Committed bytes are restored on all attempted volumes.
The tarball is deleted.
No container is added to containerSet.
This is important because the PR changes the final error message to “already exists on all candidate volumes”, but currently the main added tests mostly verify the happy retry path.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #10161:
URL: https://github.com/apache/ozone/pull/10161#issuecomment-4385516092
Hi @devmadhuu , do we see container replication eventually failed in real case due to this?
I'm not sure if we need this as it will create more same container replicas on single datanode cases, and is current duplicate replica deletion logic ready to handle this?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15135. Import container to another volume if already exists in selected one [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10161:
URL: https://github.com/apache/ozone/pull/10161#issuecomment-5383276051
Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org