impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <>
Subject [Impala-CR](cdh5-2.5.0_5.7.0) IMPALA-3085: Unregister data sinks' MemTrackers at their Close() functions.
Date Fri, 26 Feb 2016 07:33:26 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-3085: Unregister data sinks' MemTrackers at their Close() functions.

Patch Set 3: Code-Review+2

(1 comment)

 > Yes, I had the same question. It seems we only set auto_unregister_
 > to true for the query wide MemTracker and request pool MemTracker
 > but not for other MemTrackers. My impression is that it was
 > expecting the callers to clean up properly although we should
 > revisit whether it makes sense to have it as the default behavior
 > after this release.

We should only do memory management in the destructor (i.e. free memory owned by this object).
 In other words, let's restrict destructors to be our "garbage collector" but any control
logic should be done at an explicit point during the query lifecycle (such as Close()).  

So, I don't think we should increase usage of "auto_unregister" (instead we should get rid
of it).
File be/src/exec/

Line 310:   mem_pool_->FreeAll();
hmm looks like the mem_pool_ itself is never deleted also (other writers have this as a scoped_ptr).

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I3aec82150a933dc2b261beff41f5f4f022501bfb
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.5.0_5.7.0
Gerrit-Owner: Michael Ho <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Huaisi Xu <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Skye Wanderman-Milne <>
Gerrit-HasComments: Yes

View raw message