kfaraz commented on code in PR #18844:
URL: https://github.com/apache/druid/pull/18844#discussion_r2680897457


##########
indexing-service/src/main/java/org/apache/druid/indexing/compact/CompactionJobQueue.java:
##########
@@ -357,14 +357,15 @@ private String startTaskIfReady(CompactionJob job)
   }
 
   /**
-   * Persist the compaction state associated with the given job with {@link 
CompactionStateManager}.
+   * Persist the compaction state associated with the given job with {@link 
CompactionStateStorage}.
    */
   private void persistPendingCompactionState(CompactionJob job)
   {
-    if (job.getCompactionState() != null && 
job.getCompactionStateFingerprint() != null) {
-      jobParams.getCompactionStateManager().persistCompactionState(
+    if (job.getTargetCompactionState() != null && 
job.getTargetCompactionStateFingerprint() != null) {
+      jobParams.getCompactionStateStorageImpl().upsertCompactionState(

Review Comment:
   I wonder if it would make sense for this compaction state and fingerprint to 
be upserted only when the corresponding segments are being committed.
   - The various task types that perform compaction already generate the 
`lastCompactionState` and pass it in the `DataSegment` objects that are 
committed via the task actions (`SegmentTransactionalInsertAction` / 
`SegmentTransactionalReplaceAction`).
   - Overlord just needs to decide whether to persist one compaction state, or 
separate compaction states for all segments, or both.
   
   If we persist the compaction state proactively before the actual compaction 
task has even launched and the compaction task takes too long to finish, this 
fingerprint might be considered unreferenced and cleaned up (in case the clean 
up is too aggressive).
   
   Is there any advantage to persisting this compaction state proactively while 
creating the jobs itself?



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