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



##########
File path: 
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##########
@@ -1427,6 +1427,25 @@ private CarbonCommonConstants() {
 
   public static final String BITSET_PIPE_LINE_DEFAULT = "true";
 
+  public static final String MICROSECONDS_IN_A_DAY = "86400000";
+
+  /**
+   * this is the user defined time(in days), when a specific timestamp 
subdirectory in
+   * trash folder will expire
+   */
+  @CarbonProperty
+  public static final String TRASH_EXPIRATION_TIME = 
"carbon.trash.expiration.time";

Review comment:
       ```suggestion
     public static final String CARBON_TRASH_EXPIRATION_TIME = 
"carbon.trash.expiration.time";
   ```

##########
File path: 
core/src/main/java/org/apache/carbondata/core/metadata/SegmentFileStore.java
##########
@@ -1105,28 +1109,79 @@ public static void cleanSegments(CarbonTable table, 
List<PartitionSpec> partitio
    * @throws IOException
    */
   public static void deleteSegment(String tablePath, Segment segment,
-      List<PartitionSpec> partitionSpecs,
-      SegmentUpdateStatusManager updateStatusManager) throws Exception {
+      List<PartitionSpec> partitionSpecs, SegmentUpdateStatusManager 
updateStatusManager,
+      SegmentStatus segmentStatus, Boolean isPartitionTable, String timeStamp)

Review comment:
       please rename timeStamp, same as above comment

##########
File path: 
core/src/main/java/org/apache/carbondata/core/util/DeleteLoadFolders.java
##########
@@ -67,22 +69,23 @@ private static String 
getSegmentPath(AbsoluteTableIdentifier identifier,
   }
 
   public static void physicalFactAndMeasureMetadataDeletion(CarbonTable 
carbonTable,
-      LoadMetadataDetails[] newAddedLoadHistoryList,
-      boolean isForceDelete,
-      List<PartitionSpec> specs) {
+      LoadMetadataDetails[] newAddedLoadHistoryList, boolean isForceDelete,
+      List<PartitionSpec> specs, String timeStamp) {

Review comment:
       variable name

##########
File path: 
core/src/main/java/org/apache/carbondata/core/util/CarbonProperties.java
##########
@@ -2116,6 +2086,20 @@ public int getMaxSIRepairLimit(String dbName, String 
tableName) {
     return Math.abs(Integer.parseInt(thresholdValue));
   }
 
+  /**
+   * The below method returns the microseconds after which the trash folder 
will expire
+   */
+  public long getTrashFolderExpirationTime() {
+    String configuredValue = 
getProperty(CarbonCommonConstants.TRASH_EXPIRATION_DAYS,
+            CarbonCommonConstants.TRASH_EXPIRATION_DAYS_DEFAULT);
+    int result = Integer.parseInt(configuredValue);

Review comment:
       it may throw numberFormatException

##########
File path: 
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##########
@@ -1427,6 +1427,25 @@ private CarbonCommonConstants() {
 
   public static final String BITSET_PIPE_LINE_DEFAULT = "true";
 
+  public static final String MICROSECONDS_IN_A_DAY = "86400000";
+
+  /**
+   * this is the user defined time(in days), when a specific timestamp 
subdirectory in
+   * trash folder will expire
+   */
+  @CarbonProperty
+  public static final String TRASH_EXPIRATION_TIME = 
"carbon.trash.expiration.time";
+
+  /**
+   * Default expiration time of trash folder is 3 days.
+   */
+  public static final String TRASH_EXPIRATION_TIME_DEFAULT = "3";

Review comment:
       ```suggestion
     public static final String CARBON_TRASH_EXPIRATION_TIME_DEFAULT = "3";
   ```

##########
File path: 
core/src/main/java/org/apache/carbondata/core/metadata/SegmentFileStore.java
##########
@@ -1033,7 +1034,7 @@ public static void commitDropPartitions(CarbonTable 
carbonTable, String uniqueId
    * @throws IOException
    */
   public static void cleanSegments(CarbonTable table, List<PartitionSpec> 
partitionSpecs,
-      boolean forceDelete) throws IOException {
+      String timeStamp, boolean forceDelete) throws IOException {

Review comment:
       what is this timestamp? please give a meaningful variable name

##########
File path: 
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##########
@@ -1427,6 +1428,25 @@ private CarbonCommonConstants() {
 
   public static final String BITSET_PIPE_LINE_DEFAULT = "true";
 
+  public static final long MILLIS_SECONDS_IN_A_DAY = TimeUnit.DAYS.toMillis(1);
+
+  /**
+   * this is the user defined time(in days), when a specific timestamp 
subdirectory in
+   * trash folder will expire
+   */
+  @CarbonProperty
+  public static final String TRASH_EXPIRATION_DAYS = 
"carbon.trash.expiration.days";
+
+  /**
+   * Default expiration time of trash folder is 3 days.
+   */
+  public static final String TRASH_EXPIRATION_DAYS_DEFAULT = "3";

Review comment:
       ```suggestion
     public static final String CARBON_TRASH_EXPIRATION_DAYS_DEFAULT = "3";
   ```

##########
File path: 
core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentStatusManager.java
##########
@@ -1049,7 +1049,7 @@ private static ReturnTuple isUpdateRequired(boolean 
isForceDeletion, CarbonTable
   }
 
   public static void deleteLoadsAndUpdateMetadata(CarbonTable carbonTable, 
boolean isForceDeletion,
-      List<PartitionSpec> partitionSpecs) throws IOException {
+      List<PartitionSpec> partitionSpecs, String timeStamp) throws IOException 
{

Review comment:
       please rename everywhere

##########
File path: 
core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
##########
@@ -792,4 +793,9 @@ public static String getParentPath(String dataFilePath) {
       return dataFilePath;
     }
   }
+
+  public static String getTrashFolder(String carbonTablePath) {

Review comment:
       ```suggestion
     public static String getTrashFolderPath(String carbonTablePath) {
   ```

##########
File path: 
core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##########
@@ -1427,6 +1428,25 @@ private CarbonCommonConstants() {
 
   public static final String BITSET_PIPE_LINE_DEFAULT = "true";
 
+  public static final long MILLIS_SECONDS_IN_A_DAY = TimeUnit.DAYS.toMillis(1);
+
+  /**
+   * this is the user defined time(in days), when a specific timestamp 
subdirectory in
+   * trash folder will expire
+   */
+  @CarbonProperty
+  public static final String TRASH_EXPIRATION_DAYS = 
"carbon.trash.expiration.days";

Review comment:
       ```suggestion
     public static final String CARBON_TRASH_EXPIRATION_DAYS = 
"carbon.trash.expiration.days";
   ```




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


Reply via email to