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 <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-HasComments: Yes
