[ 
https://issues.apache.org/jira/browse/HDFS-12325?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16135670#comment-16135670
 ] 

Chen Liang commented on HDFS-12325:
-----------------------------------

Thanks [~elgoiri] for the review, this is good point!

But just like [~arpitagarwal] mentioned, the exception handling of the current 
code has other issue. Specifically, it may leak client connections. For 
example, in {{SFTPFileSystem#create}}:
{code}
    if (parent == null || !mkdirs(client, parent, FsPermission.getDefault())) {
      parent = (parent == null) ? new Path("/") : parent;
      disconnect(client);
      throw new IOException(String.format(E_CREATE_DIR, parent));
    }
{code}
This code is making the syntax implication that before throwing the exception 
to the caller, the client should be disconnected. However if mkdirs does not 
even return but simply throws exception, then there is no catch clause in this 
method and the exception gets thrown to the caller, in which case nowhere in 
the code the client would be disconnected and thus is leaked. So like Arpit 
suggested, we are planning to revisit all of this class's exception handling 
and will file another JIRA to fix all of them. In that JIRA we will reexamine 
how to handle the exception caused by {{cd}} part here as you pointed out.

> SFTPFileSystem operations should restore cwd
> --------------------------------------------
>
>                 Key: HDFS-12325
>                 URL: https://issues.apache.org/jira/browse/HDFS-12325
>             Project: Hadoop HDFS
>          Issue Type: Bug
>            Reporter: Namit Maheshwari
>            Assignee: Chen Liang
>             Fix For: 2.9.0, 3.0.0-beta1
>
>         Attachments: HDFS-12325.001.patch
>
>
> We've seen a case where writing to {{SFTPFileSystem}} led to unexpected 
> behaviour:
> Given a directory ./data with more than one files in it, the steps it took to 
> get this error was simply:
> {code}
> hdfs dfs -fs sftp://x.y.z -mkdir dir0
> hdfs dfs -fs sftp://x.y.z -copyFromLocal data dir0
> hdfs dfs -fs sftp://x.y.z -ls -R dir0
> {code}
> But not all files show up as in the ls output, in fact more often just one 
> single file shows up in that path...
> Digging deeper, we found that rename, mkdirs and create operations in 
> {{SFTPFileSystem}} are changing the current working directory during it's 
> execution. For example in create there are:
> {code}
>       client.cd(parent.toUri().getPath());
>       os = client.put(f.getName());
> {code}
> The issue here is {{SFTPConnectionPool}} is caching SFTP sessions (in 
> {{idleConnections}}), which contains their current working directory. So 
> after these operations, the sessions will be put back to cache with a changed 
> working directory. This accumulates in each call and ends up causing 
> unexpected weird behaviour. Basically this error happens when processing 
> multiple file system objects in one operation, and relative path is being 
> used. 
> The fix here is to restore the current working directory of the SFTP sessions.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to