impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4674: Part 2: port backend exec to BufferPool
Date Thu, 13 Jul 2017 20:59:20 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4674: Part 2: port backend exec to BufferPool

Patch Set 29:

File be/src/exec/

PS30, Line 1246: 
> typo
File be/src/runtime/

Line 366:             << PrettyPrinter::Print(bytes_limit, TUnit::BYTES);
> maybe log bufffer_pool_capacity as well? or do we do that already somewhere
File be/src/runtime/

PS26, Line 149:   bytes_limit = query_options().mem_limit;
> How about we file a jira for this and consider it for a follow on change? o
Filed IMPALA-5661.

I agree that the process-wide buffer pool limit kicks in automatically so we don't need to
add a query-local buffer pool limit if there's no query mem_limit.

Restructured the code to exploit that fact and be less confusing.
File tests/query_test/

Line 126:                        'Q21' : 187, 'Q22' : 125}
> Is that because we have to assume all fragment executions overlap, or pessi
I think the fragment overlapping is the main problem. Mostly within a single fragment I'd
expect it to be pretty accurate, since in the old code the small buffers get allocated on
Open() around the same time as the reservation is acquired.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I7fc7fe1c04e9dfb1a0c749fb56a5e0f2bf9c6c3e
Gerrit-PatchSet: 29
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message