[
https://issues.apache.org/jira/browse/HADOOP-15565?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16906972#comment-16906972
]
Jinglun commented on HADOOP-15565:
----------------------------------
Thanks [~xkrogen] for your nice comments.
{quote}I don't think publicly exposing cacheSize() on FileSystem is a great
idea. Can we make it package-private, and if it is needed in non-package-local
tests, use a test utility to export it publicly?
{quote}
Reasonable! I don't want to start a new file so I'll add the test utility to
TestFileUtil.java.
{quote}Is there a chance the cache will be accessed in a multi-threaded way? If
so we need to harden it for concurrent access. Looks like it will only work in
a single-threaded fashion currently. If the FS instances are actually all
created on startup, then I think we should explicitly populate the cache on
startup.
{quote}
The fs instances are all created on startup, I'll make the cache unmodifiable
so we know it will only be created on startup and won't be modified anymore.
{quote}I agree that swallowing exceptions on child FS close is the right move,
but probably we should at least put them at INFO level?
{quote}
Right! I'll change it.
{quote}This seems less strict than FileSystem.CACHE when checking for equality;
it doesn't use the UserGroupInformation at all. I think this is safe because
the cache is local to a single ViewFileSystem, so all of the inner cached
instances must share the same UGI, but please help me to confirm.
{quote}
Yes, it's safe. As all the instances share the same UGI we can make the Key
simple.
{quote}We can use Objects.hash() for the hashCode() method of Key.
{quote}
Right! That's a good practice! I'll update it.
{quote}On ViewFileSystem L257, you shouldn't initialize fs – you can just
declare it: FileSystem fs; (this allows the compiler to help ensure that you
remember to initialize it later)
{quote}
Right! I'll update it.
Upload patch-006.
> 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
>
>
> 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
(v7.6.14#76016)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
