cecemei commented on code in PR #19059:
URL: https://github.com/apache/druid/pull/19059#discussion_r2862031303


##########
server/src/main/java/org/apache/druid/server/compaction/CompactionMode.java:
##########
@@ -27,5 +27,6 @@ public enum CompactionMode
   /**
    * Indicates that all existing segments of the interval will be picked for 
compaction.
    */
-  FULL_COMPACTION;
+  FULL_COMPACTION,
+  INCREMENTAL_COMPACTION;

Review Comment:
   done



##########
server/src/main/java/org/apache/druid/server/compaction/MostFragmentedIntervalFirstPolicy.java:
##########
@@ -124,7 +147,12 @@ public boolean equals(Object o)
     MostFragmentedIntervalFirstPolicy policy = 
(MostFragmentedIntervalFirstPolicy) o;
     return minUncompactedCount == policy.minUncompactedCount
            && Objects.equals(minUncompactedBytes, policy.minUncompactedBytes)
-           && Objects.equals(maxAverageUncompactedBytesPerSegment, 
policy.maxAverageUncompactedBytesPerSegment);
+           && Objects.equals(maxAverageUncompactedBytesPerSegment, 
policy.maxAverageUncompactedBytesPerSegment)
+           // Use Double.compare instead of == to handle NaN correctly and 
keep equals() consistent with hashCode() (especially for +0.0 vs -0.0).
+           && Double.compare(
+        incrementalCompactionUncompactedRatioThreshold,
+        policy.incrementalCompactionUncompactedRatioThreshold
+    ) == 0;

Review Comment:
   yea it looks odds because intellij styles method call wrapping takes 
precedence over binary expression continuation, fixed it



##########
indexing-service/src/test/java/org/apache/druid/indexing/common/actions/SegmentUpgradeActionTest.java:
##########


Review Comment:
   updated



##########
server/src/main/java/org/apache/druid/server/compaction/CompactionCandidate.java:
##########
@@ -151,6 +151,13 @@ public CompactionStatistics getUncompactedStats()
            ? null : currentStatus.getUncompactedStats();
   }
 
+  @Nullable
+  public List<DataSegment> getUncompactedSegments()
+  {
+    return (currentStatus == null || currentStatus.getUncompactedSegments() == 
null)
+           ? null : currentStatus.getUncompactedSegments();

Review Comment:
   updated



##########
server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java:
##########
@@ -1852,9 +1853,18 @@ protected Set<DataSegment> insertSegments(
   }
 
   /**
-   * Creates new versions of segments appended while a "REPLACE" task was in 
progress.
+   * Creates upgraded versions of segments that were appended while a REPLACE 
task was in progress.

Review Comment:
   true, updated the wording



##########
processing/src/main/java/org/apache/druid/timeline/partition/LinearShardSpec.java:
##########
@@ -55,6 +55,18 @@ public int getNumCorePartitions()
     return 0;
   }
 
+  @Override
+  public ShardSpec withPartitionNum(int partitionNum1)

Review Comment:
   this is from earlier implementation, reverted the 1 suffix



##########
indexing-service/src/main/java/org/apache/druid/indexing/compact/CompactionJobQueue.java:
##########
@@ -282,18 +283,20 @@ private boolean startJobIfPendingAndReady(
     }
 
     // Check if the job is already running, completed or skipped

Review Comment:
   there's no logic change here, just made the code more defensive.  updated 
the comment here. ihmo, `CompactionStatus` is not the best class to use here. 



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