[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4096: [CARBONDATA-4133] Concurrent Insert Overwrite with static partition on Index server fails

2021-03-08 Thread GitBox


Indhumathi27 commented on a change in pull request #4096:
URL: https://github.com/apache/carbondata/pull/4096#discussion_r589412990



##
File path: 
core/src/main/java/org/apache/carbondata/core/indexstore/ExtendedBlocklet.java
##
@@ -225,25 +223,15 @@ public void deserializeFields(DataInput in, String[] 
locations, String tablePath
 if (in.readBoolean()) {
   indexUniqueId = in.readUTF();
 }
-String filePath = getPath();
-boolean isLocalFile = FileFactory.getCarbonFile(filePath) instanceof 
LocalCarbonFile;
-
-// For external segment, table path need not be appended to filePath as 
contains has full path
-// Example filepath for ext segment:
-//  1. /home/user/carbondata/integration/spark/newsegmentpath
-//  2. hdfs://hacluster/opt/newsegmentpath/
-// Example filepath for loaded segment: /Fact/Part/Segment0
-// To identify a filePath as ext segment path,
-// for other storage systems (hdfs,s3): filePath doesn't start with File 
separator.
-// for ubuntu storage: it starts with File separator, so check if given 
path exists or not.
-if ((!isLocalFile && filePath.startsWith(File.separator)) || (isLocalFile 
&& !FileFactory
-.isFileExist(filePath))) {
-  setFilePath(tablePath + filePath);
-} else {
-  setFilePath(filePath);
-}
 boolean isSplitPresent = in.readBoolean();
 if (isSplitPresent) {
+  String filePath = getPath();
+  boolean isExternalPath = in.readBoolean();

Review comment:
   ```suggestion
 boolean isExternalSegment = in.readBoolean();
   ```





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4096: [CARBONDATA-4133] Concurrent Insert Overwrite with static partition on Index server fails

2021-03-07 Thread GitBox


Indhumathi27 commented on a change in pull request #4096:
URL: https://github.com/apache/carbondata/pull/4096#discussion_r589197682



##
File path: 
core/src/main/java/org/apache/carbondata/core/indexstore/ExtendedBlockletWrapper.java
##
@@ -121,8 +121,9 @@ public ExtendedBlockletWrapper(List 
extendedBlockletList, Stri
 DataOutputStream stream = new DataOutputStream(bos);
 try {
   for (ExtendedBlocklet extendedBlocklet : extendedBlockletList) {
+boolean isExternalPath = 
!extendedBlocklet.getFilePath().contains(tablePath);

Review comment:
   what if external segment path starts with table path?

##
File path: core/src/main/java/org/apache/carbondata/core/index/Segment.java
##
@@ -378,11 +382,10 @@ public void write(DataOutput out) throws IOException {
   out.writeUTF(segmentString);
 }
 out.writeLong(indexSize);
-if (segmentPath == null) {
-  out.writeBoolean(false);
-} else {
+if (isExternalSegment()) {

Review comment:
   No need to have if check. Just "out..writeBoolean(isExternalSegment)"

##
File path: core/src/main/java/org/apache/carbondata/core/index/Segment.java
##
@@ -79,7 +79,9 @@
   /**
* Path of segment where it exists
*/
-  private String segmentPath;
+  private transient String segmentPath;
+
+  private boolean isExternalSegment = false;

Review comment:
   please add comment





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org