Posted by
GitBox on
Dec 04, 2020; 5:49pm
URL: http://apache-carbondata-dev-mailing-list-archive.168.s1.nabble.com/GitHub-carbondata-vikramahuja1001-opened-a-new-pull-request-4035-WIP-CleanFiles-Behaviour-Change-tp104048p104288.html
akashrn5 commented on a change in pull request #4035:
URL:
https://github.com/apache/carbondata/pull/4035#discussion_r535840999##########
File path: core/src/main/java/org/apache/carbondata/core/util/DeleteLoadFolders.java
##########
@@ -173,40 +176,66 @@ public boolean accept(CarbonFile file) {
}
private static boolean checkIfLoadCanBeDeleted(LoadMetadataDetails oneLoad,
- boolean isForceDelete) {
- if ((SegmentStatus.MARKED_FOR_DELETE == oneLoad.getSegmentStatus() ||
- SegmentStatus.COMPACTED == oneLoad.getSegmentStatus() ||
- SegmentStatus.INSERT_IN_PROGRESS == oneLoad.getSegmentStatus() ||
- SegmentStatus.INSERT_OVERWRITE_IN_PROGRESS == oneLoad.getSegmentStatus())
- && oneLoad.getVisibility().equalsIgnoreCase("true")) {
- if (isForceDelete) {
- return true;
- }
- long deletionTime = oneLoad.getModificationOrDeletionTimestamp();
- return TrashUtil.isTrashRetentionTimeoutExceeded(deletionTime) && CarbonUpdateUtil
- .isMaxQueryTimeoutExceeded(deletionTime);
+ boolean isForceDelete, boolean cleanStaleInProgress) {
+ if (oneLoad.getVisibility().equalsIgnoreCase("true")) {
+ return canDeleteThisLoad(oneLoad, isForceDelete, cleanStaleInProgress);
}
-
return false;
}
private static boolean checkIfLoadCanBeDeletedPhysically(LoadMetadataDetails oneLoad,
- boolean isForceDelete) {
+ boolean isForceDelete, boolean cleanStaleInProgress) {
// Check if the segment is added externally and path is set then do not delete it
- if ((SegmentStatus.MARKED_FOR_DELETE == oneLoad.getSegmentStatus()
- || SegmentStatus.COMPACTED == oneLoad.getSegmentStatus()) && (oneLoad.getPath() == null
- || oneLoad.getPath().equalsIgnoreCase("NA"))) {
- if (isForceDelete) {
+ if (oneLoad.getPath() == null || oneLoad.getPath().equalsIgnoreCase("NA")) {
+ return canDeleteThisLoad(oneLoad, isForceDelete, cleanStaleInProgress);
+ }
+ return false;
+ }
+
+ private static Boolean canDeleteThisLoad(LoadMetadataDetails oneLoad,
+ boolean isForceDelete, boolean cleanStaleInProgress) {
+ /*
+ * if cleanStaleInProgress == false and force == false, clean MFD and Compacted
Review comment:
```suggestion
* if cleanStaleInProgress == false and isForceDelete== false, clean MFD and Compacted
```
please handle in other comments
##########
File path: core/src/main/java/org/apache/carbondata/core/util/DeleteLoadFolders.java
##########
@@ -173,40 +176,66 @@ public boolean accept(CarbonFile file) {
}
private static boolean checkIfLoadCanBeDeleted(LoadMetadataDetails oneLoad,
- boolean isForceDelete) {
- if ((SegmentStatus.MARKED_FOR_DELETE == oneLoad.getSegmentStatus() ||
- SegmentStatus.COMPACTED == oneLoad.getSegmentStatus() ||
- SegmentStatus.INSERT_IN_PROGRESS == oneLoad.getSegmentStatus() ||
- SegmentStatus.INSERT_OVERWRITE_IN_PROGRESS == oneLoad.getSegmentStatus())
- && oneLoad.getVisibility().equalsIgnoreCase("true")) {
- if (isForceDelete) {
- return true;
- }
- long deletionTime = oneLoad.getModificationOrDeletionTimestamp();
- return TrashUtil.isTrashRetentionTimeoutExceeded(deletionTime) && CarbonUpdateUtil
- .isMaxQueryTimeoutExceeded(deletionTime);
+ boolean isForceDelete, boolean cleanStaleInProgress) {
+ if (oneLoad.getVisibility().equalsIgnoreCase("true")) {
+ return canDeleteThisLoad(oneLoad, isForceDelete, cleanStaleInProgress);
}
-
return false;
}
private static boolean checkIfLoadCanBeDeletedPhysically(LoadMetadataDetails oneLoad,
- boolean isForceDelete) {
+ boolean isForceDelete, boolean cleanStaleInProgress) {
// Check if the segment is added externally and path is set then do not delete it
- if ((SegmentStatus.MARKED_FOR_DELETE == oneLoad.getSegmentStatus()
- || SegmentStatus.COMPACTED == oneLoad.getSegmentStatus()) && (oneLoad.getPath() == null
- || oneLoad.getPath().equalsIgnoreCase("NA"))) {
- if (isForceDelete) {
+ if (oneLoad.getPath() == null || oneLoad.getPath().equalsIgnoreCase("NA")) {
+ return canDeleteThisLoad(oneLoad, isForceDelete, cleanStaleInProgress);
+ }
+ return false;
+ }
+
+ private static Boolean canDeleteThisLoad(LoadMetadataDetails oneLoad,
+ boolean isForceDelete, boolean cleanStaleInProgress) {
+ /*
+ * if cleanStaleInProgress == false and force == false, clean MFD and Compacted
+ * segments will depend on query timeout(1 hr) and trashRetentionTimeout(7 days, default).
+ * For example:
+ * If trashRetentionTimeout is 7 days and query timeout is 1 hr--> Delete after 7 days
+ * If trashRetentionTimeout is 0 days and query timeout is 1 hr--> Delete after 1 hr
+ *
+ * if cleanStaleInProgress == false and force == true, clean MFD and Compacted
+ * segments immediately(Do not check for any timeout)
+ *
+ * if cleanStaleInProgress == true and force == false, clean Stale Inprogress, MFD and
+ * compacted segments after 7 days(taking carbon.trash.retention.time value)
+ *
+ * if cleanStaleInProgress == true and force == true, clean MFD, Compacted and
+ * stale inprogress segments immediately.(Do not check for any timeout)
+ */
+ if (isForceDelete) {
+ // immediately delete compacted and MFD
+ if (SegmentStatus.COMPACTED == oneLoad.getSegmentStatus() || SegmentStatus
+ .MARKED_FOR_DELETE == oneLoad.getSegmentStatus()) {
return true;
}
- long deletionTime = oneLoad.getModificationOrDeletionTimestamp();
-
+ // immediately delete inprogress segments if cleanstaleinprogress is true
+ return cleanStaleInProgress && (SegmentStatus.INSERT_IN_PROGRESS == oneLoad
+ .getSegmentStatus() || SegmentStatus.INSERT_OVERWRITE_IN_PROGRESS == oneLoad
+ .getSegmentStatus());
+ }
+ long deletionTime = oneLoad.getModificationOrDeletionTimestamp();
+ // in case there is no deletion or modification timestamp, take the load start time as
+ // deleteTime
+ if (deletionTime == 0) {
+ deletionTime = oneLoad.getLoadStartTime();
+ }
+ if (SegmentStatus.COMPACTED == oneLoad.getSegmentStatus() || SegmentStatus
+ .MARKED_FOR_DELETE == oneLoad.getSegmentStatus()) {
+ // delete MFD, compacted segments after checking trash timeout and query timeout
return TrashUtil.isTrashRetentionTimeoutExceeded(deletionTime) && CarbonUpdateUtil
- .isMaxQueryTimeoutExceeded(deletionTime);
-
+ .isMaxQueryTimeoutExceeded(deletionTime);
}
-
- return false;
+ return (SegmentStatus.INSERT_IN_PROGRESS == oneLoad.getSegmentStatus() || SegmentStatus
Review comment:
where is the handling of the discussion, like, when any clean files called, we should check the trash folder and based on timeout we should delete it?
##########
File path: core/src/main/java/org/apache/carbondata/core/util/DeleteLoadFolders.java
##########
@@ -173,40 +176,66 @@ public boolean accept(CarbonFile file) {
}
private static boolean checkIfLoadCanBeDeleted(LoadMetadataDetails oneLoad,
- boolean isForceDelete) {
- if ((SegmentStatus.MARKED_FOR_DELETE == oneLoad.getSegmentStatus() ||
- SegmentStatus.COMPACTED == oneLoad.getSegmentStatus() ||
- SegmentStatus.INSERT_IN_PROGRESS == oneLoad.getSegmentStatus() ||
- SegmentStatus.INSERT_OVERWRITE_IN_PROGRESS == oneLoad.getSegmentStatus())
- && oneLoad.getVisibility().equalsIgnoreCase("true")) {
- if (isForceDelete) {
- return true;
- }
- long deletionTime = oneLoad.getModificationOrDeletionTimestamp();
- return TrashUtil.isTrashRetentionTimeoutExceeded(deletionTime) && CarbonUpdateUtil
- .isMaxQueryTimeoutExceeded(deletionTime);
+ boolean isForceDelete, boolean cleanStaleInProgress) {
+ if (oneLoad.getVisibility().equalsIgnoreCase("true")) {
+ return canDeleteThisLoad(oneLoad, isForceDelete, cleanStaleInProgress);
}
-
return false;
}
private static boolean checkIfLoadCanBeDeletedPhysically(LoadMetadataDetails oneLoad,
- boolean isForceDelete) {
+ boolean isForceDelete, boolean cleanStaleInProgress) {
// Check if the segment is added externally and path is set then do not delete it
- if ((SegmentStatus.MARKED_FOR_DELETE == oneLoad.getSegmentStatus()
- || SegmentStatus.COMPACTED == oneLoad.getSegmentStatus()) && (oneLoad.getPath() == null
- || oneLoad.getPath().equalsIgnoreCase("NA"))) {
- if (isForceDelete) {
+ if (oneLoad.getPath() == null || oneLoad.getPath().equalsIgnoreCase("NA")) {
+ return canDeleteThisLoad(oneLoad, isForceDelete, cleanStaleInProgress);
+ }
+ return false;
+ }
+
+ private static Boolean canDeleteThisLoad(LoadMetadataDetails oneLoad,
+ boolean isForceDelete, boolean cleanStaleInProgress) {
+ /*
+ * if cleanStaleInProgress == false and force == false, clean MFD and Compacted
+ * segments will depend on query timeout(1 hr) and trashRetentionTimeout(7 days, default).
+ * For example:
+ * If trashRetentionTimeout is 7 days and query timeout is 1 hr--> Delete after 7 days
+ * If trashRetentionTimeout is 0 days and query timeout is 1 hr--> Delete after 1 hr
+ *
+ * if cleanStaleInProgress == false and force == true, clean MFD and Compacted
+ * segments immediately(Do not check for any timeout)
+ *
+ * if cleanStaleInProgress == true and force == false, clean Stale Inprogress, MFD and
+ * compacted segments after 7 days(taking carbon.trash.retention.time value)
+ *
+ * if cleanStaleInProgress == true and force == true, clean MFD, Compacted and
+ * stale inprogress segments immediately.(Do not check for any timeout)
+ */
+ if (isForceDelete) {
+ // immediately delete compacted and MFD
+ if (SegmentStatus.COMPACTED == oneLoad.getSegmentStatus() || SegmentStatus
+ .MARKED_FOR_DELETE == oneLoad.getSegmentStatus()) {
return true;
}
- long deletionTime = oneLoad.getModificationOrDeletionTimestamp();
-
+ // immediately delete inprogress segments if cleanstaleinprogress is true
+ return cleanStaleInProgress && (SegmentStatus.INSERT_IN_PROGRESS == oneLoad
+ .getSegmentStatus() || SegmentStatus.INSERT_OVERWRITE_IN_PROGRESS == oneLoad
+ .getSegmentStatus());
+ }
+ long deletionTime = oneLoad.getModificationOrDeletionTimestamp();
Review comment:
you can combine from 224 to 229 to conditional statement
----------------------------------------------------------------
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]