[GitHub] [carbondata] vikramahuja1001 opened a new pull request #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.

[GitBox]

vikramahuja1001 opened a new pull request #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
URL: https://github.com/apache/carbondata/pull/3659
 
 
   
    ### Why is this PR needed?
   Unable to delete segment in case of SI when table status file is not present.  
   
    ### What changes were proposed in this PR?
   Checking for the table status file before triggering delete for that segment.
       
    ### Does this PR introduce any user interface change?
    - No
    - Yes. (please explain the change and update document)
   
    ### Is any new testcase added?
    - No
    - Yes
   
       
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
URL: https://github.com/apache/carbondata/pull/3659#issuecomment-595229564
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2346/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
URL: https://github.com/apache/carbondata/pull/3659#issuecomment-595229627
 
 
   Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/639/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

Indhumathi27 commented on a change in pull request #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
URL: https://github.com/apache/carbondata/pull/3659#discussion_r388805586
 
 

 ##########
 File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonAddLoadCommand.scala
 ##########
 @@ -93,7 +93,14 @@ case class CarbonAddLoadCommand(
 
     // If a path is already added then we should block the adding of the same path again.
     val allSegments = SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath)
-    if (allSegments.exists(a => a.getPath != null && a.getPath.equalsIgnoreCase(inputPath))) {
+    // If the segment has been already loaded from the same path and its status is SUCCESS or
+    // PARTIALLY_SUCCESS, throw an exception as we should block the adding of the same path again.
+    if (allSegments.exists(a => a.getPath != null && a.getPath.equalsIgnoreCase(inputPath) &&
+                                (a.getSegmentStatus.getMessage
+                                   .equalsIgnoreCase(SegmentStatus.SUCCESS.toString) ||
 
 Review comment:
   Can directly check for `a.getSegmentStatus == SegmentStatus.SUCCESS || a.getSegmentStatus ==
   SegmentStatus .LOAD_PARTIAL_SUCCESS`

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
URL: https://github.com/apache/carbondata/pull/3659#discussion_r390240212
 
 

 ##########
 File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonAddLoadCommand.scala
 ##########
 @@ -93,7 +93,14 @@ case class CarbonAddLoadCommand(
 
     // If a path is already added then we should block the adding of the same path again.
     val allSegments = SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath)
-    if (allSegments.exists(a => a.getPath != null && a.getPath.equalsIgnoreCase(inputPath))) {
+    // If the segment has been already loaded from the same path and its status is SUCCESS or
+    // PARTIALLY_SUCCESS, throw an exception as we should block the adding of the same path again.
+    if (allSegments.exists(a => a.getPath != null && a.getPath.equalsIgnoreCase(inputPath) &&
+                                (a.getSegmentStatus.getMessage
+                                   .equalsIgnoreCase(SegmentStatus.SUCCESS.toString) ||
 
 Review comment:
   format the code properly

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

akashrn5 commented on a change in pull request #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
URL: https://github.com/apache/carbondata/pull/3659#discussion_r397061034
 
 

 ##########
 File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/events/DeleteSegmentByIdListener.scala
 ##########
 @@ -49,8 +51,13 @@ class DeleteSegmentByIdListener extends OperationEventListener with Logging {
           val table = metastore
             .lookupRelation(Some(carbonTable.getDatabaseName), tableName)(sparkSession)
             .asInstanceOf[CarbonRelation].carbonTable
-          CarbonStore
-            .deleteLoadById(loadIds, carbonTable.getDatabaseName, table.getTableName, table)
+          val dataLoadLocation = CarbonTablePath.getTableStatusFilePath(table.getTablePath)
+          // this check is added to check if the table status file exists or not. Delete on index
 
 Review comment:
   rename `dataLoadLocation` to `tableStatusFilePath` , and please improve the comment, make it more meaningful to user, because its not index files, its index table, please update

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
URL: https://github.com/apache/carbondata/pull/3659#issuecomment-609595908
 
 
   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/933/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
URL: https://github.com/apache/carbondata/pull/3659#issuecomment-609595946
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2643/
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

vikramahuja1001 commented on a change in pull request #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
URL: https://github.com/apache/carbondata/pull/3659#discussion_r403964076
 
 

 ##########
 File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/events/DeleteSegmentByIdListener.scala
 ##########
 @@ -49,8 +51,13 @@ class DeleteSegmentByIdListener extends OperationEventListener with Logging {
           val table = metastore
             .lookupRelation(Some(carbonTable.getDatabaseName), tableName)(sparkSession)
             .asInstanceOf[CarbonRelation].carbonTable
-          CarbonStore
-            .deleteLoadById(loadIds, carbonTable.getDatabaseName, table.getTableName, table)
+          val dataLoadLocation = CarbonTablePath.getTableStatusFilePath(table.getTablePath)
+          // this check is added to check if the table status file exists or not. Delete on index
 
 Review comment:
   done

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

vikramahuja1001 commented on a change in pull request #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
URL: https://github.com/apache/carbondata/pull/3659#discussion_r403964131
 
 

 ##########
 File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/management/CarbonAddLoadCommand.scala
 ##########
 @@ -93,7 +93,14 @@ case class CarbonAddLoadCommand(
 
     // If a path is already added then we should block the adding of the same path again.
     val allSegments = SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath)
-    if (allSegments.exists(a => a.getPath != null && a.getPath.equalsIgnoreCase(inputPath))) {
+    // If the segment has been already loaded from the same path and its status is SUCCESS or
+    // PARTIALLY_SUCCESS, throw an exception as we should block the adding of the same path again.
+    if (allSegments.exists(a => a.getPath != null && a.getPath.equalsIgnoreCase(inputPath) &&
+                                (a.getSegmentStatus.getMessage
+                                   .equalsIgnoreCase(SegmentStatus.SUCCESS.toString) ||
 
 Review comment:
   done

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

kunal642 commented on issue #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
URL: https://github.com/apache/carbondata/pull/3659#issuecomment-610772531
 
 
   LGTM

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services

[GitBox]

asfgit closed pull request #3659: [CARBONDATA-3738] : Delete seg. by ID is displaying as failed with invalid ID upon deleting a added parquet segment.
URL: https://github.com/apache/carbondata/pull/3659
 
 
   

----------------------------------------------------------------
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:
[hidden email]


With regards,
Apache Git Services