Hello Quanlong Huang,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/24784
to review the following change.
Change subject: IMPALA-15324: Size exchange memory on input rows
......................................................................
IMPALA-15324: Size exchange memory on input rows
ExchangeNode sized both parts of its memory estimate from its own
cardinality, which computeStats() has already reduced by the offset. The
rows that pass through the receiver's queues are not reduced: the senders
never apply the offset - DistributedPlanner clears it on the sender-side
sort and, when there is a limit, raises that sort's limit to
limit + offset - and GetNextMerging() drops the first offset rows here as
it reads. Both estimates were short by that many rows.
Size them from the rows that reach the queues instead, which is the input
cardinality. When the input estimate is unknown the node's own
cardinality still stands in, so the limit remains the upper bound as
before, and an exchange without an offset gets the number it got before.
card-limit-offset.test now runs with VALIDATE_RESOURCES, which compares
the resource lines its queries already printed. Its merging exchanges
move: 37.76KB to 54.75KB where 7.20K rows arrive for a 1000-row limit,
and 16.00KB to 27.56KB where the offset is above the scan estimate and
730 rows arrive for a cardinality of 1. A third case covers OFFSET
without a LIMIT, where the sender-side sort keeps no limit at all and the
estimate has to cover the whole 7.30K-row input.
Two older gaps in the same estimate are left alone: the queue term
divides the arriving bytes by getNumNodes(), which on an exchange is the
sender count and not the number of receivers, and a limit that applies
per instance is counted once for the whole fragment. Both predate this
change and neither depends on the offset.
Testing:
- PlannerTest#testOffsetCardinality, and the same file with the fix
reverted, to see that the numbers move with it
- Not run: tpcds_cpu_cost/ddl.test and tpcds_tuple_cache/ddl.test, which
also validate resources and carry an offset; no TPC-DS catalog in the
environment used above. By the formula they stay at MIN_ESTIMATE_BYTES
Change-Id: I4ab880549e63267f97b2c193cf76aa92bbfc581c
---
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M
testdata/workloads/functional-planner/queries/PlannerTest/card-limit-offset.test
3 files changed, 69 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/24784/1
--
To view, visit http://gerrit.cloudera.org:8080/24784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4ab880549e63267f97b2c193cf76aa92bbfc581c
Gerrit-Change-Number: 24784
Gerrit-PatchSet: 1
Gerrit-Owner: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
