[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
Github user asfgit closed the pull request at: https://github.com/apache/carbondata/pull/2161 ---
[GitHub] carbondata pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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);
}
+ /**
+ * RenameForce of the fileName for the AlluxioFileSystem
Implementation.
+ * Involves by opening a {@link FSDataInputStream} from the existing
path and copy
+ * bytes to {@link FSDataOutputStream}.
+ *
+ * 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.
+ * @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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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);
}
+ /**
+ * RenameForce of the fileName for the AlluxioFileSystem
Implementation.
+ * Involves by opening a {@link FSDataInputStream} from the existing
path and copy
+ * bytes to {@link FSDataOutputStream}.
+ *
+ * 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.
+ * @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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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);
}
+ /**
+ * RenameForce of the fileName for the AlluxioFileSystem
Implementation.
+ * Involves by opening a {@link FSDataInputStream} from the existing
path and copy
+ * bytes to {@link FSDataOutputStream}.
+ *
+ * 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.
+ * @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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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);
}
+ /**
+ * RenameForce of the fileName for the AlluxioFileSystem
Implementation.
+ * Involves by opening a {@link FSDataInputStream} from the existing
path and copy
+ * bytes to {@link FSDataOutputStream}.
+ *
+ * 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.
+ * @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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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...
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...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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...
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...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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() {
@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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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() {
@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...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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 pull request #2161: [CARBONDATA-2218] AlluxioCarbonFile while try...
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.
---
