umamaheswararao commented on a change in pull request #3128:
URL: https://github.com/apache/ozone/pull/3128#discussion_r820870123
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockOutputStreamEntry.java
##########
@@ -338,30 +334,16 @@ private boolean isFailed(
= null;
try {
containerCommandResponseProto = chunkWriteResponseFuture != null ?
- chunkWriteResponseFuture.get() :
- null;
+ chunkWriteResponseFuture.get() : null;
} catch (InterruptedException e) {
outputStream.setIoException(e);
Thread.currentThread().interrupt();
} catch (ExecutionException e) {
outputStream.setIoException(e);
}
- if ((outputStream != null && containerCommandResponseProto != null)
- && (outputStream.getIoException() != null || isStreamFailed(
- containerCommandResponseProto, outputStream))) {
- return true;
- }
- return false;
- }
-
- boolean isStreamFailed(
- ContainerProtos.ContainerCommandResponseProto responseProto,
- ECBlockOutputStream stream) {
- try {
- ContainerProtocolCalls.validateContainerResponse(responseProto);
- } catch (StorageContainerException sce) {
- stream.setIoException(sce);
+ if (containerCommandResponseProto != null
Review comment:
I think here it seems there is a problem right?
It's not from this patch, but existing code has issue looks like.
Let's say when we catch ExecutionException above, we set the IOEception on
stream, but containerCommandResponseProto will be null right. So, the below
case will not be evaluated. and we may return false. get may return null if
interrupted.Should we just log and ignore or we consider null also failed
stream?
```
/**
* Returns raw result after waiting, or null if interruptible and
* interrupted.
*/
```
So, I think we should seperate the checks...Let's keep
if(outputStream.getIoException() != null) check first and return true.
Then handle the containerCommandResponseProto == null case?
CC: @cchenax
--
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]
