[GitHub] carbondata pull request #2161: Adding Alluxio Supporty

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/2161
 
    @chandrasaripaka thanks for contributing this, I have only last 1 commend. It can be merged after that is resolved, if it is really an issue


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2161#discussion_r244182177
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFile.java ---
    @@ -90,21 +97,82 @@ public CarbonFile getParentFile() {
         return null == parent ? null : new AlluxioCarbonFile(parent);
       }
     
    +  /**
    +   * <p>RenameForce of the fileName for the AlluxioFileSystem Implementation.
    +   * Involves by opening a {@link FSDataInputStream} from the existing path and copy
    +   * bytes to {@link FSDataOutputStream}.
    +   * </p>
    +   * <p>Close the output and input streams only after the files have been written
    +   * Also check for the existence of the changed path and then delete the previous Path.
    +   * The No of Bytes that can be read is controlled by {@literal io.file.buffer.size},
    +   * where the default value is 4096.</p>
    +   * @param changeToName
    +   * @return
    +   */
       @Override
       public boolean renameForce(String changeToName) {
    -    FileSystem fs;
    +    FileSystem fs = null;
    +    FSDataOutputStream fsdos = null;
    +    FSDataInputStream fsdis = null;
         try {
    -      fs = fileStatus.getPath().getFileSystem(FileFactory.getConfiguration());
    -      if (fs instanceof DistributedFileSystem) {
    -        ((DistributedFileSystem) fs).rename(fileStatus.getPath(), new Path(changeToName),
    -            org.apache.hadoop.fs.Options.Rename.OVERWRITE);
    +      Path actualPath = fileStatus.getPath();
    +      Path changedPath = new Path(changeToName);
    +      fs = actualPath.getFileSystem(hadoopConf);
    +      fsdos = fs.create(changedPath, true);
    +      fsdis = fs.open(actualPath);
    +      if (null != fsdis && null != fsdos) {
    +        IOUtils.copyBytes(fsdis, fsdos, hadoopConf, true);
             return true;
    --- End diff --
   
    ThankYou Changed it


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2161#discussion_r244182374
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFile.java ---
    @@ -90,21 +97,82 @@ public CarbonFile getParentFile() {
         return null == parent ? null : new AlluxioCarbonFile(parent);
       }
     
    +  /**
    +   * <p>RenameForce of the fileName for the AlluxioFileSystem Implementation.
    +   * Involves by opening a {@link FSDataInputStream} from the existing path and copy
    +   * bytes to {@link FSDataOutputStream}.
    +   * </p>
    +   * <p>Close the output and input streams only after the files have been written
    +   * Also check for the existence of the changed path and then delete the previous Path.
    +   * The No of Bytes that can be read is controlled by {@literal io.file.buffer.size},
    +   * where the default value is 4096.</p>
    +   * @param changeToName
    +   * @return
    +   */
       @Override
       public boolean renameForce(String changeToName) {
    -    FileSystem fs;
    +    FileSystem fs = null;
    +    FSDataOutputStream fsdos = null;
    +    FSDataInputStream fsdis = null;
         try {
    -      fs = fileStatus.getPath().getFileSystem(FileFactory.getConfiguration());
    -      if (fs instanceof DistributedFileSystem) {
    -        ((DistributedFileSystem) fs).rename(fileStatus.getPath(), new Path(changeToName),
    -            org.apache.hadoop.fs.Options.Rename.OVERWRITE);
    +      Path actualPath = fileStatus.getPath();
    +      Path changedPath = new Path(changeToName);
    +      fs = actualPath.getFileSystem(hadoopConf);
    +      fsdos = fs.create(changedPath, true);
    +      fsdis = fs.open(actualPath);
    +      if (null != fsdis && null != fsdos) {
    +        IOUtils.copyBytes(fsdis, fsdos, hadoopConf, true);
             return true;
    -      } else {
    -        return false;
           }
    +      return false;
         } catch (IOException e) {
           LOGGER.error("Exception occured: " + e.getMessage());
           return false;
    +    } finally {
    +      try {
    +        if (null != fsdis && null != fsdos) {
    +          if (fs.exists(new Path(changeToName))) {
    +            fs.delete(fileStatus.getPath(), true);
    --- End diff --
   
    Reassigned the fileStatus to the changedPath


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
 
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10248/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/2161
 
    LGTM. Merging into master branch


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
 
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10308/



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
 
    @chandrasaripaka Can you verify and fix this issue in branch-1.5,1.4,1.3?


---

[qiuchenjian-2]

Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
 
    @xubo245 yes will do


---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
 
    Thanks @chandrasaripaka


---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
 
    @chandrasaripaka Please add [hidden email] to your GitHub mail list.  Because your commit is Chandra <[hidden email]>.


---