aho135 commented on code in PR #19627:
URL: https://github.com/apache/druid/pull/19627#discussion_r3532572681
##########
processing/src/main/java/org/apache/druid/query/groupby/GroupByStatsProvider.java:
##########
@@ -224,9 +266,24 @@ public void mergeBufferAcquisitionTime(long delay)
mergeBufferAcquisitionTimeNs.addAndGet(delay);
}
- public void maxMergeBufferUsedBytes(long bytes)
+ /**
+ * Accumulates the peak merge-buffer usage of one grouper (slice). Despite
the previous "max" naming, this method
+ * sums across the slices a query holds; see {@link #mergeBufferUsedBytes}.
+ */
+ public void addMergeBufferUsedBytes(long bytes)
+ {
+ mergeBufferUsedBytes.addAndGet(bytes);
+ }
+
+ /**
+ * Records one slice's peak used bytes and its spill threshold ({@code
sliceSize * maxLoadFactor}). Both are tracked
+ * as maxima, so after all slices close the pair describes the single
fullest slice — the one that drives spilling.
+ * Used to compute {@code mergeBuffer/maxSpillProximity}.
+ */
+ public void sliceUsage(long usedBytes, long spillThresholdBytes)
{
- maxMergeBufferUsedBytes.addAndGet(bytes);
+ maxSliceUsedBytes.accumulateAndGet(usedBytes, Math::max);
+ sliceSpillThresholdBytes.accumulateAndGet(spillThresholdBytes,
Math::max);
Review Comment:
Good catch — fixed in fc770ee. I replaced the two independent maxima
(`maxSliceUsedBytes` and `sliceSpillThresholdBytes`) with a single
`DoubleAccumulator` that maxes the per-slice ratio: `sliceUsage()` now computes
`min(1.0, usedBytes / spillThresholdBytes)` at the point of measurement, so
each slice's used bytes stay paired with its own threshold.
This closes exactly the case you described: when one `PerQueryStats` flows
through both small `ConcurrentGrouper` slices and a full-buffer
`SpillingGrouper` (subtotal/nested processing), a saturated small slice now
reports the true `1.0` instead of being diluted to ~`1/concurrencyHint` by the
larger threshold winning the separate max. Added
`testSpillProximityKeepsPerSliceRatioWhenThresholdsDiffer` covering the
mixed-threshold scenario.
--
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]
