Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16240 )

Change subject: IMPALA-9955,IMPALA-9957: Fix not enough reservation for large 
pages in GroupingAggregator
......................................................................


Patch Set 7:

(2 comments)

Hi Tim, this patch is ready to review and it's passed the core tests.

However, I'm not sure if other spillable operators have the same issue. If so, 
maybe we need a deeper fix inside the BufferedTupleStream to fix the issues in 
all operators. Also not sure if the tests have covered all the branches added 
in this patch. So I'll still play around with this patch and try to add more 
tests.

http://gerrit.cloudera.org:8080/#/c/16240/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16240/5//COMMIT_MSG@17
PS5, Line 17: To be specific, for how to manage the large write page 
reservation,
> I'll need to look in more detail but I think the overal approach makes sens
Revised the commit message to make this more specific.


http://gerrit.cloudera.org:8080/#/c/16240/5//COMMIT_MSG@35
PS5, Line 35: needs for spilling.
> Maybe I missed something when I initially did this, but I didn't think we n
Oh, I see. So this is not a bug. I need this just because I save the extra lage 
page reservation at the end of GroupingAggregator::Open(), so the hash table 
could occupy some extra space. Actually, we can save them before creating the 
hash tables. Then we don't need this and don't need to change the FE test files 
which complicate backporting this patch.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3d9c3a2e7f0da60071b920dec979729e86459775
Gerrit-Change-Number: 16240
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Wed, 05 Aug 2020 13:05:36 +0000
Gerrit-HasComments: Yes

Reply via email to