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]