[
https://issues.apache.org/jira/browse/HADOOP-15565?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16923308#comment-16923308
]
Jinglun commented on HADOOP-15565:
----------------------------------
Hi [~xkrogen], thanks very much for your nice review ! I followed all the
suggestions and upload patch- 007.
{quote}Can you explain why the changes in
{{TestViewFileSystemDelegationTokenSupport}} are necessary? Same for
{{TestViewFileSystemDelegation}} – it seems like the old way of returning the
created {{fs}} was cleaner?
{quote}
Good question! I change the unit tests because: Before we add the cache, all
the children filesystems are cached in FileSystem.CACHE. So the filesystem
instance returned by setupMockFileSystem() is exactly the child filesystem of
viewFs. After adding the ViewFileSystem.InnerCache, viewFs's children
filesystem instances are no longer cached in FileSystem.CACHE, so we can't set
fs1 and fs2 to the FileSystem instance returned by setupMockFileSystem().
{quote}I also don't understand the need for changes in {{testSanity()}} – does
the string comparison no longer work?
{quote}
About testSanity(), after changing to
{code:java}
fs1 = (FakeFileSystem) getChildFileSystem((ViewFileSystem) viewFs, new
URI("fs1:/"));{code}
fs1.getUri() will have a path which is set by ViewFileSystem.InnerCache.get(URI
uri, Configuratioin config). So comparing the URI.toString() doesn't work any
more. And I change to compare the scheme and authority.
{quote}Can you describe why the changes in {{TestViewFsDefaultValue}} are
necessary?
{quote}
Good question! I did the changes because: In
TestViewFsDefaultValue.clusterSetupAtBegining(), there are 2 Configuration
instances *CONF* and *conf*. For key _DFS_REPLICATION_KEY_, *CONF* is set to
_DFS_REPLICATION_DEFAULT + 1_ while *conf* is the default value. Before we have
the InnerCache, the child filesystem instance of vfs is got from
FileSystem.CACHE which is constructed with *CONF*. After the InnerCache the
child filesystem instance is got with *conf*. In the test case
testGetDefaultReplication(), the default replication is got from the child
FileSystem instance. When using *CONF* it will be _DFS_REPLICATION_DEFAULT + 1_
and when using *conf* it will be _DFS_REPLICATION_DEFAULT_. Because
testGetDefaultReplication() tests the default replication of the mount point
path, I set _<DFS_REPLICATION_KEY, DFS_REPLICATION_DEFAULT + 1>_ to *conf* to
make it work_._
> ViewFileSystem.close doesn't close child filesystems and causes FileSystem
> objects leak.
> ----------------------------------------------------------------------------------------
>
> Key: HADOOP-15565
> URL: https://issues.apache.org/jira/browse/HADOOP-15565
> Project: Hadoop Common
> Issue Type: Bug
> Reporter: Jinglun
> Assignee: Jinglun
> Priority: Major
> Attachments: HADOOP-15565.0001.patch, HADOOP-15565.0002.patch,
> HADOOP-15565.0003.patch, HADOOP-15565.0004.patch, HADOOP-15565.0005.patch,
> HADOOP-15565.0006.patch, HADOOP-15565.0007.patch
>
>
> ViewFileSystem.close() does nothing but remove itself from FileSystem.CACHE.
> It's children filesystems are cached in FileSystem.CACHE and shared by all
> the ViewFileSystem instances. We could't simply close all the children
> filesystems because it will break the semantic of FileSystem.newInstance().
> We might add an inner cache to ViewFileSystem, let it cache all the children
> filesystems. The children filesystems are not shared any more. When
> ViewFileSystem is closed we close all the children filesystems in the inner
> cache. The ViewFileSystem is still cached by FileSystem.CACHE so there won't
> be too many FileSystem instances.
> The FileSystem.CACHE caches the ViewFileSysem instance and the other
> instances(the children filesystems) are cached in the inner cache.
--
This message was sent by Atlassian Jira
(v8.3.2#803003)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
