Thanks for the thoughtful response Manoj. I need to think about this some more, but a few ideas for discussion:
IMHO, ViewFsMountPoint should be abstracted and expose only the needed attributes – the MountedOn path and its target FileSystem. The FileSystem could be a hdfs:// or it could be a one for MergeFs, but I don't see a need for exposing all the NameServices, at least for now.
I think the intent was to implement merging in ViewFileSystem itself, rather than a new FileSystem. So we'd need to return an array here, like in the original MountPoint.
Our user API for referring to a FileSystem is also by URI, not by object reference. Yes, the user can always call getUri, but there is global state in a FileSystem like file handles and statistics, and it might be better to not share that by handing out a FileSystem object which they can poke at. Also, since it looks like we allow mounting subdirectories, FileSystem#getUri by itself is underspecified without the path component.
Finally, what is the reason for using generics? getTargetFileSystem will always return a FileSystem right?
Not worth to separate out, though we should think about this some more.
As a semi-side note, I'm quite surprised that ViewFileSystem is annotated @Public. My impression from DistributedFileSystem is that the FileSystem subclasses are private, and are only used when casted as a FileSystem. This is why we have HdfsAdmin, which lets you do DFS-specific operations.
ViewFsUtil is similar to HdfsAdmin, but used to examine an already created ViewFileSystem instance. However, since getStatus takes a ViewFileSystem, it forces the user to downcast which is unfortunate. Instead, we could have an isViewFileSystem API, and having getStatus take a FileSystem and throwing UnsupportedOperation if the passed FS is not a VFS.
Finally, we should probably also name this ViewFileSystemUtil since ViewFs is the FileContext implementation.
I have seen the unix df command getting stuck at times when NFS servers are not reachable. But, I am totally ok to remove this extra feature and error out when any of the backing NameServices are not reachable.
Good point, I've seen similar behavior as well. Let's tackle this in a separate JIRA though, and maybe put the behavior behind a flag. I do think we should return non-zero in this case, and think about how scripts will be able to parse the output.