adoroszlai commented on a change in pull request #1537:
URL: https://github.com/apache/ozone/pull/1537#discussion_r516801651
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/DownloadAndImportReplicator.java
##########
@@ -122,7 +125,7 @@ public void replicate(ReplicationTask task) {
LOG.info("Container {} is replicated successfully", containerID);
task.setStatus(Status.DONE);
} catch (Exception e) {
- LOG.error("Container replication was unsuccessful .", e);
+ LOG.error("Container replication was unsuccessful.", e);
Review comment:
```suggestion
LOG.error("Container {} replication was unsuccessful.", containerID,
e);
```
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/DownloadAndImportReplicator.java
##########
@@ -89,6 +91,7 @@ public void importContainer(long containerID, Path
tarFilePath) {
LOG.error(
"Can't import the downloaded container data id=" + containerID,
e);
+ throw e;
Review comment:
I think we should remove this `catch` block completely. Logging the
same exception both here and in the caller (`replicate()`) is IMO unnecessary.
##########
File path:
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/replication/TestReplicationSupervisor.java
##########
@@ -173,6 +178,36 @@ public void stalledDownload() {
}
}
+ @Test
+ public void testDownloadAndImportReplicatorFailure() {
+ ReplicationSupervisor supervisor =
+ new ReplicationSupervisor(set, mutableReplicator,
+ newDirectExecutorService());
+
+ // Mock to fetch an exception in the importContainer method.
+ SimpleContainerDownloader moc =
+ Mockito.mock(SimpleContainerDownloader.class);
+ CompletableFuture<Path> res = new CompletableFuture<>();
+ res.complete(Paths.get("file:/tmp/file"));
Review comment:
```suggestion
res.complete(Paths.get("file:/tmp/no-such-file"));
```
This would document that the expected failure is due to
`FileNotFoundException`.
----------------------------------------------------------------
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]
