paul-rogers commented on a change in pull request #2000: DRILL-7607: support
dynamic credit based flow control
URL: https://github.com/apache/drill/pull/2000#discussion_r385997746
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java
##########
@@ -90,14 +100,39 @@ public boolean isEmpty() {
@Override
public void add(RawFragmentBatch batch) {
+ int recordCount = batch.getHeader().getDef().getRecordCount();
+ long bathByteSize = batch.getByteCount();
+ if (recordCount != 0) {
+ //skip first header batch
+ totalBatchSize += bathByteSize;
+ sampleTimes++;
+ }
+ if (sampleTimes == maxSampleTimes) {
+ long averageBathSize = totalBatchSize / sampleTimes;
+ //make a decision
+ long limit = context.getAllocator().getLimit();
+ long thresholdNetworkMem = (long) (limit * 0.40);
Review comment:
This code should handle an undersize limit. If the limit can be set, someone
will set it to smaller than a single batch. (I had to deal with this issue when
fixing the external sort several years ago.)
So, the math needs to handle the case where the first batches are already
close to the limit.
In particular, the "available memory" seems to be assumed at 40% of the
limit, or 4 GB for a default limit. The average batch size could, in theory, be
larger, leaving a `runtimeSoftLimit` of 0. So, seems it should be clamped at 1.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
