josh-mckenzie commented on code in PR #1770:
URL: https://github.com/apache/cassandra/pull/1770#discussion_r940521631


##########
src/java/org/apache/cassandra/db/commitlog/CommitLogSegmentManagerCDC.java:
##########
@@ -190,14 +189,20 @@ public CommitLogSegment.Allocation allocate(Mutation 
mutation, int size) throws
         return alloc;
     }
 
-    // Non-blocking mode has just recently been enabled for CDC.
-    // The segment is still marked as FORBIDDEN. It should be set to PERMITTED.
-    private void ensureSegmentPermittedIfNotBlockWrites(CommitLogSegment 
segment)
+    // Permit a forbidden segment under the following conditions.
+    // - Non-blocking mode has just recently been enabled for CDC.
+    // - The CDC total space has droppped below the limit (e.g. CDC consumer 
cleans up).
+    private void permitSegmentMaybe(CommitLogSegment segment)
     {
-        if (!DatabaseDescriptor.getCDCBlockWrites() && segment.getCDCState() 
== CDCState.FORBIDDEN)
+        if (segment.getCDCState() != CDCState.FORBIDDEN)
+            return;
+
+        if (!DatabaseDescriptor.getCDCBlockWrites()
+            || cdcSizeTracker.sizeInProgress.get() < 
DatabaseDescriptor.getCDCTotalSpace())

Review Comment:
   I'm confused. Here's what I'm seeing in the code:
   ```
       public CommitLogSegment createSegment()
       {
           CommitLogSegment segment = CommitLogSegment.createSegment(commitLog, 
this);
   
           // Hard link file in cdc folder for realtime tracking
           FileUtils.createHardLink(segment.logFile, segment.getCDCFile());
   
           cdcSizeTracker.processNewSegment(segment);
           return segment;
       }
   ```
   I think the code would need to read something like this to prevent 
hard-linking files with cdc disabled:
   ```
       public CommitLogSegment createSegment()
       {
           CommitLogSegment segment = CommitLogSegment.createSegment(commitLog, 
this);
   
           if (segment.getCDCState() != CDCState.FORBIDDEN)
           {
               // Hard link file in cdc folder for realtime tracking
               FileUtils.createHardLink(segment.logFile, segment.getCDCFile());
           }
   
           cdcSizeTracker.processNewSegment(segment);
           return segment;
       }
   ```
   
   Further, we'd need to add logic to hard link the file on transition from 
FORBIDDEN to PERMITTED if that happened later in its lifecycle.
   
   That make sense?



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

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to