Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201710181613.v9IGDiu2030387@ip-10-146-233-104.ec2.internal>
Date: Wed, 18 Oct 2017 16:13:44 +0000
From: "Tim Armstrong (Code Review)" <gerrit@cloudera.org>
To: Joe McDonnell <joemcdonnell@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-1575=3A_Part_1=3A_eagerly_release_query_exec_resources=0A?=
In-Reply-To: <gerrit.1508272023000.I41ff374b0403f10a145f7fee9b3145953ee32341@gerrit.cloudera.org>
References: <gerrit.1508272023000.I41ff374b0403f10a145f7fee9b3145953ee32341@gerrit.cloudera.org>
Reply-To: tarmstrong@cloudera.com, impala-cr@cloudera.com, marcelk@gmail.com,
        joemcdonnell@cloudera.com, reviews@impala.incubator.apache.org
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.14.2
Content-Type: multipart/alternative; boundary="k7xFYCw8ubI="; charset=UTF-8
archived-at: Wed, 18 Oct 2017 16:13:52 -0000

--k7xFYCw8ubI=
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hello Joe McDonnell, 

I'd like you to reexamine a change=2E Please visit

=
    http://gerrit=2Ecloudera=2Eorg:8080/8303

to look at the new patch set =
(#9)=2E

Change subject: IMPALA-1575: Part 1: eagerly release query exec re=
sources
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E

IMPA=
LA-1575: Part 1: eagerly release query exec resources

Release of backend r=
esources for query execution on the coordinator
daemon should occur eagerly=
 as soon as query execution is finished
or cancelled=2E Before this patch s=
ome resources managed by QueryState,
like scratch files, were only released=
 when the query was closed
and all of the query's control structures were t=
orn down=2E

These resources are referred to as "ExecResources" in various =
places to
distinguish them from resources associated with the client reques=
t (like
the result cache) that are still required after the query finishes
=
executing=2E

This first change does not solve the admission control proble=
m for two
reasons:
* We don't release the "admitted" memory on the coordina=
tor until
  the query is unregistered=2E
* Admission control still consider=
s the memory reserved until the
  query memtracker is unregistered, which h=
appens only when the
  QueryState is destroyed: see MemTracker::GetPoolMemR=
eserved()=2E

The flow is mostly similar to initial_reservation_refcnt_, ex=
cept the
coordinator also holds onto a reference count, which is released
w=
hen either the final row is returned or cancellation is initiated=2E
After =
the coordinator releases its refcount, the resources can be
freed as soon a=
s local fragments also release their refcounts=2E

Also clean up Coordinato=
r slightly by preventing runtime_state() from
leaking out to ClientRequestS=
tate - instead it's possible to log
the query MemTracker by following paren=
t links in the MemTracker=2E

This patch is partially based on Joe McDonnel=
l's IMPALA-1575 patch=2E

Testing:
Ran core tests=2E

Change-Id: I41ff374b0=
403f10a145f7fee9b3145953ee32341
---
M be/src/runtime/coordinator=2Ecc
M be/=
src/runtime/coordinator=2Eh
M be/src/runtime/mem-tracker=2Ecc
M be/src/runt=
ime/mem-tracker=2Eh
M be/src/runtime/query-exec-mgr=2Ecc
M be/src/runtime/q=
uery-state=2Ecc
M be/src/runtime/query-state=2Eh
M be/src/runtime/runtime-s=
tate=2Ecc
M be/src/runtime/test-env=2Ecc
M be/src/service/client-request-st=
ate=2Ecc
10 files changed, 122 insertions(+), 85 deletions(-)


  git pull =
ssh://gerrit=2Ecloudera=2Eorg:29418/Impala-ASF refs/changes/03/8303/9
-- 
T=
o view, visit http://gerrit=2Ecloudera=2Eorg:8080/8303
To unsubscribe, visi=
t http://gerrit=2Ecloudera=2Eorg:8080/settings

Gerrit-Project: Impala-ASF
=
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I41=
ff374b0403f10a145f7fee9b3145953ee32341
Gerrit-Change-Number: 8303
Gerrit-Pa=
tchSet: 9
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera=2Ecom>
Gerrit-Re=
viewer: Joe McDonnell <joemcdonnell@cloudera=2Ecom>

--k7xFYCw8ubI=--