rohangarg commented on code in PR #13420:
URL: https://github.com/apache/druid/pull/13420#discussion_r1032532619
##########
processing/src/main/java/org/apache/druid/query/QueryMetrics.java:
##########
@@ -396,6 +396,30 @@
*/
QueryMetrics<QueryType> reportParallelMergeTotalCpuTime(long timeNs);
+ /**
+ * Reports broker total "wall" time in nanoseconds from parallel merge start
sequence creation to total
+ * consumption.
+ */
+ QueryMetrics<QueryType> reportParallelMergeTotalTime(long timeNs);
+
+ /**
+ * Reports broker "wall" time in nanoseconds for the fastest parallel merge
sequence partition to be 'initialized',
+ * where 'initialized' is time to the first result batch is populated from
data servers and merging can begin.
+ *
+ * Similar to query 'time to first byte' metrics, except is a composite of
the whole group of data servers which are
+ * present in the merge partition, which all must supply an initial result
batch before merging can actually begin.
+ */
+ QueryMetrics<QueryType> reportParallelMergeFastestPartitionTime(long timeNs);
Review Comment:
1. would the slowest `ttfb` node produce slowest partition and same for the
fastest? or could it be that the result batching changes the distribution of
slowest and fastest partition - hence these metrics might be different from
`ttfb`
2. should this method also include `initialized` in it somewhere?
##########
core/src/main/java/org/apache/druid/java/util/common/guava/ParallelMergeCombiningSequence.java:
##########
@@ -121,8 +123,9 @@ public <OutType> Yielder<OutType> toYielder(OutType
initValue, YieldingAccumulat
if (inputSequences.isEmpty()) {
return Sequences.<T>empty().toYielder(initValue, accumulator);
}
-
- final BlockingQueue<ResultBatch<T>> outputQueue = new
ArrayBlockingQueue<>(queueSize);
+ // we make final output queue larger than the merging queues so if
downstream readers are slower to read there is
+ // less chance of blocking the merge
+ final BlockingQueue<ResultBatch<T>> outputQueue = new
ArrayBlockingQueue<>(4 * queueSize);
Review Comment:
I think this might conflict with the `maxQueuedBytes` limit but maybe this
won't hog too much memory as well so is fine. wdyt?
--
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]
