Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4053: Address follow up comments for IMPALA-3610
......................................................................


Patch Set 2:

(1 comment)

let's put this on hold until mostafa verifies that the general approach fixes 
the observed memory issue.

http://gerrit.cloudera.org:8080/#/c/4306/2/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

Line 2090:     state->AssignOutgoingFilter(params, filter_mem_tracker_.get(), 
rpc_params.get());
> We didn't need to TryConsume() memory for a broadcast filter update as the 
i think you're over-finessing this. if a single 8mb filter makes the query 
exceed a limit, things are precariously tight anyway, and chances are the query 
will not finish. trying to optimize for the case where the first filter is over 
the limit but maybe one of the subsequent filters isn't won't change the 
picture. it is better to keep the code simple.


-- 
To view, visit http://gerrit.cloudera.org:8080/4306
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7499558cc9d3622a9f3b68904aa5fe92a113f579
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to