Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11692 )
Change subject: IMPALA-7351: Add estimates to Exchange node ...................................................................... Patch Set 2: (8 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: yup that can be handled as a part of IMPALA-5485 http://gerrit.cloudera.org:8080/#/c/11692/2/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@201 PS2, Line 201: int numSenders = 0; > I think I'd prefer if some of these sub-computations were broken out into t Done http://gerrit.cloudera.org:8080/#/c/11692/2/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@213 PS2, Line 213: Reciev > Receive Done 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 sen Done 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? > Yeah agreed on both points. Done 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() ? Done http://gerrit.cloudera.org:8080/#/c/11692/2/fe/src/main/java/org/apache/impala/planner/ExchangeNode.java@230 PS2, Line 230: // TODO: set a min cap = MemPool::INITIAL_CHUNK_SIZE * someFactor? I have noticed a > This seems reasonable. If you're empirically seeing a minimum of 16KB that Done 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 Done -- 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: 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 21:16:14 +0000 Gerrit-HasComments: Yes
