Thomas Wolf created SSHD-1220:
---------------------------------
Summary: SFTP: too many LSTAT calls
Key: SSHD-1220
URL: https://issues.apache.org/jira/browse/SSHD-1220
Project: MINA SSHD
Issue Type: Improvement
Affects Versions: 2.7.0
Reporter: Thomas Wolf
While looking at an alternate solution for SSHD-1217 I noticed that the
{{AbstractSftpSubsystemHelper}} makes several LSTAT calls for a single
{{FileSystem.readAttributes()}} invocation.
Basically it makes one LSTAT call per supported attributes view; only to
collect the returned items then in one single map anyway.
This doesn't make any sense. SFTP file attributes are a single, defined
structure. If {{AbstractSftpSubsystemHelper.getAttributes()}} needs to collect
all these different views ({{BasicFileAttributes}}, {{PosixFileAttributes}},
and so on) on these underlying SFTP attributes, then all these views should
build upon the same single {{SftpClient.Attributes}} gotten exactly _once_.
This could be solved with careful temporary caching of SFTP attributes on the
{{SftpPath}} once read while in {{AbstractSftpSubsystemHelper.getAttributes()}}
and clearing that cache at the end.
The problem is more general, though. The code in several places makes up-front
checks whether a file exists, is a directory, and so on. This is a questionable
pattern anyway, since the result of a {{Files.exists()}} is outdated
immediately; one cannot rely on the file being still there on the next access.
With an {{SftpPath}}, this {{exists()}} call is an (L)STAT remote call getting
the attributes. Now look at
{{AbstractSftpSubsystemHelper.resolveFileAttributes}}: here getting the
attributes themselves is so guarded, so it makes at least _two_ LSTAT calls.
This should be improved. A solution might be not checking for existence,
isDirectory(), and such up front but translating {{SftpExceptions}} at the
{{FileSystem}} boundary into more standard exceptions if possible and then just
doing the operations. For instance,
{{AbstractSftpSubsystemHelper.resolveFileAttributes}} currently is
{code:java}
protected NavigableMap<String, Object> resolveFileAttributes(
Path file, int flags, LinkOption... options)
throws IOException {
Boolean status = IoUtils.checkFileExists(file, options);
if (status == null) {
return handleUnknownStatusFileAttributes(file, flags, options);
} else if (!status) {
throw new NoSuchFileException(file.toString(), file.toString(),
"Attributes N/A for target");
} else {
return getAttributes(file, flags, options);
}
}
{code}
The {{getAttributes()}} call will call {{java.nio.file.Files.getAttributes()}},
which calls {{SftpFileSystemProvider.readAttributes()}}. That function is at
the {{FileSystem}} API boundary, and should translate an {{SftpException}} with
subcode SH_FX_NO_SUCH_FILE into a {{java.nio.file.NoSuchFileException}}. (And a
few others.)
The {{handleUnknownStatusFileAttributes}} mechanism is invoked if the file
neither {{Files.exists()}} nor {{Files.notExists()}}. IMO that whole case would
just disappear. There is no need for up-front checking in many cases; here
either {{getAttributes()}} succeeds or throws an appropriate exception.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org
