You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/10/09 15:26:59 UTC
[GitHub] [commons-net] payal-meh opened a new pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
payal-meh opened a new pull request #65:
URL: https://github.com/apache/commons-net/pull/65
…from z/OS and z/VM
Closing inputstream prior to reading transfer reply seems fixes the issue.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] payal-meh commented on a change in pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
payal-meh commented on a change in pull request #65:
URL: https://github.com/apache/commons-net/pull/65#discussion_r502533767
##########
File path: src/main/java/org/apache/commons/net/ftp/FTPClient.java
##########
@@ -1917,6 +1917,7 @@ protected boolean _retrieveFile(final String command, final String remote, final
Util.copyStream(input, local, getBufferSize(),
CopyStreamEvent.UNKNOWN_STREAM_SIZE, mergeListeners(csl),
false);
+ input.close();
Review comment:
It looks like the inputstream needs to be closed before trying to get the transfer response.
When I stepped through the debugger would hang on FTP.__getReply line 320
` String line = _controlInput_.readLine();`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] garydgregory closed pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
garydgregory closed pull request #65:
URL: https://github.com/apache/commons-net/pull/65
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] garydgregory commented on a change in pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #65:
URL: https://github.com/apache/commons-net/pull/65#discussion_r502608413
##########
File path: src/main/java/org/apache/commons/net/ftp/FTPClient.java
##########
@@ -1917,6 +1917,7 @@ protected boolean _retrieveFile(final String command, final String remote, final
Util.copyStream(input, local, getBufferSize(),
CopyStreamEvent.UNKNOWN_STREAM_SIZE, mergeListeners(csl),
false);
+ input.close();
Review comment:
Please rebase on master for your experiment, I fixed a possible (if unlikely) socket and input stream leak on socket exception in `org.apache.commons.net.ftp.FTPClient._retrieveFile(String, String, OutputStream)`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] sebbASF commented on pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
sebbASF commented on pull request #65:
URL: https://github.com/apache/commons-net/pull/65#issuecomment-707752309
Sorry, no idea.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] payal-meh commented on a change in pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
payal-meh commented on a change in pull request #65:
URL: https://github.com/apache/commons-net/pull/65#discussion_r503198200
##########
File path: src/main/java/org/apache/commons/net/ftp/FTPClient.java
##########
@@ -1917,6 +1917,7 @@ protected boolean _retrieveFile(final String command, final String remote, final
Util.copyStream(input, local, getBufferSize(),
CopyStreamEvent.UNKNOWN_STREAM_SIZE, mergeListeners(csl),
false);
+ input.close();
Review comment:
I'm still seeing the same behaviour as before. When I retrieve the file against the z/OS FTP server it hangs for 2 minutes and then completes. Against the z/VM FTP server it hangs until the control channel times out after 5 minutes and I get the following stack trace:
` java.net.SocketException: Connection reset
at java.net.SocketInputStream.read(SocketInputStream.java:210)
at java.net.SocketInputStream.read(SocketInputStream.java:141)
at sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:284)
at sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:326)
at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:178)
at java.io.InputStreamReader.read(InputStreamReader.java:184)
at java.io.BufferedReader.fill(BufferedReader.java:161)
at java.io.BufferedReader.read(BufferedReader.java:182)
at org.apache.commons.net.io.CRLFLineReader.readLine(CRLFLineReader.java:58)
at org.apache.commons.net.ftp.FTP.__getReply(FTP.java:320)
at org.apache.commons.net.ftp.FTP.__getReply(FTP.java:299)
at org.apache.commons.net.ftp.FTP.getReply(FTP.java:728)
at org.apache.commons.net.ftp.FTPClient.completePendingCommand(FTPClient.java:1850)
at org.apache.commons.net.ftp.FTPClient._retrieveFile(FTPClient.java:1920)
at org.apache.commons.net.ftp.FTPClient.retrieveFile(FTPClient.java:1882)
at com.ibm.ftp.testing.FTPTimeout.getFile(FTPTimeout.java:123)
at com.ibm.ftp.testing.FTPTimeout.getFileFTP(FTPTimeout.java:69)
at com.ibm.ftp.testing.FTPTimeout.main(FTPTimeout.java:41)`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] garydgregory commented on a change in pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #65:
URL: https://github.com/apache/commons-net/pull/65#discussion_r502526562
##########
File path: src/main/java/org/apache/commons/net/ftp/FTPClient.java
##########
@@ -1917,6 +1917,7 @@ protected boolean _retrieveFile(final String command, final String remote, final
Util.copyStream(input, local, getBufferSize(),
CopyStreamEvent.UNKNOWN_STREAM_SIZE, mergeListeners(csl),
false);
+ input.close();
Review comment:
The finally block calls `Util.closeQuietly(input);` so why do we need this?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] payal-meh commented on pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
payal-meh commented on pull request #65:
URL: https://github.com/apache/commons-net/pull/65#issuecomment-707869433
I have tested with that snapshot and it looks good. Retrieves from z/OS and z/VM using FTP now work as expected. Retrieves using FTPS still work as they did before as well. Thank you for implementing that change.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] garydgregory commented on pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #65:
URL: https://github.com/apache/commons-net/pull/65#issuecomment-707730159
@sebbASF
Any ideas?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] garydgregory commented on pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #65:
URL: https://github.com/apache/commons-net/pull/65#issuecomment-707917616
YW. I'll try to cut a release candidate relatively soonish, maybe this week...
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] garydgregory commented on a change in pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #65:
URL: https://github.com/apache/commons-net/pull/65#discussion_r502608413
##########
File path: src/main/java/org/apache/commons/net/ftp/FTPClient.java
##########
@@ -1917,6 +1917,7 @@ protected boolean _retrieveFile(final String command, final String remote, final
Util.copyStream(input, local, getBufferSize(),
CopyStreamEvent.UNKNOWN_STREAM_SIZE, mergeListeners(csl),
false);
+ input.close();
Review comment:
Please rebase on master for your experiment, I fix possible (if unlikely) socket and input stream leak on socket exception in `org.apache.commons.net.ftp.FTPClient._retrieveFile(String, String, OutputStream)`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] garydgregory commented on a change in pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #65:
URL: https://github.com/apache/commons-net/pull/65#discussion_r503606611
##########
File path: src/main/java/org/apache/commons/net/ftp/FTPClient.java
##########
@@ -1917,6 +1917,7 @@ protected boolean _retrieveFile(final String command, final String remote, final
Util.copyStream(input, local, getBufferSize(),
CopyStreamEvent.UNKNOWN_STREAM_SIZE, mergeListeners(csl),
false);
+ input.close();
Review comment:
Does this happen for all downloads or just some of the time?
What version of z/OS and so on?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] payal-meh commented on a change in pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
payal-meh commented on a change in pull request #65:
URL: https://github.com/apache/commons-net/pull/65#discussion_r503829626
##########
File path: src/main/java/org/apache/commons/net/ftp/FTPClient.java
##########
@@ -1917,6 +1917,7 @@ protected boolean _retrieveFile(final String command, final String remote, final
Util.copyStream(input, local, getBufferSize(),
CopyStreamEvent.UNKNOWN_STREAM_SIZE, mergeListeners(csl),
false);
+ input.close();
Review comment:
Happens on all retrieves against z/OS or z/VM (both when requesting multiple files or just one). I get the same behaviour using binary file type + stream mode transfer as well as EBCDIC file type + block mode transfer. I do not have access to any distributed hosts which have FTP enabled so cannot test against those, but when I tried against a public FTP server (ftp.mirrorservice.org) I did NOT have the problem. I presume because that FTP server closes the data socket once the transfer is complete (replying with 226) whereas the z/OS and z/VM FTP servers do not (replying with 250).
The z/OS systems I have tried against are z/OS 2.3 and z/OS 2.4. The z/VM host is at 6.4.0.
This not an issue when using the FTPS Client or if switching back to commons-net 3.6.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [commons-net] garydgregory commented on pull request #65: NET-690 Performance issue when using the FTPClient to retrieve files …
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #65:
URL: https://github.com/apache/commons-net/pull/65#issuecomment-707839303
@payal-meh
I applied a change to the the resource management in this method.
Please try the latest SNAPSHOT build by adding this Maven repository to your POM: https://repository.apache.org/content/repositories/snapshots/
----------------------------------------------------------------
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:
users@infra.apache.org