Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -664,15 +664,24 @@ private void cleanupFailedImport() {
if (containerData.hasSchema(OzoneConsts.SCHEMA_V3)) {
BlockUtils.removeContainerFromDB(containerData, config);
}
FileUtils.deleteDirectory(new File(containerData.getMetadataPath()));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devmadhuu , we can first move the container directory to tmp, and then delete it. If any deletion in between failed, the residual will not impact normal flow, and it will be deleted on DN restart or next time this container is downloaded for import.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ChenSammi , this is better approach. I updated the patch to move the container directory to the deleted-container tmp area first, then delete it from there.

FileUtils.deleteDirectory(new File(containerData.getChunksPath()));
FileUtils.deleteDirectory(new File(getContainerData().getContainerPath()));
File containerDir = new File(getContainerData().getContainerPath());
if (containerDir.exists()) {
KeyValueContainerUtil.moveToDeletedContainerDir(containerData,
containerData.getVolume());
deleteDirectory(KeyValueContainerUtil.getTmpDirectoryPath(containerData,
containerData.getVolume()).toFile());
}
} catch (Exception ex) {
LOG.error("Failed to cleanup destination directories for container {}",
containerData.getContainerID(), ex);
}
}

@VisibleForTesting
void deleteDirectory(File directory) throws IOException {
FileUtils.deleteDirectory(directory);
}

@Override
public void exportContainerData(OutputStream destination,
ContainerPacker<KeyValueContainerData> packer) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
Expand Down Expand Up @@ -79,6 +80,8 @@
import org.apache.hadoop.ozone.container.common.helpers.BlockData;
import org.apache.hadoop.ozone.container.common.impl.ContainerDataYaml;
import org.apache.hadoop.ozone.container.common.impl.ContainerLayoutVersion;
import org.apache.hadoop.ozone.container.common.interfaces.Container;
import org.apache.hadoop.ozone.container.common.interfaces.ContainerPacker;
import org.apache.hadoop.ozone.container.common.interfaces.DBHandle;
import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration;
import org.apache.hadoop.ozone.container.common.utils.DatanodeStoreCache;
Expand Down Expand Up @@ -492,6 +495,68 @@ public void testContainerImportExport(ContainerTestVersionInfo versionInfo)
}
}

@ContainerTestVersionInfo.ContainerTest
public void testFailedImportCleanupMovesContainerBeforeDelete(
ContainerTestVersionInfo versionInfo) throws Exception {
init(versionInfo);

HddsVolume containerVolume = volumeChoosingPolicy.chooseVolume(
StorageVolumeUtil.getHddsVolumesList(volumeSet.getVolumesList()), 1);

KeyValueContainer container = new KeyValueContainer(
keyValueContainerData, CONF) {
@Override
void deleteDirectory(File directory) throws IOException {
File deletedContainerDir = KeyValueContainerUtil.getTmpDirectoryPath(
getContainerData(), getContainerData().getVolume()).toFile();
if (directory.equals(deletedContainerDir)) {
throw new IOException("Injected tmp cleanup failure");
}
super.deleteDirectory(directory);
}
};
container.populatePathFields(scmId, containerVolume);

ContainerPacker<KeyValueContainerData> failingPacker =
new ContainerPacker<KeyValueContainerData>() {
@Override
public byte[] unpackContainerData(
Container<KeyValueContainerData> containerToUnpack,
InputStream inputStream, Path tmpDir, Path destContainerDir)
throws IOException {
Files.createDirectories(new File(containerToUnpack
.getContainerData().getChunksPath()).toPath());
Files.createDirectories(new File(containerToUnpack
.getContainerData().getMetadataPath()).toPath());
throw new IOException("Injected import failure");
}

@Override
public void pack(Container<KeyValueContainerData> containerToPack,
OutputStream destination) {
}

@Override
public byte[] unpackContainerDescriptor(InputStream inputStream) {
return null;
}
};

assertThrows(IOException.class, () -> container.importContainerData(
new ByteArrayInputStream(new byte[0]), failingPacker));

assertFalse(new File(container.getContainerData().getContainerPath())
.exists());
File deletedContainerDir = KeyValueContainerUtil.getTmpDirectoryPath(
container.getContainerData(), container.getContainerData().getVolume())
.toFile();
assertTrue(deletedContainerDir.exists());
assertTrue(new File(deletedContainerDir, OzoneConsts.STORAGE_DIR_CHUNKS)
.exists());
assertTrue(new File(deletedContainerDir, OzoneConsts.CONTAINER_META_PATH)
.exists());
}

private void checkContainerFilesPresent(KeyValueContainerData data,
long expectedNumFilesInChunksDir) throws IOException {
File chunksDir = new File(data.getChunksPath());
Expand Down