[
https://issues.apache.org/jira/browse/SSHD-1220?focusedWorklogId=672338&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-672338
]
ASF GitHub Bot logged work on SSHD-1220:
----------------------------------------
Author: ASF GitHub Bot
Created on: 30/Oct/21 11:57
Start Date: 30/Oct/21 11:57
Worklog Time Spent: 10m
Work Description: tomaswolf commented on a change in pull request #207:
URL: https://github.com/apache/mina-sshd/pull/207#discussion_r739644691
##########
File path: sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpPath.java
##########
@@ -25,12 +25,80 @@
import java.util.List;
import org.apache.sshd.common.file.util.BasePath;
+import org.apache.sshd.sftp.client.SftpClient;
public class SftpPath extends BasePath<SftpPath, SftpFileSystem> {
+
+ private SftpClient.Attributes attributes;
+
+ private int cachingLevel;
+
public SftpPath(SftpFileSystem fileSystem, String root, List<String>
names) {
super(fileSystem, root, names);
}
+ /**
+ * {@link SftpPath} instances can cache SFTP {@link
SftpClient.Attributes}. Caching can be enabled by passing
+ * {@code true}. If the {@link SftpPath} instance is already caching
attributes, a counter is increased only. To
+ * disable caching, pass {@code false}, which decreases the counter. The
cache is cleared when the counter reaches
+ * zero again.
+ * <p>
+ * Each call of {@code cacheAttributes(true)} must be matched by a call to
{@code cacheAttributes(false)}. Such call
+ * pairs can be nested; caching is enabled for the duration of the
outermost such pair. The outermost call passing
+ * {@code true} clears any possibly already cached attributes so that the
next attempt to read remote attributes
+ * will fetch them anew.
+ *
+ * @param doCache whether to start caching (increasing the cache level) or
to stop caching (decreasing the cache
+ * level)
+ */
+ protected void cacheAttributes(boolean doCache) {
+ if (doCache) {
+ // Start caching. Clear possibly already cached data
+ if (cachingLevel == 0) {
+ attributes = null;
+ }
+ cachingLevel++;
+ } else if (!doCache) {
+ // Stop caching
+ if (cachingLevel > 0) {
+ cachingLevel--;
+ if (cachingLevel == 0) {
+ attributes = null;
+ }
+ }
+ }
+ }
+
+ /**
+ * Sets the cached attributes to the argument if this {@link SftpPath}
instance is currently caching attributes.
+ * Otherwise a no-op.
+ *
+ * @param attributes the {@link SftpClient.Attributes} to cache
+ */
+ protected void cacheAttributes(SftpClient.Attributes attributes) {
+ if (cachingLevel > 0) {
+ setAttributes(attributes);
+ }
+ }
+
+ /**
+ * Unconditionally set the cached attributes, whether or not this
instance's attribute cache is enabled.
+ *
+ * @param attributes the {@link SftpClient.Attributes} to cache
+ */
+ void setAttributes(SftpClient.Attributes attributes) {
Review comment:
Refactored anyway. This is now on `SftpPathImpl`.
--
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: dev-unsubscr...@mina.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 672338)
Time Spent: 1h 20m (was: 1h 10m)
> 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
> Assignee: Thomas Wolf
> Priority: Major
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> 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 also be improved.
--
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
