adoroszlai commented on code in PR #3504:
URL: https://github.com/apache/ozone/pull/3504#discussion_r899132012
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java:
##########
@@ -235,46 +229,47 @@ void reconstructECBlockGroup(BlockLocationInfo
blockLocationInfo,
int readLen = sis.recoverChunks(bufs);
// TODO: can be submitted in parallel
for (int i = 0; i < bufs.length; i++) {
- targetBlockStreams[i].write(bufs[i]);
- if (isFailed(targetBlockStreams[i],
- targetBlockStreams[i].getCurrentChunkResponseFuture())) {
- // If one chunk response failed, we should retry.
- // Even after retries if it failed, we should declare the
- // reconstruction as failed.
- // For now, let's throw the exception.
- throw new IOException(
- "Chunk write failed at the new target node: "
- + targetBlockStreams[i].getDatanodeDetails()
- + ". Aborting the reconstruction process.");
- }
+ CompletableFuture<ContainerProtos.ContainerCommandResponseProto>
+ future = targetBlockStreams[i].write(bufs[i]);
+ checkFailures(targetBlockStreams[i], future);
bufs[i].clear();
}
length -= readLen;
}
- for (int i = 0; i < targetBlockStreams.length; i++) {
- try {
- targetBlockStreams[i]
+ try {
+ for (ECBlockOutputStream targetStream : targetBlockStreams) {
+ targetStream
.executePutBlock(true, true, blockLocationInfo.getLength());
- if (isFailed(targetBlockStreams[i],
- targetBlockStreams[i].getCurrentPutBlkResponseFuture())) {
- // If one chunk response failed, we should retry.
- // Even after retries if it failed, we should declare the
- // reconstruction as failed.
- // For now, let's throw the exception.
- throw new IOException(
- "Chunk write failed at the new target node: "
- + targetBlockStreams[i].getDatanodeDetails()
- + ". Aborting the reconstruction process.");
- }
- } finally {
- byteBufferPool.putBuffer(bufs[i]);
- targetBlockStreams[i].close();
+ checkFailures(targetStream,
+ targetStream.getCurrentPutBlkResponseFuture());
+ }
+ } finally {
+ for (ByteBuffer buf : bufs) {
+ byteBufferPool.putBuffer(buf);
+ }
+ for (ECBlockOutputStream targetStream : targetBlockStreams) {
+ IOUtils.cleanupWithLogger(LOG, targetStream);
}
Review Comment:
Nit: `cleanupWithLogger` accepts multiple items, so we can get rid of the
loop:
```suggestion
IOUtils.cleanupWithLogger(LOG, targetBlockStreams);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
