[GitHub] carbondata pull request #2868: [WIP] Improve drop table performance by reduc...

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2868#discussion_r229363490
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/CarbonFile.java ---
    @@ -62,6 +62,11 @@
     
       boolean renameForce(String changetoName);
     
    +  /**
    +   * This method will delete the files recursively from file system
    +   *
    +   * @return
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2868#discussion_r229363551
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/LocalCarbonFile.java ---
    @@ -141,7 +141,12 @@ public boolean renameTo(String changetoName) {
       }
     
       public boolean delete() {
    -    return file.delete();
    +    try {
    +      return deleteFile(file.getAbsolutePath(), FileFactory.getFileType(file.getAbsolutePath()));
    +    } catch (IOException e) {
    +      LOGGER.error("Exception occurred:" + e.getMessage());
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2868
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1167/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2868
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1380/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2868#discussion_r229577855
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ---
    @@ -300,40 +300,27 @@ public static void deleteFoldersAndFiles(final File... path)
     
           @Override public Void run() throws Exception {
             for (int i = 0; i < path.length; i++) {
    -          deleteRecursive(path[i]);
    +          CarbonFile carbonFile = FileFactory.getCarbonFile(path[i].getAbsolutePath());
    +          boolean delete = carbonFile.delete();
    +          if (!delete) {
    +            throw new IOException("Error while deleting the folders and files");
    --- End diff --
   
    better to print the file location


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2868#discussion_r229577891
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ---
    @@ -300,40 +300,27 @@ public static void deleteFoldersAndFiles(final File... path)
     
           @Override public Void run() throws Exception {
             for (int i = 0; i < path.length; i++) {
    -          deleteRecursive(path[i]);
    +          CarbonFile carbonFile = FileFactory.getCarbonFile(path[i].getAbsolutePath());
    +          boolean delete = carbonFile.delete();
    +          if (!delete) {
    +            throw new IOException("Error while deleting the folders and files");
    +          }
             }
             return null;
           }
         });
       }
     
    -  /**
    -   * Recursively delete the files
    -   *
    -   * @param f File to be deleted
    -   * @throws IOException
    -   */
    -  private static void deleteRecursive(File f) throws IOException {
    -    if (f.isDirectory()) {
    -      File[] files = f.listFiles();
    -      if (null != files) {
    -        for (File c : files) {
    -          deleteRecursive(c);
    -        }
    -      }
    -    }
    -    if (f.exists() && !f.delete()) {
    -      throw new IOException("Error while deleting the folders and files");
    -    }
    -  }
    -
       public static void deleteFoldersAndFiles(final CarbonFile... file)
           throws IOException, InterruptedException {
         UserGroupInformation.getLoginUser().doAs(new PrivilegedExceptionAction<Void>() {
     
           @Override public Void run() throws Exception {
             for (int i = 0; i < file.length; i++) {
    -          deleteRecursive(file[i]);
    +          boolean delete = file[i].delete();
    +          if (!delete) {
    +            throw new IOException("Error while deleting the folders and files");
    --- End diff --
   
    better to print the file location


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2868#discussion_r229578798
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ---
    @@ -300,40 +300,27 @@ public static void deleteFoldersAndFiles(final File... path)
     
           @Override public Void run() throws Exception {
             for (int i = 0; i < path.length; i++) {
    -          deleteRecursive(path[i]);
    +          CarbonFile carbonFile = FileFactory.getCarbonFile(path[i].getAbsolutePath());
    +          boolean delete = carbonFile.delete();
    +          if (!delete) {
    +            throw new IOException("Error while deleting the folders and files");
    +          }
             }
             return null;
           }
         });
       }
     
    -  /**
    -   * Recursively delete the files
    -   *
    -   * @param f File to be deleted
    -   * @throws IOException
    -   */
    -  private static void deleteRecursive(File f) throws IOException {
    -    if (f.isDirectory()) {
    -      File[] files = f.listFiles();
    -      if (null != files) {
    -        for (File c : files) {
    -          deleteRecursive(c);
    -        }
    -      }
    -    }
    -    if (f.exists() && !f.delete()) {
    -      throw new IOException("Error while deleting the folders and files");
    -    }
    -  }
    -
       public static void deleteFoldersAndFiles(final CarbonFile... file)
           throws IOException, InterruptedException {
         UserGroupInformation.getLoginUser().doAs(new PrivilegedExceptionAction<Void>() {
     
           @Override public Void run() throws Exception {
             for (int i = 0; i < file.length; i++) {
    -          deleteRecursive(file[i]);
    +          boolean delete = file[i].delete();
    +          if (!delete) {
    +            throw new IOException("Error while deleting the folders and files");
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2868#discussion_r229578811
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ---
    @@ -300,40 +300,27 @@ public static void deleteFoldersAndFiles(final File... path)
     
           @Override public Void run() throws Exception {
             for (int i = 0; i < path.length; i++) {
    -          deleteRecursive(path[i]);
    +          CarbonFile carbonFile = FileFactory.getCarbonFile(path[i].getAbsolutePath());
    +          boolean delete = carbonFile.delete();
    +          if (!delete) {
    +            throw new IOException("Error while deleting the folders and files");
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2868
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1176/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2868
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1389/



---

[qiuchenjian-2]

Github user manishgupta88 commented on the issue:

    https://github.com/apache/carbondata/pull/2868
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2868
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1179/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2868
 
    Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9443/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2868
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1392/



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/2868
 
    LGTM


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

    https://github.com/apache/carbondata/pull/2868


---