[GitHub] [cassandra] yifan-c commented on a change in pull request #1379: CASSANDRA-17233: Fix deleting old CDC commit log segments

[GitBox <gi...@apache.org>]

yifan-c commented on a change in pull request #1379:
URL: https://github.com/apache/cassandra/pull/1379#discussion_r786422090



##########
File path: src/java/org/apache/cassandra/db/commitlog/CommitLogSegmentManagerCDC.java
##########
@@ -84,21 +84,46 @@ public void discard(CommitLogSegment segment, boolean delete)
 
     /**
      * Delete the oldest hard-linked CDC commit log segment to free up space.
+     * @param bytesToFree, the minimum space to free up
      * @return total deleted file size in bytes
      */
-    public long deleteOldestLinkedCDCCommitLogSegment()
+    public long deleteOldLinkedCDCCommitLogSegment(long bytesToFree)
     {
+        if (bytesToFree <= 0)
+            return 0;
+
         File cdcDir = new File(DatabaseDescriptor.getCDCLogLocation());
         Preconditions.checkState(cdcDir.isDirectory(), "The CDC directory does not exist.");
         File[] files = cdcDir.tryList(f -> CommitLogDescriptor.isValid(f.name()));
-        Preconditions.checkState(files != null && files.length > 0,
-                                 "There should be at least 1 CDC commit log segment.");
+        if (files == null || files.length == 0)
+        {
+            logger.warn("Skip deleting due to no CDC commit log segments found.");
+            return 0;
+        }
         List<File> sorted = Arrays.stream(files)
-                                  .sorted(Comparator.comparingLong(File::lastModified))
+                                  // commit log file name (contains id) increases monotonically

Review comment:
       Agree. I hesitated between comparing the file name and the `id` value before pushing the commit. 
   Here, it only depends on the `id` field to sort. I will update the comparator to use id, so it is explicit. 




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

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org