capistrant commented on code in PR #19205:
URL: https://github.com/apache/druid/pull/19205#discussion_r2989656110
##########
server/src/main/java/org/apache/druid/server/compaction/MostFragmentedIntervalFirstPolicy.java:
##########
@@ -223,9 +249,25 @@ public Eligibility checkEligibilityForCompaction(
uncompactedBytesRatio,
minUncompactedBytesPercentForFullCompaction
);
- } else {
- return Eligibility.FULL;
}
+
+ // Check uncompacted rows ratio if total rows are available
+ final Long uncompactedRows = uncompacted.getTotalRows();
+ final Long compactedRows = candidate.getCompactedStats().getTotalRows();
+ if (uncompactedRows != null && compactedRows != null) {
+ final double uncompactedRowsRatio = (double) uncompactedRows /
+ (uncompactedRows + compactedRows)
+ * 100;
Review Comment:
while I expect that it is impossible for this to happen (at least I hope),
there is a divide by 0 exception hiding here. can we eagerly sum them and then
skip the ration calculation if they sum to 0 to be safe. maybe a warn log to
say this shouldn't happen too?
##########
server/src/main/java/org/apache/druid/server/compaction/CompactionStatistics.java:
##########
@@ -21,17 +21,20 @@
/**
* Used to track statistics for segments in different states of compaction.
+ * totalRows can be null for old segments where row count was not stored.
*/
public class CompactionStatistics
{
private long totalBytes;
+ private Long totalRows;
Review Comment:
should totalRows get `@Nullable` annotations where applicable in this class?
--
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]
