What is the state of the other side of the socket connection?

@Allen Wittenauer
Unknown, I've no access to the remote system

@Allen Wittenauer
Update.

None. `netstat -ano` shown NO any connection to the client. But only a few ESTABLISHED connection from some other datanode / clients

Recheck on the client side, the connection may still remain CLOSE_WAIT.

Hi Dennis,

What is this client code doing? Is this your program which uses HDFS APIs to talk to datanode and namenode?

-Bharath

It creates a singleton FileSystem object which is connected to a HDFS cloud.
And it does nothing with hadoop but one simple thing, copy local files into hadoop.

We use only simple FileSystem.create(path, true) and IOUtils.copyLarge(). And already applied IOUtils.closeQuietly(out); in the finally block.

Nothing more than that.

Two new finding after reading the hadoop source.

1. If there were an exception between lines DFSClient.java:2883-2901. The reference to blockReplyStream may lost and remain unclosed condition.

2. processDatanodeError() will not close the previous Socket, if any.

I've run the patched 0.20.2 for a few days and the number of CLOSE_WAIT socket to port 50010 drop to zero.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12478378/patch-draft-1836.patch
against trunk revision 1100054.

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

-1 patch. The patch command could not apply the patch.

Console output: https://builds.apache.org/hudson/job/PreCommit-HDFS-Build/463//console

This message is automatically generated.

Dennis,

For me, the following code seems like an issue.

try

catch (IOException e) {
}

Reason: if blockStream throws an exception, blockReplyStream will not be closed.

Can we replace all the places (2 places) in DFSClient with the following and try?

try

catch (IOException e) {
}
try

catch (IOException e) {
}

Can you just try this change in your environment?

Retyping with format for better readability.

Change the following code

try { 
    blockStream.close(); 
    blockReplyStream.close(); 
} catch (IOException e) {
}

to this:

try { 
   blockStream.close(); 
} catch (IOException e) {
}
try { 
   blockReplyStream.close(); 
} catch (IOException e) {
}

How about using IOUtils.cleanup, which does the above but also logs errors, etc?

+1 to Todd's suggestion.

Though current code logs errors in debug mode only.

Looks like this code was fixed in trunk by HADOOP-5859

Here's a patch against branch-0.20. Bharath, can you take a look?

Thats correct. This code is part of trunk already.

Todd, One minor comment.

1. Can we also pass LOG object to this method? Users who wants to debug can enable debug option.
IOUtils.cleanup(LOG, blockStream, blockReplyStream);

Otherwise, patch looks good. Thank you.

Sure, no problem. I was trying to maintain the old behavior, but I agree it's nicer to provide the exceptions if we have debug on.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12479218/hdfs-1836-0.20.txt
against trunk revision 1103987.

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

-1 patch. The patch command could not apply the patch.

Console output: https://builds.apache.org/hudson/job/PreCommit-HDFS-Build/536//console

This message is automatically generated.

+1 to the latest hdfs-1836-0.20.txt patch for 20.

Here are test-patch results. This is just cleanup so no new tests are required. The eclipse classpath error is unrelated.

     [exec] 
     [exec] -1 overall.  
     [exec] 
     [exec]     +1 @author.  The patch does not contain any @author tags.
     [exec] 
     [exec]     -1 tests included.  The patch doesn't appear to include any new or modified tests.
     [exec]                         Please justify why no tests are needed for this patch.
     [exec] 
     [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
     [exec] 
     [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
     [exec] 
     [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
     [exec] 
     [exec]     -1 Eclipse classpath. The patch causes the Eclipse classpath to differ from the contents of the lib directories.
     [exec]

I've committed this. Thanks Todd!

Attaching a patch for 0.20.205 version. I just eliminated some hunks.

Reopening because Bharath's adapted patch for 0.20.205.0 has sat here for a week, and no one noticed because the bug is closed and already says fixed in 0.20.205.0 – but it isn't.

Committed to 0.20-security for .206 release.
Will correct the "Fix Versions" field when .206 is added.

Marking it fixed in 205 since 206 isn't there yet.

@Matt, I think this should be marked as fixed in 0.20.205.0. Is that correct?

That's correct. This patch was NOT in the old 205 branch which Owen merged into 204. It was only committed to 0.20-security, which will be in the new 205. Thanks for the catch.

This patch is not required for trunk as it has equivalent code.

Closed upon release of 0.20.205.0