Federico Mariani created CAMEL-26656:
----------------------------------------
Summary: camel-sftp - existsFile, fastExistsCheck and
readLock=changed list the whole parent directory instead of one stat
Key: CAMEL-26656
URL: https://issues.apache.org/jira/browse/CAMEL-26656
Project: Camel
Issue Type: Improvement
Components: camel-sftp
Affects Versions: 4.18.5, 4.23.0, 4.22.2
Reporter: Federico Mariani
h3. Problem
The JSch-based sftp component finds out whether a remote file exists, and reads
its size and timestamp for {{readLock=changed}}, by listing the parent
directory (main, commit 7978db3d42dd):
- {{SftpOperations.existsFile}} calls {{channel.ls(directory)}} and compares
every name
({{components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/SftpOperations.java}}
line 1409).
- {{fastExistsFile}} (line 1455) calls {{channel.ls(name)}}. In JSch 2.28.8,
{{ls}} of an existing file sends STAT, then OPENDIR on the parent and READDIR
until EOF, keeping only the entry with that name. Only a missing file is cheap
(one STAT). So {{fastExistsCheck}} does not help when the file exists.
- {{readLock=changed}} lists the parent directory on every check, with or
without {{fastExistsCheck}} ({{strategy/SftpExclusiveReadLockCheck.java}} line
55 and lines 97-121). With the default {{readLockMinAge=0}} that is two full
listings per file.
The producer runs this check per message for {{tempFileName}}/{{tempPrefix}}
(twice: target and temp file,
{{components/camel-file/.../GenericFileProducer.java}} lines 159 and 169),
{{fileExist=Ignore|Fail|Move}} ({{SftpOperations.java}} line 1318),
{{fileExist=Ignore}} with {{checksumFileAlgorithm}} (a second check,
{{GenericFileProducer.java}} line 327), a producer {{doneFileName}} (line 305)
and the move-existing strategy. The cost of each such message grows with the
size of the directory.
h3. How to reproduce
Measured with camel-sftp against the embedded MINA SFTP server used by the
camel-ftp tests, counting SFTP requests and returned directory entries with an
{{SftpEventListener}}. Producer: 50 messages into {{/p/out}} holding 2,000
files, counted after a warm-up message. Consumer: 20 files consumed
({{noop=true}}, {{maxMessagesPerPoll=20}}) from 1,000 files, given as the extra
cost per file over the same consumer with {{readLock=none}}.
||Scenario||SFTP requests per message||directory entries read per message||
|producer defaults (control)|15|129|
|{{tempPrefix=.tmp}}|56|4,184|
|{{tempPrefix=.tmp&fastExistsCheck=true}}, targets absent|18|129|
|{{tempPrefix=.tmp&fileExist=TryRename}}|16|129|
|{{fileExist=Ignore}}, targets absent|35|2,157|
|{{fileExist=Ignore&fastExistsCheck=true}}, targets exist|32|2,132|
|{{fileExist=Ignore&checksumFileAlgorithm=md5}}|59|4,235|
|producer {{doneFileName}}|49|2,312|
|consumer {{readLock=changed}}, extra per file|+24|+2,004|
|same with {{fastExistsCheck=true}}|+24|+2,004|
|same with {{readLockMinAge=1}}|+12|+1,002|
The 129 entries of the control come from the {{autoCreate}} directory check
(one READDIR batch). With the plain JSch client in the same 2,000-file
directory, {{ls}} of one existing file cost 1 STAT, 1 OPENDIR, 17 READDIR and 1
CLOSE (2,002 entries); {{lstat}} of the same file cost 1 LSTAT and no entries.
Workarounds today: {{fileExist=TryRename}} with a temp file name skips both
checks (CAMEL-11792); {{readLockMinAge}} halves the {{readLock=changed}}
listings once files are old enough. When the file exists, nothing avoids the
listing, for {{fileExist=Ignore|Fail|Move}} or for {{readLock=changed}}.
h3. Proposed change
- {{existsFile}} and {{fastExistsFile}}: a single {{channel.lstat(path)}},
where SSH_FX_NO_SUCH_FILE means false.
- {{readLock=changed}}: one stat per check instead of a listing; document that
{{fastExistsCheck}} no longer matters for sftp.
- Expected effect: each check is one request whatever the directory size; for
example {{tempPrefix}} drops from 4,184 to 129 entries per message (the rest is
the {{autoCreate}} check, a separate issue).
Behaviour notes, measured with MINA SFTP on a case-insensitive file system:
- Dangling symbolic link: the default listing and {{lstat}} both report it as
existing; today's {{fastExistsCheck=true}} reports it as missing (its STAT
follows the link).
- For {{readLock=changed}} on a symlinked file the listing reports the link's
own attributes, while {{stat}} would report the target's: choose deliberately.
- Case-insensitive servers (Windows- or macOS-backed): {{lstat}} finds
{{LOWER.TXT}} when only {{lower.txt}} exists, while both of today's paths
compare names case-sensitively and say false. This changes
{{fileExist=Fail|Ignore|Move}} and temp file handling on such servers (arguably
more accurate, as the upload replaces that file).
- Directory paths: no difference, all report true.
- Upload-only directories (no read permission): today's listing fails with
SSH_FX_PERMISSION_DENIED, while {{lstat}} answers correctly. Keep a fallback to
the listing on PERMISSION_DENIED or OP_UNSUPPORTED.
- Not verified: a server that answers the generic SSH_FX_FAILURE instead of
NO_SUCH_FILE for a missing path would make {{lstat}} fail where the listing
returned false; fall back to the listing on FAILURE as well.
h3. Impact / upgrade
- No option or default changes. Behaviour changes on case-insensitive servers,
and for dangling links with {{fastExistsCheck=true}}: worth a 4.24 upgrade
guide note.
- Affected: camel-sftp (JSch) producer and consumer. camel-mina-sftp already
uses a single {{stat}} ({{MinaSftpOperations.java}} line 1514). On ftp/ftps,
{{fastExistsCheck}} already avoids the directory listing (NLST or LIST of the
file).
h3. Related
- CAMEL-4356 (fastExistsCheck, opt-in because some FTP servers reject a file
name argument), CAMEL-5512 (readLock=changed reuses fastExistsCheck),
CAMEL-11792 (TryRename), CAMEL-14947 (autoCreate directory check), CAMEL-26635
(consumer listing stops at maxMessagesPerPoll), CAMEL-9324, CAMEL-26648.
- Code checked on camel-4.18.5, camel-4.22.2 and camel-4.23.0 (same calls; JSch
2.28.2, 2.28.6 and 2.28.8 have the same {{ls}} logic).
_Claude Code on behalf of Federico Mariani_
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)