sodonnel commented on code in PR #5687:
URL: https://github.com/apache/hadoop/pull/5687#discussion_r1203703794
##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DataStreamer.java:
##########
@@ -916,7 +917,7 @@ void waitForAckedSeqno(long seqno) throws IOException {
try (TraceScope ignored = dfsClient.getTracer().
newScope("waitForAckedSeqno")) {
LOG.debug("{} waiting for ack for: {}", this, seqno);
- int dnodes = nodes != null ? nodes.length : 3;
+ int dnodes = nodes.length > 0 ? nodes.length : 3;
Review Comment:
I think there is still a risk of a race here. In the original problem after
nodes.length evaulated to say 3, then nodes was set to null. Then the "IF"
statement tried to call null.length, leading to the error.
In this changed code, you could evaluate nodes.length to get 3, and then
nodes is swapped to be the new `EMPTY_DATANODES`, then in the IF statement its
length will be zero, setting dnodes to zero rather than 3, which will probably
trip up the `getDatanodeWriteTimeout` method it is passed too.
To make it safe, you probably have to:
```
int currentNodes = nodes.length;
int dnodes = currentNodes > 0 ? currentNoides : 3;
```
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
