impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-1575: Part 1: eagerly release query exec resources
Date Fri, 27 Oct 2017 19:37:47 GMT
Dan Hecht has posted comments on this change. ( )

Change subject: IMPALA-1575: Part 1: eagerly release query exec resources

Patch Set 11:

File be/src/runtime/
PS11, Line 359: was not available
> We do call it on the process MemTracker in QueryState::Init().
Okay, but my original question was whether the comment should be reworded for that case:

... if this tracker is part of the query hierarchy.

i.e. "not available" seems misleading now that we don't get this from 'state' (which is the
thing that wasn't always available).
File be/src/runtime/
PS11, Line 95:   if (!released_exec_resources_) ReleaseExecResources();
> My initial concern was that handling it in Init() would get messy in the ca
What I proposed was not to handle it in Init(), but let the caller handle it. i.e. Init()
always gets a resource ref count and QueryExecMgr::StartQuery() has to handle the error. It
already has to handle this and other errors and release ref counts if it won't be passing
them along...
Is there a reason to not just do that?
File be/src/runtime/
PS11, Line 88:   local_query_state_->AcquireExecResourceRefcount(); // Decremented in ReleaseResources().
> The resources used for evaluating exprs, etc - this is used in various test
Why are exvaluating exprs query-wide resources. Aren't they per-thread resources?

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I41ff374b0403f10a145f7fee9b3145953ee32341
Gerrit-Change-Number: 8303
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Joe McDonnell <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Comment-Date: Fri, 27 Oct 2017 19:37:47 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message