[GitHub] drill pull request #652: DRILL-4990:Use new HDFS API access instead of listS...

[sohami <gi...@git.apache.org>]

Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/652#discussion_r148991415
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceSchemaFactory.java ---
    @@ -151,17 +152,32 @@ public WorkspaceSchemaFactory(
        */
       public boolean accessible(final String userName) throws IOException {
         final FileSystem fs = ImpersonationUtil.createFileSystem(userName, fsConf);
    +    boolean tryListStatus = false;
         try {
    -      // We have to rely on the listStatus as a FileSystem can have complicated controls such as regular unix style
    -      // permissions, Access Control Lists (ACLs) or Access Control Expressions (ACE). Hadoop 2.7 version of FileSystem
    -      // has a limited private API (FileSystem.access) to check the permissions directly
    -      // (see https://issues.apache.org/jira/browse/HDFS-6570). Drill currently relies on Hadoop 2.5.0 version of
    -      // FileClient. TODO: Update this when DRILL-3749 is fixed.
    -      fs.listStatus(wsPath);
    +      // access API checks if a user has certain permissions on a file or directory.
    +      // returns normally if requested permissions are granted and throws an exception
    +      // if access is denied. This API was added in HDFS 2.6 (see HDFS-6570).
    +      // It is less expensive (than listStatus which was being used before) and hides the
    +      // complicated access control logic underneath.
    +      fs.access(wsPath, FsAction.READ);
         } catch (final UnsupportedOperationException e) {
    -      logger.trace("The filesystem for this workspace does not support this operation.", e);
    +      logger.debug("The filesystem for this workspace does not support access operation.", e);
    +      tryListStatus = true;
         } catch (final FileNotFoundException | AccessControlException e) {
    -      return false;
    +      logger.debug("file {} not found or cannot be accessed", wsPath.toString(), e);
    +      tryListStatus = true;
    --- End diff --
    
    Is access function never trustworthy for negative cases ? Or is it windows platform specific issue ?


---