[GitHub] [carbondata] vikramahuja1001 opened a new pull request #4066: [WIP] fixed clean files listener

[GitBox]

vikramahuja1001 opened a new pull request #4066:
URL: https://github.com/apache/carbondata/pull/4066


    ### Why is this PR needed?
   
   
    ### What changes were proposed in this PR?
   
       
    ### 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]

[GitBox]

akashrn5 commented on a change in pull request #4066:
URL: https://github.com/apache/carbondata/pull/4066#discussion_r549358345



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/events/CleanFilesPostEventListener.scala
##########
@@ -133,16 +134,16 @@ class CleanFilesPostEventListener extends OperationEventListener with Logging {
           val carbonFile = FileFactory
             .getCarbonFile(CarbonTablePath
               .getSegmentPath(indexTable.getTablePath, detail.getLoadName))
+          LOGGER.info(s"Deleting segment folder: ${carbonFile.getName}")
           CarbonUtil.deleteFoldersAndFiles(carbonFile)
         }
         unnecessarySegmentsOfSI.foreach { detail =>
           detail.setSegmentStatus(segToStatusMap(detail.getLoadName))
           detail.setVisibility("false")
         }
-
         SegmentStatusManager.writeLoadDetailsIntoFile(
-          indexTable.getMetadataPath + CarbonTablePath.TABLE_STATUS_FILE,
-          unnecessarySegmentsOfSI.toArray)
+          indexTable.getMetadataPath + CarbonCommonConstants.FILE_SEPARATOR +
+            CarbonTablePath.TABLE_STATUS_FILE, indexTableMetadataDetails.toArray)

Review comment:
       here, if you send `indexTableMetadataDetails` again, it will have same status, so you need to change the status and visibility if `unnecessarySegmentsOfSI` inside `indexTableMetadataDetails` and then write




----------------------------------------------------------------
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]

[GitBox]

CarbonDataQA2 commented on pull request #4066:
URL: https://github.com/apache/carbondata/pull/4066#issuecomment-751729365


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5250/
   


----------------------------------------------------------------
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]

[GitBox]

CarbonDataQA2 commented on pull request #4066:
URL: https://github.com/apache/carbondata/pull/4066#issuecomment-751733044


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3489/
   


----------------------------------------------------------------
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]

[GitBox]

vikramahuja1001 commented on a change in pull request #4066:
URL: https://github.com/apache/carbondata/pull/4066#discussion_r549418847



##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/events/CleanFilesPostEventListener.scala
##########
@@ -133,16 +134,16 @@ class CleanFilesPostEventListener extends OperationEventListener with Logging {
           val carbonFile = FileFactory
             .getCarbonFile(CarbonTablePath
               .getSegmentPath(indexTable.getTablePath, detail.getLoadName))
+          LOGGER.info(s"Deleting segment folder: ${carbonFile.getName}")
           CarbonUtil.deleteFoldersAndFiles(carbonFile)
         }
         unnecessarySegmentsOfSI.foreach { detail =>
           detail.setSegmentStatus(segToStatusMap(detail.getLoadName))
           detail.setVisibility("false")
         }
-
         SegmentStatusManager.writeLoadDetailsIntoFile(
-          indexTable.getMetadataPath + CarbonTablePath.TABLE_STATUS_FILE,
-          unnecessarySegmentsOfSI.toArray)
+          indexTable.getMetadataPath + CarbonCommonConstants.FILE_SEPARATOR +
+            CarbonTablePath.TABLE_STATUS_FILE, indexTableMetadataDetails.toArray)

Review comment:
       .filter in this case do not create new duplicate objects, but is just a pointer to that object. When something is changed in `unnecessarySegmentsOfSI`, it will automatically be changed in `indexTableMetadataDetails` as well.




----------------------------------------------------------------
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]

[GitBox]

akashrn5 commented on pull request #4066:
URL: https://github.com/apache/carbondata/pull/4066#issuecomment-751799012


   @vikramahuja1001 please change the title and update description


----------------------------------------------------------------
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]

[GitBox]

vikramahuja1001 commented on pull request #4066:
URL: https://github.com/apache/carbondata/pull/4066#issuecomment-751946546


   @akashrn5 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]

[GitBox]

CarbonDataQA2 commented on pull request #4066:
URL: https://github.com/apache/carbondata/pull/4066#issuecomment-751968105


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5255/
   


----------------------------------------------------------------
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]

[GitBox]

CarbonDataQA2 commented on pull request #4066:
URL: https://github.com/apache/carbondata/pull/4066#issuecomment-751968760


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3494/
   


----------------------------------------------------------------
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]

[GitBox]

akashrn5 commented on pull request #4066:
URL: https://github.com/apache/carbondata/pull/4066#issuecomment-751999747


   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]

[GitBox]

asfgit closed pull request #4066:
URL: https://github.com/apache/carbondata/pull/4066


   


----------------------------------------------------------------
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]