adoroszlai commented on a change in pull request #2196:
URL: https://github.com/apache/ozone/pull/2196#discussion_r628170059
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/TarContainerPacker.java
##########
@@ -152,6 +156,20 @@ public void pack(Container<KeyValueContainerData>
container,
try (OutputStream compressed = compress(output);
ArchiveOutputStream archiveOutput = tar(compressed)) {
+ if (!containerData.getDbFile().exists()) {
+ LOG.warn("DBfile {} not exist",
+ containerData.getDbFile().toPath().toString());
+ return;
+ } else if (!new File(containerData.getChunksPath()).exists()) {
+ LOG.warn("Chunkfile {} not exist",
+ containerData.getDbFile().toPath().toString());
+ return;
+ } else if (!container.getContainerFile().exists()) {
+ LOG.warn("Containerfile {} not exist",
+ containerData.getDbFile().toPath().toString());
+ return;
+ }
+
Review comment:
Would it make sense to utilize `Container.scanMetadata()` instead? More
complete checks without introducing duplication.
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/DownloadAndImportReplicator.java
##########
@@ -121,10 +121,17 @@ public void replicate(ReplicationTask task) {
LOG.info("Container {} is downloaded with size {}, starting to
import.",
containerID, bytes);
task.setTransferredBytes(bytes);
-
- importContainer(containerID, path);
- LOG.info("Container {} is replicated successfully", containerID);
- task.setStatus(Status.DONE);
+ // if tar is null, the tar size is 45 bytes
+ if (bytes <= 45) {
+ task.setStatus(Status.FAILED);
+ LOG.warn("Container {} is downloaded with size {}, " +
+ "if size less than 45 bytes the tar file is null",
+ containerID, bytes);
Review comment:
I think these kind of tar-specific details should be handled by
`TarContainerPacker`.
##########
File path:
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
##########
@@ -238,6 +239,55 @@ public void testContainerImportExport() throws Exception {
}
}
+ @Test
+ public void testContainerMissingFileImportExport() throws Exception {
+ long containerId = keyValueContainer.getContainerData().getContainerID();
+ createContainer();
+ long numberOfKeysToWrite = 12;
+ closeContainer();
+ populate(numberOfKeysToWrite);
+
+ //destination path
+ File folderToExport = folder.newFile("exported.tar.gz");
+ TarContainerPacker packer = new TarContainerPacker();
+
+ //if missing chunksfile
+ File chunkfile = new File(keyValueContainer.
+ getContainerData().getChunksPath());
+ Files.delete(chunkfile.toPath());
+ Assert.assertFalse(chunkfile.exists());
+ //export the container
+ try (FileOutputStream fos = new FileOutputStream(folderToExport)) {
+ keyValueContainer
+ .exportContainerData(fos, packer);
+ }
+
+ //delete the original one
+ keyValueContainer.delete();
+
+ //create a new one
+ KeyValueContainerData containerData =
+ new KeyValueContainerData(containerId,
+ keyValueContainerData.getLayOutVersion(),
+ keyValueContainerData.getMaxSize(), UUID.randomUUID().toString(),
+ datanodeId.toString());
+ KeyValueContainer container = new KeyValueContainer(containerData, CONF);
+
+ HddsVolume containerVolume = volumeChoosingPolicy.chooseVolume(volumeSet
+ .getVolumesList(), 1);
+ String hddsVolumeDir = containerVolume.getHddsRootDir().toString();
+
+ container.populatePathFields(scmId, containerVolume, hddsVolumeDir);
+ long bytes = Files.size(folderToExport.toPath());
+ Assert.assertTrue(bytes <= 45);
+
+ try (FileInputStream fis = new FileInputStream(folderToExport)) {
+ container.importContainerData(fis, packer);
+ } catch (Exception ex) {
+ assertTrue(ex instanceof NullPointerException);
Review comment:
* This passes even if `importContainerData` does not throw, which is not
expected.
* Some exception other than `NullPointerException` may better describe the
error.
* `Assert.assertThrows` or `LambdaTestUtils.intercept` could provide more
details if the assertion fails (eg. if not the expected kind of exception was
thrown).
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
