Don't make RemoteFileUtils.makeDirectories syncronized #3370
Comments
dongja3
added
status: waiting-for-triage
dongja3
changed the title
|
It was synchronized to fix a race condition. |
|
I think the request is valid. Even with that It is better to rework this logic to really catch an exception from the WDYT? |
|
Agreed; yes. |
|
Hi, Aterm, Gary |
artembilan
added
backport 5.2.x
in: ftp
in: sftp
and removed
status: waiting-for-triage
artembilan
added a commit
to artembilan/spring-integration
that referenced
this issue
Sep 10, 2020
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**
garyrussell
pushed a commit
that referenced
this issue
Sep 10, 2020
* 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`
garyrussell
pushed a commit
that referenced
this issue
Sep 10, 2020
* 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`
garyrussell
pushed a commit
that referenced
this issue
Sep 10, 2020
* 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`
artembilan
added a commit
that referenced
this issue
Sep 10, 2020
* 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In what version(s) of Spring Integration are you seeing this issue?
5.3.1.RELEASE
Describe the bug
It caused by org.springframework.integration.file.remote.RemoteFileUtils#makeDirectories, which is synchronized static method, when there are lots of (S)ftp move operation concurrently and slow speed of network, all requests of AbstractRemoteFileOutboundGateway#mv are queued and observed as very slow speed.
https://stackoverflow.com/questions/62772002/outbound-gateway-executes-the-mv-command-very-slowly
Expected behavior
in api doc, RemoteFileUtils#makeDirectories is static not synchronized,
https://docs.spring.io/spring-integration/docs/5.3.1.RELEASE/api/
However, in source code is static and syncronized.
The text was updated successfully, but these errors were encountered: