{"action":"created","issue":{"url":"https://api.github.com/repos/amplab/tachyon/issues/647","labels_url":"https://api.github.com/repos/amplab/tachyon/issues/647/labels{/name}","comments_url":"https://api.github.com/repos/amplab/tachyon/issues/647/comments","events_url":"https://api.github.com/repos/amplab/tachyon/issues/647/events","html_url":"https://github.com/amplab/tachyon/pull/647","id":56712539,"number":647,"title":"[WIP] [TACHYON-221] Modularize out Under File System implementations (Attempt #2)","user":{"login":"rvesse","id":2104864,"avatar_url":"https://avatars.githubusercontent.com/u/2104864?v=3","gravatar_id":"","url":"https://api.github.com/users/rvesse","html_url":"https://github.com/rvesse","followers_url":"https://api.github.com/users/rvesse/followers","following_url":"https://api.github.com/users/rvesse/following{/other_user}","gists_url":"https://api.github.com/users/rvesse/gists{/gist_id}","starred_url":"https://api.github.com/users/rvesse/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rvesse/subscriptions","organizations_url":"https://api.github.com/users/rvesse/orgs","repos_url":"https://api.github.com/users/rvesse/repos","events_url":"https://api.github.com/users/rvesse/events{/privacy}","received_events_url":"https://api.github.com/users/rvesse/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":7,"created_at":"2015-02-05T18:54:38Z","updated_at":"2015-02-06T06:13:48Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/amplab/tachyon/pulls/647","html_url":"https://github.com/amplab/tachyon/pull/647","diff_url":"https://github.com/amplab/tachyon/pull/647.diff","patch_url":"https://github.com/amplab/tachyon/pull/647.patch"},"body":"This is a first pass pull request to gather comments on the proposed changes that modularise out the Under File System implementations as far as I have been meaningfully able to.\r\n\r\nThis has been updated from pull request #642 to rebase against the latest master which has the hierarchy store improvements merged in\r\n\r\n### Overview\r\n\r\nThe current code base has three implementations:\r\n\r\n* Local\r\n* HDFS\r\n* Gluster (which is conflated into HDFS implementation at the moment)\r\n\r\nThis pull request leaves the local implementation in the core module because we need something for basic integration testing though it does move it into a dedicated `tachyon.underfs` package.  It then pulls the other two implementations into their own modules, actually 3 new Maven modules are added:\r\n\r\n* `underfs`\r\n    * `underfs-hdfs`\r\n    * `underfs-glusterfs`\r\n\r\nThe extra parent module is used to logically group the modules within the source tree and provides a useful future home for any generic under file system code that may be abstracted out of core in the future.\r\n\r\n`UnderFileSystem` instances are now created indirectly using a new `UnderFileSystemFactory`, this provides a means to determine if a factory supports a given path and if so to create an instance.  Factories are registered and managed by the `UnderFileSystemRegistry` which primarily auto-discovers available implementations using the `ServiceLoader` mechanism to discover implementations declared in the `META-INF\\services\\tachyon.underfs.UnderFileSystemFactory` file.  The registry can also be used to manually register factories in the case where the automatic discovery proves insufficient.  The existing `UnderFileSystem.get()` methods remain but these now use the `UnderFileSystemRegistry` to determine which implementation to use rather than hard coding that decision which allows much more flexibility e.g. substituting in alternative `UnderFileSystem` implementations for HDFS.\r\n\r\n### Testing\r\n\r\nOne advantage of this modularisation is that a single build can now potentially test all the under file system implementations because each is located in a different module.  The POMs of the new modules use the surefire plugin to pull in the tests from the core module and re-run them with their different implementations.  As a result the `underfs-hdfs` module will now always run HDFS backed tests (unless tests are being skipped) since we are able to start a miniature HDFS cluster in-memory so it is no longer necessary to have a separate profile for these tests.\r\n\r\nHowever in the case of `underfs-gluster` the `glusterfsTest` profile must still be explicitly activated as we don't expect users to have Gluster FS available on their system nor do we have the ability to emulate it as we can with HDFS.\r\n\r\nBottom line is that the build with tests will now fully test both local and HDFS under file system implementations.\r\n\r\n### Maven\r\n\r\nA side effect of the refactoring was that certain plugin configuration files were pulled up under a `build` directory in the top level directory (Checkstyle, FindBugs, License Header) and the plugin configurations appropriately replicated to the new modules.  This bit could potentially be cleaned up to allow the plugins to be defined at a higher level to avoid replicating some configuration though while the number of modules remains small this may not be such an issue.\r\n\r\nOne new addition to the POMs is that license headers are now enforced as far as I thought was meaningful (for example Thrift generated code is excluded) using the `license-maven-plugin` which is useful to ensure consistency in headers and avoid missing headers.  As a result of this there are some minor formatting changes to some headers to ensure that they are detected correctly.\r\n\r\n### TODOs\r\n\r\nThere were a few places where I couldn't simplify the existing code as much as I would have liked because of the need to support pre 2.x Hadoop versions (e.g. the `isHadoopUnderFS()` method could otherwise be greatly simplified/eliminated).  There are also a couple of unit test specific pieces of codes that it would be nice to generalise but it was unclear exactly how this could be done.\r\n\r\n`TODO` comments are added in to indicate these options"},"comment":{"url":"https://api.github.com/repos/amplab/tachyon/issues/comments/73187703","html_url":"https://github.com/amplab/tachyon/pull/647#issuecomment-73187703","issue_url":"https://api.github.com/repos/amplab/tachyon/issues/647","id":73187703,"user":{"login":"haoyuan","id":1200956,"avatar_url":"https://avatars.githubusercontent.com/u/1200956?v=3","gravatar_id":"","url":"https://api.github.com/users/haoyuan","html_url":"https://github.com/haoyuan","followers_url":"https://api.github.com/users/haoyuan/followers","following_url":"https://api.github.com/users/haoyuan/following{/other_user}","gists_url":"https://api.github.com/users/haoyuan/gists{/gist_id}","starred_url":"https://api.github.com/users/haoyuan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/haoyuan/subscriptions","organizations_url":"https://api.github.com/users/haoyuan/orgs","repos_url":"https://api.github.com/users/haoyuan/repos","events_url":"https://api.github.com/users/haoyuan/events{/privacy}","received_events_url":"https://api.github.com/users/haoyuan/received_events","type":"User","site_admin":false},"created_at":"2015-02-06T06:13:48Z","updated_at":"2015-02-06T06:13:48Z","body":"@hsaputra \r\n+1 on a isolated patch."}}