PDGGK opened a new pull request, #13112:
URL: https://github.com/apache/seatunnel/pull/13112
### Purpose of this pull request
With the default `connection_mode` (`active_local`),
`SeaTunnelFTPFileSystem` probes the server on every connection by creating
`/.ftptest<currentTimeMillis>` with `MKD`. The `finally` block meant to clean
it up lists that path and calls `deleteFile` only if the listing returned
entries. The probe directory is always empty, so nothing is ever deleted, and
`deleteFile` (`DELE`) could not remove a directory anyway.
As a result, every connection leaves an empty `.ftptest<millis>` directory
in the FTP root whenever the account can write there. The file system opens a
new connection for most calls, so a single job leaves many of them. This also
happens for source jobs that only read.
This PR removes the probe directory with `RMD` when `MKD` created it, and
logs a warning if the server refuses. Mode selection is unchanged: if `MKD`
fails, the client still falls back to passive mode.
### Does this PR introduce _any_ user-facing change?
Yes. With `connection_mode` unset or set to `active_local`, SeaTunnel no
longer leaves `.ftptest<millis>` directories in the FTP root. The probe now
sends one `RMD` instead of a `LIST` of the probe path, so it no longer opens a
data connection. The probe code is the same in every release since 2.3.10. I
reproduced the leak with 3.0.0 and with current `dev`.
The check used vsftpd, with a chrooted local user whose home is writable.
The FtpFile file system was given the options from the source docs: `host`,
`port`, `user`, `password`, `path` and `file_format_type = csv`, with no
`connection_mode`. The run listed `/data` and read its two CSV files, which
took 8 connections.
Before, with the 3.0.0 release jars (current `dev` gives the same counts):
```
FTP root: . .. .ftptest1793635439717 .ftptest1793635439762
.ftptest1793635439799 .ftptest1793635439836 .ftptest1793635439873
.ftptest1793635439910 .ftptest1793635439989 .ftptest1793635440028 data
server log: MKD=8 RMD=0 LIST=12 RETR=2
```
After:
```
FTP root: . .. data
server log: MKD=8 RMD=8 LIST=4 RETR=2
```
### How was this patch tested?
- Added
`SeaTunnelFTPFileSystemTest#testActiveModeProbeDoesNotLeaveDirectoryOnServer`.
It makes two file system calls in the default mode against MockFtpServer,
checks that the client stayed in active mode (so the probe directory was
created), and asserts that no `.ftptest*` entry is left in the server root.
- Without the production change it fails with `Active mode probe left
directories on the server: [.ftptest1793635406289, .ftptest1793635405686,
.ftptest1793635405383, .ftptest1793635406583, .ftptest1793635405969]`.
- With the change it passes.
- `SeaTunnelFTPFileSystemTest` and `FtpFileFactoryTest`: 18 tests, 0
failures.
- `mvn spotless:check -pl
seatunnel-connectors-v2/connector-file/connector-file-ftp` passes.
- Manual run against vsftpd on a Docker network, with the client in a Java
11 container on the same network. This produced the output above.
- With the user's root made read-only, `MKD` is refused, the client falls
back to passive mode as before, both files are read, and no `RMD` is sent.
- I did not run the FTP E2E suite (`FtpFileIT`) locally. That is left to CI.
#13009 also edits `connect()` in the same file. The two changes touch
different methods and merge cleanly.
### Check list
* [ ] If any new Jar binary package adding in your PR, please add License
Notice according
[New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/developer/new-license.md)
* [ ] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs
* [ ] If necessary, please update `incompatible-changes.md` to describe the
incompatibility caused by this PR.
* [ ] If you are contributing the connector code, please check that the
following files are updated:
1. Update
[plugin-mapping.properties](https://github.com/apache/seatunnel/blob/dev/plugin-mapping.properties)
and add new connector information in it
2. Update the pom file of
[seatunnel-dist](https://github.com/apache/seatunnel/blob/dev/seatunnel-dist/pom.xml)
3. Add ci label in
[label-scope-conf](https://github.com/apache/seatunnel/blob/dev/.github/workflows/labeler/label-scope-conf.yml)
4. Add e2e testcase in
[seatunnel-e2e](https://github.com/apache/seatunnel/tree/dev/seatunnel-e2e/seatunnel-connector-v2-e2e/)
5. Update connector
[plugin_config](https://github.com/apache/seatunnel/blob/dev/config/plugin_config)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
