You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by chandrasaripaka <gi...@git.apache.org> on 2018/04/11 17:51:22 UTC

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

GitHub user chandrasaripaka opened a pull request:

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

    Adding Alluxio Supporty

    Be sure to do all of the following checklist to help us incorporate 
    your contribution quickly and easily:
    
     - [No ] Any interfaces changed?
     
     - [No ] Any backward compatibility impacted?
     
     - [Yes ] Document update required?
    
     - [Partially ] Testing done
            Please provide details on 
            - Whether new unit test cases have been added or why no new tests are required?
                 It is the support for alluxio that has been fixed.
            - How it is tested? Please attach test report.
                It is tested with external alluxio.
            - Is it a performance related change? Please attach the performance test report.
            - Any additional information to help reviewers in testing this change.
                Added Alluxio Support , as it is not reflecting the correct FileSystem.
           
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA. 
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/DataHeaps/carbondata alluxio-support

Alternatively you can review and apply these changes as the patch at:

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

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2161
    
----
commit 35ce7550954fbc8d9211352b79876c1ee148b609
Author: pour <ch...@...>
Date:   2018-03-22T08:00:38Z

    Adding Alluxio Supporty

----


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r243773875
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -960,7 +960,7 @@ private CarbonCommonConstants() {
        * If set to GLOBAL_SORT, the sorting scope is bigger and one index tree per task will be
        * created, thus loading is slower but query is faster.
        */
    -  public static final String LOAD_SORT_SCOPE_DEFAULT = "LOCAL_SORT";
    +  public static final String LOAD_SORT_SCOPE_DEFAULT = "NO_SORT";
    --- End diff --
    
    resolved


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @xubo2545 looks like there is a permission issue for locating the file to build for my I'd. Actually the build is passing.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @chandrasaripaka Please add venkatarcs@dbs.com to your GitHub mail list.  Because your commit is Chandra <ve...@dbs.com>.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241983388
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/AlluxioFileLock.java ---
    @@ -0,0 +1,112 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.core.locks;
    +
    +import java.io.DataOutputStream;
    +import java.io.IOException;
    +
    +import org.apache.carbondata.common.logging.LogServiceFactory;
    +import org.apache.carbondata.core.datastore.filesystem.AlluxioCarbonFile;
    +import org.apache.carbondata.core.datastore.impl.FileFactory;
    +import org.apache.carbondata.core.metadata.AbsoluteTableIdentifier;
    +import org.apache.carbondata.core.util.path.CarbonTablePath;
    +
    +import org.apache.log4j.Logger;
    +
    +/**
    + * This class is used to handle the S3 File locking.
    + * This is acheived using the concept of acquiring the data out stream using Append option.
    + */
    +public class AlluxioFileLock extends AbstractCarbonLock {
    --- End diff --
    
    Understand , will just extend from that.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7116/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/10157/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3837/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9881/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    I guess the build passed, but at the time of locating the jar , it failed. Can somebody look into the issue with Spark2.1.0 CI build. 


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241981844
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/AlluxioFileLock.java ---
    @@ -0,0 +1,112 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.core.locks;
    +
    +import java.io.DataOutputStream;
    +import java.io.IOException;
    +
    +import org.apache.carbondata.common.logging.LogServiceFactory;
    +import org.apache.carbondata.core.datastore.filesystem.AlluxioCarbonFile;
    +import org.apache.carbondata.core.datastore.impl.FileFactory;
    +import org.apache.carbondata.core.metadata.AbsoluteTableIdentifier;
    +import org.apache.carbondata.core.util.path.CarbonTablePath;
    +
    +import org.apache.log4j.Logger;
    +
    +/**
    + * This class is used to handle the S3 File locking.
    + * This is acheived using the concept of acquiring the data out stream using Append option.
    + */
    +public class AlluxioFileLock extends AbstractCarbonLock {
    --- End diff --
    
    @chandrasaripaka What I meant was we can use the same HDFSFileLock class or extended class of HDFSFileLock as I don't see much difference in code of `AlluxioFileLock` and `HDFSFileLock`


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r185777526
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFile.java ---
    @@ -105,18 +103,21 @@ public CarbonFile getParentFile() {
         return null == parent ? null : new AlluxioCarbonFile(parent);
       }
     
    +  /**
    +   * CARBON-2218 Adopting to {@link FileSystem} , in accordance
    +   * with the AlluxioFileSystem Implementation.
    +   * Implicit expection of the AlluxioFileSystem when using this method.
    +   * If success, returns true else false when an Exception is raised.
    +   * @param changetoName
    +   * @return
    +   */
       @Override
       public boolean renameForce(String changetoName) {
         FileSystem fs;
         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);
    -        return true;
    -      } else {
    -        return false;
    -      }
    +      fs.delete(new Path(changetoName), true);
    --- End diff --
    
    It's a dangerous operation as we cannot get back the file once it is removed. So failure cases may lead to system inconsistent.  Since Alluxio does not support rename we should try to bring the framework where we should not use renameForce. We will work on it. 


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @jackylk @ravipesala Please review it again.


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241977147
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFile.java ---
    @@ -94,14 +93,9 @@ public CarbonFile getParentFile() {
       public boolean renameForce(String changeToName) {
         FileSystem fs;
         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);
    -        return true;
    -      } else {
    -        return false;
    -      }
    +      fs = fileStatus.getPath().getFileSystem(hadoopConf);
    +      fs.delete(new Path(changeToName), true);
    +      return fs.rename(fileStatus.getPath(), new Path(changeToName));
    --- End diff --
    
    It's a risky operation if 98 is failed then no way to get the file which is deleted in 97 line.  I think we should find a way to do it in single transaction.  Any better way @jackylk ?


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    refresh your local code:
        git fetch --all
    rebase to latest master branch code:
         git rebase -i origin/master
    fix the conflict and push again


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/1987/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/1710/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r237726325
  
    --- Diff: core/src/test/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFileTest.java ---
    @@ -108,12 +121,12 @@ public void testListFilesForNullListStatus() {
             alluxioCarbonFile = new AlluxioCarbonFile(fileStatusWithOutDirectoryPermission);
             new MockUp<Path>() {
                 @Mock
    -            public FileSystem getFileSystem(Configuration conf) throws IOException {
    -                return new DistributedFileSystem();
    +            public FileSystem get(FileSystemContext context) throws IOException {
    --- End diff --
    
    Please fix the test error


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r185778044
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/impl/FileFactory.java ---
    @@ -365,10 +366,11 @@ public static boolean createNewLockFile(String filePath, FileType fileType) thro
       public static String getUpdatedFilePath(String filePath, FileType fileType) {
         switch (fileType) {
           case HDFS:
    -      case ALLUXIO:
           case VIEWFS:
           case S3:
             return filePath;
    +      case ALLUXIO:
    +        return StringUtils.containsAny(filePath,"alluxio")?filePath: "alluxio://"+filePath;
    --- End diff --
    
    Why is this special handling for alluxio?


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/1921/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/2113/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @xubo245 how do we merge this


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @chandrasaripaka Can you fix the CI error?


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4545/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241980998
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/AlluxioFileLock.java ---
    @@ -0,0 +1,112 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.core.locks;
    +
    +import java.io.DataOutputStream;
    +import java.io.IOException;
    +
    +import org.apache.carbondata.common.logging.LogServiceFactory;
    +import org.apache.carbondata.core.datastore.filesystem.AlluxioCarbonFile;
    +import org.apache.carbondata.core.datastore.impl.FileFactory;
    +import org.apache.carbondata.core.metadata.AbsoluteTableIdentifier;
    +import org.apache.carbondata.core.util.path.CarbonTablePath;
    +
    +import org.apache.log4j.Logger;
    +
    +/**
    + * This class is used to handle the S3 File locking.
    + * This is acheived using the concept of acquiring the data out stream using Append option.
    + */
    +public class AlluxioFileLock extends AbstractCarbonLock {
    --- End diff --
    
    We cannot use HDFSFileLock..the reason is we cannot go back to HDFS, for file locking. I understand although the structure is copied from HDFS File..when we run our jobs on alluxio, we may not be using HDFS at all, because the default file system (fs.defaultFS) falls back to Alluxio..


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241977202
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/impl/FileFactory.java ---
    @@ -43,7 +44,7 @@
        * LOGGER
        */
       private static final Logger LOGGER =
    -      LogServiceFactory.getLogService(FileFactory.class.getName());
    +          LogServiceFactory.getLogService(FileFactory.class.getName());
    --- End diff --
    
    There are lots of unnecessary format changes, please remove those changes and keep only the changes required for this PR


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/1591/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r244004960
  
    --- 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 --
    
    I think what you do in the finally block should move to here and fileStatus should be assigned to point to the newly created file. 



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9849/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5058/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r244005006
  
    --- 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 --
    
    Here the fileStatus is not updated, I suspect there will be potential issue


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/1774/



---

[GitHub] carbondata issue #2161: Adding Alluxio Supporty

Posted by chenliang613 <gi...@git.apache.org>.
Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @chandrasaripaka   Can you provide more detail about this pull request.


---

[GitHub] carbondata issue #2161: Adding Alluxio Supporty

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Can one of the admins verify this patch?


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241625448
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/impl/FileFactory.java ---
    @@ -111,14 +112,9 @@ public static DataInputStream getDataInputStream(String path, FileType fileType)
         return getDataInputStream(path, fileType, -1);
       }
     
    -  public static DataInputStream getDataInputStream(String path, FileType fileType,
    --- End diff --
    
    ok, please rebase asap


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241264313
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/impl/FileFactory.java ---
    @@ -432,7 +437,7 @@ public static long getDirectorySize(String filePath) throws IOException {
           case VIEWFS:
           case S3:
             Path path = new Path(filePath);
    -        FileSystem fs = path.getFileSystem(getConfiguration());
    +        FileSystem fs = path.getFileSystem(configuration);
    --- End diff --
    
    Why do you need change this?


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/ApacheCarbonPRBuilder1/5229/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by kevinjmh <gi...@git.apache.org>.
Github user kevinjmh commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r243254949
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/AlluxioFileLock.java ---
    @@ -0,0 +1,112 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.core.locks;
    +
    +import java.io.DataOutputStream;
    +import java.io.IOException;
    +
    +import org.apache.carbondata.common.logging.LogServiceFactory;
    +import org.apache.carbondata.core.datastore.filesystem.AlluxioCarbonFile;
    +import org.apache.carbondata.core.datastore.impl.FileFactory;
    +import org.apache.carbondata.core.metadata.AbsoluteTableIdentifier;
    +import org.apache.carbondata.core.util.path.CarbonTablePath;
    +
    +import org.apache.log4j.Logger;
    +
    +/**
    + * This class is used to handle the S3 File locking.
    + * This is acheived using the concept of acquiring the data out stream using Append option.
    + */
    +public class AlluxioFileLock extends AbstractCarbonLock {
    +
    +  /**
    +   * LOGGER
    +   */
    +  private static final Logger LOGGER =
    +          LogServiceFactory.getLogService(AlluxioCarbonFile.class.getName());
    +  /**
    +   * lockFilePath is the location of the lock file.
    +   */
    +  private String lockFilePath;
    +
    +  /**
    +   * lockFileDir is the directory of the lock file.
    +   */
    +  private String lockFileDir;
    +
    +  private DataOutputStream dataOutputStream;
    +
    +  /**
    +   * @param tableIdentifier
    +   * @param lockFile
    +   */
    +  public AlluxioFileLock(AbsoluteTableIdentifier tableIdentifier, String lockFile) {
    +    this(tableIdentifier.getTablePath(), lockFile);
    +  }
    +
    +  /**
    +   * @param lockFileLocation
    +   * @param lockFile
    +   */
    +  public AlluxioFileLock(String lockFileLocation, String lockFile) {
    +    this.lockFileDir = CarbonTablePath.getLockFilesDirPath(lockFileLocation);
    +    this.lockFilePath = CarbonTablePath.getLockFilePath(lockFileLocation, lockFile);
    +    LOGGER.info("Alluxio lock path:" + this.lockFilePath);
    +    initRetry();
    +  }
    +
    +  /* (non-Javadoc)
    +   * @see org.apache.carbondata.core.locks.ICarbonLock#unlock()
    +   */
    +  @Override
    +  public boolean unlock() {
    +    boolean status = false;
    +    if (null != dataOutputStream) {
    +      try {
    +        dataOutputStream.close();
    +        status = true;
    +      } catch (IOException e) {
    +        status = false;
    +      }
    +    }
    +    return status;
    +  }
    +
    +  /* (non-Javadoc)
    +   * @see org.apache.carbondata.core.locks.ICarbonLock#lock()
    +   */
    +  @Override
    +  public boolean lock() {
    +    try {
    +      if (!FileFactory.isFileExist(lockFileDir)) {
    +        FileFactory.mkdirs(lockFileDir, FileFactory.getFileType(lockFileDir));
    +      }
    +      if (!FileFactory.isFileExist(lockFilePath)) {
    +        FileFactory.createNewLockFile(lockFilePath, FileFactory.getFileType(lockFilePath));
    +      }
    +      dataOutputStream =
    +              FileFactory.getDataOutputStreamUsingAppend(lockFilePath,
    +                      FileFactory.getFileType(lockFilePath));
    --- End diff --
    
    @xubo245 "tablestatus.lock already exists" is caused by this


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r200066306
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/impl/FileFactory.java ---
    @@ -365,10 +366,11 @@ public static boolean createNewLockFile(String filePath, FileType fileType) thro
       public static String getUpdatedFilePath(String filePath, FileType fileType) {
         switch (fileType) {
           case HDFS:
    -      case ALLUXIO:
           case VIEWFS:
           case S3:
             return filePath;
    +      case ALLUXIO:
    +        return StringUtils.containsAny(filePath,"alluxio")?filePath: "alluxio://"+filePath;
    --- End diff --
    
    Alluxio does the path translation in the FileSystem , that s why we have this issue. https://github.com/Alluxio/alluxio/blob/master/core/client/hdfs/src/main/java/alluxio/hadoop/AbstractFileSystem.java


---

[GitHub] carbondata issue #2161: Adding Alluxio Supporty

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Please add jira number in title


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    > @chandrasaripaka you can put the CarbonFile test with Alluxio Mini Cluster but make sure it does not go beyond a few seconds to finish the test as it impacts the build time
    
    @ravipesala , MiniClusters are multiprocess tests, they go beyond few seconds. Will check if I can make it in the timeline,


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/10170/



---

[GitHub] carbondata issue #2161: Adding Alluxio Supporty

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4953/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
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


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/1903/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @ravipesala , I want to put a CarbonFile Test based on Alluxio Mini Cluster, will that be ok ?


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by kevinjmh <gi...@git.apache.org>.
Github user kevinjmh commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r243167033
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFile.java ---
    @@ -94,14 +93,9 @@ public CarbonFile getParentFile() {
       public boolean renameForce(String changeToName) {
         FileSystem fs;
         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);
    -        return true;
    -      } else {
    -        return false;
    -      }
    +      fs = fileStatus.getPath().getFileSystem(hadoopConf);
    +      fs.delete(new Path(changeToName), true);
    +      return fs.rename(fileStatus.getPath(), new Path(changeToName));
    --- End diff --
    
    This problem is also for `S3CarbonFile` , `ViewFSCarbonFile`, `LocalCarbonFile` 


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/5892/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/9970/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by kevinjmh <gi...@git.apache.org>.
Github user kevinjmh commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r243819959
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -960,7 +960,7 @@ private CarbonCommonConstants() {
        * If set to GLOBAL_SORT, the sorting scope is bigger and one index tree per task will be
        * created, thus loading is slower but query is faster.
        */
    -  public static final String LOAD_SORT_SCOPE_DEFAULT = "LOCAL_SORT";
    +  public static final String LOAD_SORT_SCOPE_DEFAULT = "NO_SORT";
    --- End diff --
    
    OK


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

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


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3945/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241980353
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AbstractDFSCarbonFile.java ---
    @@ -550,12 +550,10 @@ public DataOutputStream getDataOutputStreamUsingAppend(String path, FileFactory.
         if (null != fileStatus && fileStatus.isDirectory()) {
    --- End diff --
    
    Missed in rebase.. Resolving this now.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    I am changing the implementation for the same to read and copy the data from file system and change the same as suggested by @ravipesala , I get the similar issue with the previous code..as we need a force rename here, we have to delete the file again..and do the previous run..


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241264683
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/CarbonLockFactory.java ---
    @@ -99,11 +103,17 @@ public static ICarbonLock getSystemLevelCarbonLockObj(String locFileLocation, St
           case CarbonCommonConstants.CARBON_LOCK_TYPE_LOCAL:
             return new LocalFileLock(lockFileLocation, lockFile);
           case CarbonCommonConstants.CARBON_LOCK_TYPE_ZOOKEEPER:
    -        return new ZooKeeperLocking(lockFileLocation, lockFile);
    +        return new ZooKeeperLocking(locFileLocation, lockFile);
    +
           case CarbonCommonConstants.CARBON_LOCK_TYPE_HDFS:
    -        return new HdfsFileLock(lockFileLocation, lockFile);
    +        return new HdfsFileLock(locFileLocation, lockFile);
    +
    --- End diff --
    
    no need to add empty Line in here


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4028/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r200064110
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFile.java ---
    @@ -105,18 +103,21 @@ public CarbonFile getParentFile() {
         return null == parent ? null : new AlluxioCarbonFile(parent);
       }
     
    +  /**
    +   * CARBON-2218 Adopting to {@link FileSystem} , in accordance
    +   * with the AlluxioFileSystem Implementation.
    +   * Implicit expection of the AlluxioFileSystem when using this method.
    +   * If success, returns true else false when an Exception is raised.
    +   * @param changetoName
    +   * @return
    +   */
       @Override
       public boolean renameForce(String changetoName) {
         FileSystem fs;
         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);
    -        return true;
    -      } else {
    -        return false;
    -      }
    +      fs.delete(new Path(changetoName), true);
    --- End diff --
    
    Any update on this. Can you suggest on a alternative way.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9797/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/ApacheCarbonPRBuilder1/5388/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241980383
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFile.java ---
    @@ -94,14 +93,9 @@ public CarbonFile getParentFile() {
       public boolean renameForce(String changeToName) {
         FileSystem fs;
         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);
    -        return true;
    -      } else {
    -        return false;
    -      }
    +      fs = fileStatus.getPath().getFileSystem(hadoopConf);
    +      fs.delete(new Path(changeToName), true);
    +      return fs.rename(fileStatus.getPath(), new Path(changeToName));
    --- End diff --
    
    I am removing the delete..as kindle refer to the rename operation in Alluxio..it conflicts the way delete is present..which is handled in the alluxio version from 1.7.1


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4221/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by kevinjmh <gi...@git.apache.org>.
Github user kevinjmh commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r243732085
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -960,7 +960,7 @@ private CarbonCommonConstants() {
        * If set to GLOBAL_SORT, the sorting scope is bigger and one index tree per task will be
        * created, thus loading is slower but query is faster.
        */
    -  public static final String LOAD_SORT_SCOPE_DEFAULT = "LOCAL_SORT";
    +  public static final String LOAD_SORT_SCOPE_DEFAULT = "NO_SORT";
    --- End diff --
    
    why should change this default value?
    
    Some more advices:
    1. blank changes in this file is unnecessary. 
    2. too many commits in this PR, better to merge them into if history does not matter. refer to command `git rebase` to merge the commits into one
    
    Hope this PR can be merged soon


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    > @chandrasaripaka Can you fix the CI error?
    
    @xubo245 , just committed please review and let me know.


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241264360
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/impl/FileFactory.java ---
    @@ -470,10 +475,11 @@ public static void createDirectoryAndSetPermission(String directoryPath, FsPermi
         switch (fileType) {
           case S3:
           case HDFS:
    +      case ALLUXIO:
           case VIEWFS:
             try {
               Path path = new Path(directoryPath);
    -          FileSystem fs = path.getFileSystem(getConfiguration());
    +          FileSystem fs = path.getFileSystem(FileFactory.configuration);
    --- End diff --
    
    Why do you need change this?


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/1912/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/10034/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241290215
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/impl/FileFactory.java ---
    @@ -111,14 +112,9 @@ public static DataInputStream getDataInputStream(String path, FileType fileType)
         return getDataInputStream(path, fileType, -1);
       }
     
    -  public static DataInputStream getDataInputStream(String path, FileType fileType,
    --- End diff --
    
    Oh thats a rebase mistake..I think need to rebase again.


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241264666
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/CarbonLockFactory.java ---
    @@ -99,11 +103,17 @@ public static ICarbonLock getSystemLevelCarbonLockObj(String locFileLocation, St
           case CarbonCommonConstants.CARBON_LOCK_TYPE_LOCAL:
             return new LocalFileLock(lockFileLocation, lockFile);
           case CarbonCommonConstants.CARBON_LOCK_TYPE_ZOOKEEPER:
    -        return new ZooKeeperLocking(lockFileLocation, lockFile);
    +        return new ZooKeeperLocking(locFileLocation, lockFile);
    +
    --- End diff --
    
    no need to add empty Line in here


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

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


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @chandrasaripaka Please rebase it。


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/6535/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/1621/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241939242
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/impl/FileFactory.java ---
    @@ -432,7 +437,7 @@ public static long getDirectorySize(String filePath) throws IOException {
           case VIEWFS:
           case S3:
             Path path = new Path(filePath);
    -        FileSystem fs = path.getFileSystem(getConfiguration());
    +        FileSystem fs = path.getFileSystem(configuration);
    --- End diff --
    
    Resolved.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5033/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5407/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

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


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5155/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/2127/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @ravipesala , @jackylk can we merge the PR?


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @chandrasaripaka OK. I will review this PR for detail this week, and plan to merge it recently.


---

[GitHub] carbondata issue #2161: Adding Alluxio Supporty

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4410/



---

[GitHub] carbondata issue #2161: Adding Alluxio Supporty

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Can one of the admins verify this patch?


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @CarbonDataQA May I know if this has to be fixed from my side..as a part of the pull request, Kindly advise. @xubo245 Also, I dont have access to resolve the conflicts and recommit. Please advise.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @chandrasaripaka  I confirmed and reproduced this issue based on carbonData lastest master and spark-2.3.2.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:

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


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r238211698
  
    --- Diff: core/src/test/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFileTest.java ---
    @@ -108,12 +121,12 @@ public void testListFilesForNullListStatus() {
             alluxioCarbonFile = new AlluxioCarbonFile(fileStatusWithOutDirectoryPermission);
             new MockUp<Path>() {
                 @Mock
    -            public FileSystem getFileSystem(Configuration conf) throws IOException {
    -                return new DistributedFileSystem();
    +            public FileSystem get(FileSystemContext context) throws IOException {
    --- End diff --
    
    Fixed the test in the latest push


---

[GitHub] carbondata issue #2161: Adding Alluxio Support CARBONDATA-2218

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    ok, please change it like that: "[CARBONDATA-2303] clean files issue resolved for partition folder"


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4240/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241980433
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/impl/FileFactory.java ---
    @@ -43,7 +44,7 @@
        * LOGGER
        */
       private static final Logger LOGGER =
    -      LogServiceFactory.getLogService(FileFactory.class.getName());
    +          LogServiceFactory.getLogService(FileFactory.class.getName());
    --- End diff --
    
    Removing the unnecessary formats..thanks for bringing up this.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9850/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9851/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by jackylk <gi...@git.apache.org>.
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


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by kevinjmh <gi...@git.apache.org>.
Github user kevinjmh commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    > Since user normally uses Alluxio as a read cache, I think we can firstly verify carbon on alluxio for the query scenario. As I am still not very sure what is the correct way to implement rename for Alluxio, in the meantime, we can merge this PR first. So please rebase it. @chandrasaripaka
    
    @jackylk My first try wants to use alluxio for query only as you said. But hive stores table location and carbon blocks `alter location` command, I have to create table after carbon runs on alluxio. Please mind this problem.


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241264817
  
    --- Diff: core/src/test/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFileTest.java ---
    @@ -20,20 +20,16 @@
     import mockit.Mock;
     import mockit.MockUp;
     import org.apache.hadoop.conf.Configuration;
    -import org.apache.hadoop.fs.FileStatus;
    -import org.apache.hadoop.fs.FileSystem;
    -import org.apache.hadoop.fs.Options;
    -import org.apache.hadoop.fs.Path;
    -import org.apache.hadoop.hdfs.DistributedFileSystem;
    -import org.apache.hadoop.hdfs.web.WebHdfsFileSystem;
    +import org.apache.hadoop.fs.*;
    +import org.apache.hadoop.fs.permission.FsPermission;
    +import org.apache.hadoop.util.Progressable;
     import org.junit.AfterClass;
     import org.junit.BeforeClass;
     import org.junit.Test;
     
    -import java.io.File;
    -import java.io.FileNotFoundException;
    -import java.io.FileOutputStream;
    -import java.io.IOException;
    +import java.io.*;
    --- End diff --
    
    Please keep import the detail class, it will import more class if change to *


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3817/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r243806487
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java ---
    @@ -960,7 +960,7 @@ private CarbonCommonConstants() {
        * If set to GLOBAL_SORT, the sorting scope is bigger and one index tree per task will be
        * created, thus loading is slower but query is faster.
        */
    -  public static final String LOAD_SORT_SCOPE_DEFAULT = "LOCAL_SORT";
    +  public static final String LOAD_SORT_SCOPE_DEFAULT = "NO_SORT";
    --- End diff --
    
    Actually thats the change coming from Master branch..when I rebased it. Please confirm..and I have squashed into one pull request.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241982006
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFile.java ---
    @@ -94,14 +93,9 @@ public CarbonFile getParentFile() {
       public boolean renameForce(String changeToName) {
         FileSystem fs;
         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);
    -        return true;
    -      } else {
    -        return false;
    -      }
    +      fs = fileStatus.getPath().getFileSystem(hadoopConf);
    +      fs.delete(new Path(changeToName), true);
    +      return fs.rename(fileStatus.getPath(), new Path(changeToName));
    --- End diff --
    
    You mean force rename is handled in Alluxio 1.7.1 ?  which Alluxio version are you using currently?


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @chandrasaripaka Please rebase carefully.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @chandrasaripaka you can put the CarbonFile test with  Alluxio Mini Cluster but make sure it does not go beyond a few seconds to finish the test as it impacts the build time


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by kevinjmh <gi...@git.apache.org>.
Github user kevinjmh commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r243168473
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/AlluxioFileLock.java ---
    @@ -0,0 +1,112 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.core.locks;
    +
    +import java.io.DataOutputStream;
    +import java.io.IOException;
    +
    +import org.apache.carbondata.common.logging.LogServiceFactory;
    +import org.apache.carbondata.core.datastore.filesystem.AlluxioCarbonFile;
    +import org.apache.carbondata.core.datastore.impl.FileFactory;
    +import org.apache.carbondata.core.metadata.AbsoluteTableIdentifier;
    +import org.apache.carbondata.core.util.path.CarbonTablePath;
    +
    +import org.apache.log4j.Logger;
    +
    +/**
    + * This class is used to handle the S3 File locking.
    + * This is acheived using the concept of acquiring the data out stream using Append option.
    + */
    +public class AlluxioFileLock extends AbstractCarbonLock {
    +
    +  /**
    +   * LOGGER
    +   */
    +  private static final Logger LOGGER =
    +          LogServiceFactory.getLogService(AlluxioCarbonFile.class.getName());
    +  /**
    +   * lockFilePath is the location of the lock file.
    +   */
    +  private String lockFilePath;
    +
    +  /**
    +   * lockFileDir is the directory of the lock file.
    +   */
    +  private String lockFileDir;
    +
    +  private DataOutputStream dataOutputStream;
    +
    +  /**
    +   * @param tableIdentifier
    +   * @param lockFile
    +   */
    +  public AlluxioFileLock(AbsoluteTableIdentifier tableIdentifier, String lockFile) {
    +    this(tableIdentifier.getTablePath(), lockFile);
    +  }
    +
    +  /**
    +   * @param lockFileLocation
    +   * @param lockFile
    +   */
    +  public AlluxioFileLock(String lockFileLocation, String lockFile) {
    +    this.lockFileDir = CarbonTablePath.getLockFilesDirPath(lockFileLocation);
    +    this.lockFilePath = CarbonTablePath.getLockFilePath(lockFileLocation, lockFile);
    +    LOGGER.info("Alluxio lock path:" + this.lockFilePath);
    +    initRetry();
    +  }
    +
    +  /* (non-Javadoc)
    +   * @see org.apache.carbondata.core.locks.ICarbonLock#unlock()
    +   */
    +  @Override
    +  public boolean unlock() {
    +    boolean status = false;
    +    if (null != dataOutputStream) {
    +      try {
    +        dataOutputStream.close();
    +        status = true;
    +      } catch (IOException e) {
    +        status = false;
    +      }
    +    }
    +    return status;
    +  }
    +
    +  /* (non-Javadoc)
    +   * @see org.apache.carbondata.core.locks.ICarbonLock#lock()
    +   */
    +  @Override
    +  public boolean lock() {
    +    try {
    +      if (!FileFactory.isFileExist(lockFileDir)) {
    +        FileFactory.mkdirs(lockFileDir, FileFactory.getFileType(lockFileDir));
    +      }
    +      if (!FileFactory.isFileExist(lockFilePath)) {
    +        FileFactory.createNewLockFile(lockFilePath, FileFactory.getFileType(lockFilePath));
    +      }
    +      dataOutputStream =
    +              FileFactory.getDataOutputStreamUsingAppend(lockFilePath,
    +                      FileFactory.getFileType(lockFilePath));
    --- End diff --
    
    Can you insert data into table? I found that the implementation of `append` function in Alluxio required to create a new file.  It will fail if file already exists. For line 100, carbon makes sure the file exists, and finally we always fail when locking.
    
    The code I mention in alluxio is this: 
    https://github.com/Alluxio/alluxio/blob/21a3a87747010f087d1937f48bba3caa883885f8/core/client/hdfs/src/main/java/alluxio/hadoop/AbstractFileSystem.java#L127-L138



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Since user normally uses Alluxio as a read cache, I think we can firstly verify carbon on alluxio for the query scenario. As I am still not very sure what is the correct way to implement rename for Alluxio, in the meantime, we can merge this PR first. So please rebase it. @chandrasaripaka 


---

[GitHub] carbondata issue #2161: Adding Alluxio Support CARBONDATA-2218

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @xubo245 CARBONDATA-2218 , which is the jira I raised before.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/10166/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241264716
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/CarbonLockFactory.java ---
    @@ -99,11 +103,17 @@ public static ICarbonLock getSystemLevelCarbonLockObj(String locFileLocation, St
           case CarbonCommonConstants.CARBON_LOCK_TYPE_LOCAL:
             return new LocalFileLock(lockFileLocation, lockFile);
           case CarbonCommonConstants.CARBON_LOCK_TYPE_ZOOKEEPER:
    -        return new ZooKeeperLocking(lockFileLocation, lockFile);
    +        return new ZooKeeperLocking(locFileLocation, lockFile);
    +
           case CarbonCommonConstants.CARBON_LOCK_TYPE_HDFS:
    -        return new HdfsFileLock(lockFileLocation, lockFile);
    +        return new HdfsFileLock(locFileLocation, lockFile);
    +
           case CarbonCommonConstants.CARBON_LOCK_TYPE_S3:
    -        return new S3FileLock(lockFileLocation, lockFile);
    +        return new S3FileLock(locFileLocation, lockFile);
    +
    --- End diff --
    
    no need to add empty Line in here, please check other places


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
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?


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
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


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @xubo245 , please merge the PR, after a review


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @chandrasaripaka Can you run with this code success? there are some error in my machine:
    
    `2018-12-17 11:24:11 ERROR AlluxioCarbonFile:107 - /xubo/default/alluxio_table_1c201e20-d7df-4b5e-b2c4-b8de82e23942/LockFiles/tablestatus.lock already exists
    java.io.IOException: /xubo/default/alluxio_table_1c201e20-d7df-4b5e-b2c4-b8de82e23942/LockFiles/tablestatus.lock already exists`


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241264812
  
    --- Diff: core/src/test/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFileTest.java ---
    @@ -20,20 +20,16 @@
     import mockit.Mock;
     import mockit.MockUp;
     import org.apache.hadoop.conf.Configuration;
    -import org.apache.hadoop.fs.FileStatus;
    -import org.apache.hadoop.fs.FileSystem;
    -import org.apache.hadoop.fs.Options;
    -import org.apache.hadoop.fs.Path;
    -import org.apache.hadoop.hdfs.DistributedFileSystem;
    -import org.apache.hadoop.hdfs.web.WebHdfsFileSystem;
    +import org.apache.hadoop.fs.*;
    --- End diff --
    
    Please keep import the detail class, it will import more class if change to *


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/2123/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @CarbonDataQA FileNotFoundException means that the credentials Jenkins is using is probably wrong. Or the user account does not have write access to the repo. org.kohsuke.github.GHFileNotFoundException: {"message":"Not Found","documentation_url":"https://developer.github.com/v3/repos/statuses/#create-a-status"} 	at 


---

[GitHub] carbondata issue #2161: Adding Alluxio Supporty

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Can one of the admins verify this patch?


---

[GitHub] carbondata issue #2161: Adding Alluxio Supporty

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @chenliang613 This PR is for supporting the Alluxio as a File System for supporting the CarbonFileFormat. Though there was a AlluxioCarbonFile in the past, this was failing with Alluxio. Since Alluxio does not inherit DistributedFileSystem, it has to be a HadoopFileSystem that will be the change, as technically it does not support the appends on the File. Also, adopting to a AlluxioLock, however one can also use the ZookeeperLock, and also navigate.


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241983377
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AlluxioCarbonFile.java ---
    @@ -94,14 +93,9 @@ public CarbonFile getParentFile() {
       public boolean renameForce(String changeToName) {
         FileSystem fs;
         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);
    -        return true;
    -      } else {
    -        return false;
    -      }
    +      fs = fileStatus.getPath().getFileSystem(hadoopConf);
    +      fs.delete(new Path(changeToName), true);
    +      return fs.rename(fileStatus.getPath(), new Path(changeToName));
    --- End diff --
    
    Actually no I mean..it checks for the existence of the file and renames only ( else it will return false), but will not do a force rename. But I am thinking we can use the same alluxio fs copy, programatically to copy the same contents from one file to another file(changedName under the same directory).


---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241977467
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/AlluxioFileLock.java ---
    @@ -0,0 +1,112 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.carbondata.core.locks;
    +
    +import java.io.DataOutputStream;
    +import java.io.IOException;
    +
    +import org.apache.carbondata.common.logging.LogServiceFactory;
    +import org.apache.carbondata.core.datastore.filesystem.AlluxioCarbonFile;
    +import org.apache.carbondata.core.datastore.impl.FileFactory;
    +import org.apache.carbondata.core.metadata.AbsoluteTableIdentifier;
    +import org.apache.carbondata.core.util.path.CarbonTablePath;
    +
    +import org.apache.log4j.Logger;
    +
    +/**
    + * This class is used to handle the S3 File locking.
    + * This is acheived using the concept of acquiring the data out stream using Append option.
    + */
    +public class AlluxioFileLock extends AbstractCarbonLock {
    --- End diff --
    
    why not use `HdfsFileLock` directly?, this class looks exactly same as `HdfsFileLock` 


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241259379
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/impl/FileFactory.java ---
    @@ -111,14 +112,9 @@ public static DataInputStream getDataInputStream(String path, FileType fileType)
         return getDataInputStream(path, fileType, -1);
       }
     
    -  public static DataInputStream getDataInputStream(String path, FileType fileType,
    --- End diff --
    
    Why do you remove this interface?


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4526/



---

[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2161#discussion_r241976975
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AbstractDFSCarbonFile.java ---
    @@ -550,12 +550,10 @@ public DataOutputStream getDataOutputStreamUsingAppend(String path, FileFactory.
         if (null != fileStatus && fileStatus.isDirectory()) {
    --- End diff --
    
    Now the `pathFilter` is not used here? it may impact the callers of this method.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by chandrasaripaka <gi...@git.apache.org>.
Github user chandrasaripaka commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    @xubo245 , so may I know if the PR can be merged  or it needs some rework..we can attach some test logs.


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    it need Committer review and merge this.


---

[GitHub] carbondata issue #2161: Adding Alluxio Supporty

Posted by chenliang613 <gi...@git.apache.org>.
Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    add to whitelist


---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2161
  
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/8793/



---

[GitHub] carbondata issue #2161: [CARBONDATA-2218] AlluxioCarbonFile while trying to ...

Posted by CarbonDataQA <gi...@git.apache.org>.
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/1916/



---