weijietong 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_r386006585
##########
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();
Review comment:
A good valuable question! thanks, but still a tough question. It is a
resource management problem. Firstly, I did not know the memory limit using a
default 10 gb value until you mentioned it. Thanks for pointing out this.
To drill's random statistic based memory allocation mechanism, it could not
manage one drillbit's memory allocation accurately. Whatever we do ,we will
risk OOM. Think about what other system do, they will account for the total
memory used by one node, if the node has not enough memory to satisfy the
query, the resource manager will not assign a query to that node. After a node
has accepted a query, they normally specify the network memory space and the
execution memory space size separately. So the system would be memory resource
safety.
To solve the memory risk not only to this PR but also to current
implementation, maybe we need a centralized resource manager not current peer
node sketchy resource management model.
We could specify a configure item to set the max network memory usage to an
appropriate value (according to our cluster hardware memory size )to a fragment
node. But we could not control the parallel receiver nodes happened at one
drillbit.
To your last question, the sender's current implementation will hold a
calculated number rows to flush out a batch. So the row number of batch would
not be a problem. But to some situation, like the row has a text column, it's
possible that the text size will not be uniform at different batches, it's
possible to OOM.
Maybe we could check for OOM .If that happened, we could notify the sender
to change to a half credit value ?
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
