GH-3370: Remove synchronized from RemoteFileUtils #3380
Conversation
Fixes spring-projects#3370 The `synchronized` on the `RemoteFileUtils.makeDirectories()` makes an application too slow, especially when we deal with different paths in different sessions * Remove the `synchronized` from that method and rework `SftpSession.mkdir()` to return `false` when "A file cannot be created if it already exists" exception is thrown from the server. Essentially make an `exists()` call to be sure that an exception is really related to "file-already-exists" answer from the server **Cherry-pick to 5.3.x, 5.2.x & 4.3.x**
| throw new NestedIOException("failed to create remote directory '" + remoteDirectory + "'.", e); | ||
| catch (SftpException ex) { | ||
| if (ex.id == ChannelSftp.SSH_FX_FAILURE && exists(remoteDirectory)) { | ||
| return false; |
There was a problem hiding this comment.
I think this should return true since it was a race condition. RFU currently discards the result; I think it should check it and throw an exception if false - see FtpSession where we simply delegate to the client.
when error code is not `4` or remote dir does not exist
|
I still think we need to check the result in RFU (for the FTP case). |
Hm. Probably that I think for now I'll just go with your suggestion to make a minimal breaking change and I'll throw some exception from the |
`RemoteFileUtils` to throw an `IOException` when `false`
It's not really a breaking change, we will just detect the problem earlier - some downstream operation would have failed if the mkdir was not successful. |
|
OK. I just mean that the |
I don't think that matters - FTP can throw an exception too. |
|
* GH-3370: Remove synchronized from RemoteFileUtils Fixes #3370 The `synchronized` on the `RemoteFileUtils.makeDirectories()` makes an application too slow, especially when we deal with different paths in different sessions * Remove the `synchronized` from that method and rework `SftpSession.mkdir()` to return `false` when "A file cannot be created if it already exists" exception is thrown from the server. Essentially make an `exists()` call to be sure that an exception is really related to "file-already-exists" answer from the server **Cherry-pick to 5.3.x, 5.2.x & 4.3.x** * * Re-throw an exception in the `SftpSession.mkdir()` when error code is not `4` or remote dir does not exist * * Check `session.mkdir()` result in the `RemoteFileUtils` to throw an `IOException` when `false` * * Fix mock test to return `true` for `mkdir` instead of `null`
* GH-3370: Remove synchronized from RemoteFileUtils Fixes #3370 The `synchronized` on the `RemoteFileUtils.makeDirectories()` makes an application too slow, especially when we deal with different paths in different sessions * Remove the `synchronized` from that method and rework `SftpSession.mkdir()` to return `false` when "A file cannot be created if it already exists" exception is thrown from the server. Essentially make an `exists()` call to be sure that an exception is really related to "file-already-exists" answer from the server **Cherry-pick to 5.3.x, 5.2.x & 4.3.x** * * Re-throw an exception in the `SftpSession.mkdir()` when error code is not `4` or remote dir does not exist * * Check `session.mkdir()` result in the `RemoteFileUtils` to throw an `IOException` when `false` * * Fix mock test to return `true` for `mkdir` instead of `null`
|
Cherry-picked to 5.3.x, 5.2.x - 4.3.x needs a back-port. |
* GH-3370: Remove synchronized from RemoteFileUtils Fixes #3370 The `synchronized` on the `RemoteFileUtils.makeDirectories()` makes an application too slow, especially when we deal with different paths in different sessions * Remove the `synchronized` from that method and rework `SftpSession.mkdir()` to return `false` when "A file cannot be created if it already exists" exception is thrown from the server. Essentially make an `exists()` call to be sure that an exception is really related to "file-already-exists" answer from the server **Cherry-pick to 5.3.x, 5.2.x & 4.3.x** * * Re-throw an exception in the `SftpSession.mkdir()` when error code is not `4` or remote dir does not exist * * Check `session.mkdir()` result in the `RemoteFileUtils` to throw an `IOException` when `false` * * Fix mock test to return `true` for `mkdir` instead of `null` # Conflicts: # spring-integration-file/src/main/java/org/springframework/integration/file/remote/RemoteFileUtils.java # spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java # spring-integration-file/src/test/java/org/springframework/integration/file/remote/gateway/RemoteFileOutboundGatewayTests.java # spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpOutboundTests.java
|
Back-ported to |
Fixes #3370
The
synchronizedon theRemoteFileUtils.makeDirectories()makes an application tooslow, especially when we deal with different paths in different sessions
synchronizedfrom that method and reworkSftpSession.mkdir()to return
falsewhen "A file cannot be created if it already exists" exceptionis thrown from the server.
Essentially make an
exists()call to be sure that an exception is really relatedto "file-already-exists" answer from the server
Cherry-pick to 5.3.x, 5.2.x & 4.3.x