[GitHub] [carbondata] vikramahuja1001 commented on a change in pull request #4035: [CARBONDATA-4067]: CleanFiles Behaviour Change
vikramahuja1001 commented on a change in pull request #4035:
URL: https://github.com/apache/carbondata/pull/4035#discussion_r534678969
##
File path:
core/src/main/java/org/apache/carbondata/core/util/DeleteLoadFolders.java
##
@@ -173,40 +176,62 @@ 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 cleanStaleInProgress, boolean cleanCompactedAndMFD) {
+if (oneLoad.getVisibility().equalsIgnoreCase("true")) {
+ return checkLoadDeletionLogic(oneLoad, cleanCompactedAndMFD,
cleanStaleInProgress);
}
-
return false;
}
private static boolean checkIfLoadCanBeDeletedPhysically(LoadMetadataDetails
oneLoad,
- boolean isForceDelete) {
+ boolean cleanCompactedAndMFD, 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) {
-return true;
- }
- long deletionTime = oneLoad.getModificationOrDeletionTimestamp();
+if (oneLoad.getPath() == null || oneLoad.getPath().equalsIgnoreCase("NA"))
{
+ return checkLoadDeletionLogic(oneLoad, cleanCompactedAndMFD,
cleanStaleInProgress);
+}
+return false;
+ }
- return TrashUtil.isTrashRetentionTimeoutExceeded(deletionTime) &&
CarbonUpdateUtil
+ private static Boolean checkLoadDeletionLogic(LoadMetadataDetails oneLoad,
+ boolean cleanCompactedAndMFD, boolean cleanStaleInProgress) {
+/*
+ * if cleanStaleInProgress == false and cleanCompactedAndMFD == false,
clean MFD and Compacted
+ * segments after trashtimeout(7days) && query timeout(1 hr)
+ * if cleanStaleInProgress == false and cleanCompactedAndMFD == true,
clean MFD and Compacted
+ * segments immediately
+ * if cleanStaleInProgress == true and cleanCompactedAndMFD == false,
clean Stale Inprogress
+ * segments after 7 days(taking carbon.trash.retention.time value)
+ * if cleanStaleInProgress == true and cleanCompactedAndMFD == true,
clean MFD, Compacted and
+ * stale inprogress segments immediately.
+ */
+if (!cleanCompactedAndMFD && !cleanStaleInProgress) {
+ if (SegmentStatus.COMPACTED == oneLoad.getSegmentStatus() ||
SegmentStatus
+ .MARKED_FOR_DELETE == oneLoad.getSegmentStatus()) {
+long deletionTime = oneLoad.getModificationOrDeletionTimestamp();
+return TrashUtil.isTrashRetentionTimeoutExceeded(deletionTime) &&
CarbonUpdateUtil
.isMaxQueryTimeoutExceeded(deletionTime);
-
+ }
+ return false;
+} else if (cleanCompactedAndMFD && !cleanStaleInProgress) {
+ return SegmentStatus.COMPACTED == oneLoad.getSegmentStatus() ||
SegmentStatus
+.MARKED_FOR_DELETE == oneLoad.getSegmentStatus();
+} else if (!cleanCompactedAndMFD) {
Review comment:
I have added comments in the code to tell what exactly is happening and
also added in the PR description. Please refer it once
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] vikramahuja1001 commented on a change in pull request #4035: [CARBONDATA-4067]: CleanFiles Behaviour Change
vikramahuja1001 commented on a change in pull request #4035:
URL: https://github.com/apache/carbondata/pull/4035#discussion_r534678759
##
File path:
core/src/main/java/org/apache/carbondata/core/util/DeleteLoadFolders.java
##
@@ -173,40 +176,62 @@ 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 cleanStaleInProgress, boolean cleanCompactedAndMFD) {
+if (oneLoad.getVisibility().equalsIgnoreCase("true")) {
+ return checkLoadDeletionLogic(oneLoad, cleanCompactedAndMFD,
cleanStaleInProgress);
}
-
return false;
}
private static boolean checkIfLoadCanBeDeletedPhysically(LoadMetadataDetails
oneLoad,
- boolean isForceDelete) {
+ boolean cleanCompactedAndMFD, 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) {
-return true;
- }
- long deletionTime = oneLoad.getModificationOrDeletionTimestamp();
+if (oneLoad.getPath() == null || oneLoad.getPath().equalsIgnoreCase("NA"))
{
+ return checkLoadDeletionLogic(oneLoad, cleanCompactedAndMFD,
cleanStaleInProgress);
+}
+return false;
+ }
- return TrashUtil.isTrashRetentionTimeoutExceeded(deletionTime) &&
CarbonUpdateUtil
+ private static Boolean checkLoadDeletionLogic(LoadMetadataDetails oneLoad,
+ boolean cleanCompactedAndMFD, boolean cleanStaleInProgress) {
+/*
+ * if cleanStaleInProgress == false and cleanCompactedAndMFD == false,
clean MFD and Compacted
+ * segments after trashtimeout(7days) && query timeout(1 hr)
+ * if cleanStaleInProgress == false and cleanCompactedAndMFD == true,
clean MFD and Compacted
+ * segments immediately
+ * if cleanStaleInProgress == true and cleanCompactedAndMFD == false,
clean Stale Inprogress
+ * segments after 7 days(taking carbon.trash.retention.time value)
+ * if cleanStaleInProgress == true and cleanCompactedAndMFD == true,
clean MFD, Compacted and
+ * stale inprogress segments immediately.
+ */
+if (!cleanCompactedAndMFD && !cleanStaleInProgress) {
+ if (SegmentStatus.COMPACTED == oneLoad.getSegmentStatus() ||
SegmentStatus
+ .MARKED_FOR_DELETE == oneLoad.getSegmentStatus()) {
+long deletionTime = oneLoad.getModificationOrDeletionTimestamp();
+return TrashUtil.isTrashRetentionTimeoutExceeded(deletionTime) &&
CarbonUpdateUtil
.isMaxQueryTimeoutExceeded(deletionTime);
-
+ }
+ return false;
+} else if (cleanCompactedAndMFD && !cleanStaleInProgress) {
Review comment:
previously forceDelete would delete MFD, Compacted and stale INprogress
segments immediately. cleanCompactedAndMFD means only delete MFD,
Compacted(immediately) and cleanStaleInProgress means only delete
staleInprogress segments.
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] vikramahuja1001 commented on a change in pull request #4035: [CARBONDATA-4067]: CleanFiles Behaviour Change
vikramahuja1001 commented on a change in pull request #4035:
URL: https://github.com/apache/carbondata/pull/4035#discussion_r534678032
##
File path:
core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentStatusManager.java
##
@@ -1039,17 +1039,19 @@ private static void
writeLoadMetadata(AbsoluteTableIdentifier identifier,
}
}
- private static ReturnTuple isUpdateRequired(boolean isForceDeletion,
CarbonTable carbonTable,
- AbsoluteTableIdentifier absoluteTableIdentifier, LoadMetadataDetails[]
details) {
+ private static ReturnTuple isUpdateRequired(boolean cleanStaleInProgress,
boolean
Review comment:
Instead of just a force option deleting all MFD, Compacted and stale
Insert In Progress segments, dividing them into 2 parameters,
cleanMFDandCompacted and cleanStaleInProgress. CleanMFDAndCompacted parameter
will decide if MFD and Compacted segments can be deleted and
cleanStaleInProgress will decide if stale InProgress segments can be deleted.
After giving these 2 parameters, force can be removed. if the user wants to do
force deletion, they can set both cleanMFDandCompacted and cleanStaleInProgress
to true.
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
