Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/11692 )
Change subject: IMPALA-7351: Add estimates to Exchange node ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/11692/2/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java File fe/src/main/java/org/apache/impala/planner/ExchangeNode.java: http://gerrit.cloudera.org:8080/#/c/11692/2/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@192 PS2, Line 192: There is also a deferred rpc queue which queues at max : // one rpc payload (containing the rowbatch) per sender in-case the row : // batch queue hits the previously mentioned soft limit In the long run, we should consider either of the followings: - enable spilling in exchange node - throttle the sender until space opens up on exchange node. May waste a bit of network bandwidth in some cases due to resend. I suppose either of the above may eventually get the memory consumption of an exchange node under control. http://gerrit.cloudera.org:8080/#/c/11692/2/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@214 PS2, Line 214: // TODO: will this be different for a broadcast exchange? Yes, for broadcasting, each exchange node will get the aggregate of all senders' produced row batches. May be worth having that distinction here. Also, does the following line make assumption that there is no skew in distribution during data shuffling ? May be worth documenting that. http://gerrit.cloudera.org:8080/#/c/11692/2/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@223 PS2, Line 223: cardinality_ How is this different from getCardinality() ? http://gerrit.cloudera.org:8080/#/c/11692/2/fe/src/main/java/org/apache/impala/planner/PlanNode.java File fe/src/main/java/org/apache/impala/planner/PlanNode.java: http://gerrit.cloudera.org:8080/#/c/11692/2/fe/src/main/java/org/apache/impala/planner/PlanNode.java@74 PS2, Line 74: nit: extra blank line -- To view, visit http://gerrit.cloudera.org:8080/11692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5b577f9511abc48b992e814d50bba4959f23f7fd Gerrit-Change-Number: 11692 Gerrit-PatchSet: 2 Gerrit-Owner: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Tue, 23 Oct 2018 00:56:52 +0000 Gerrit-HasComments: Yes
