Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201710272253.v9RMrki4002263@ip-10-146-233-104.ec2.internal>
Date: Fri, 27 Oct 2017 22:53:45 +0000
From: "Tim Armstrong (Code Review)" <gerrit@cloudera.org>
To: Sailesh Mukil <sailesh@cloudera.com>,
        Joe McDonnell <joemcdonnell@cloudera.com>,
        Dan Hecht <dhecht@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,
        sailesh@cloudera.com, marcelk@gmail.com, dhecht@cloudera.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="AraBmqvlyOI="; charset=UTF-8
archived-at: Fri, 27 Oct 2017 22:53:52 -0000

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

Hello Sailesh Mukil, Joe McDonnell, Dan Hecht, 

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

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

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

Change subject: IMPALA-1575: Part 1: ea=
gerly release query exec resources
=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

IMPALA-1575: Part 1: eagerly release query exec reso=
urces

Release of backend resources for query execution on the coordinator
=
daemon should occur eagerly as soon as query execution is finished
or cance=
lled=2E Before this patch some resources managed by QueryState,
like scratc=
h files, were only released when the query was closed
and all of the query'=
s control structures were torn down=2E

These resources are referred to as =
"ExecResources" in various places to
distinguish them from resources associ=
ated with the client request (like
the result cache) that are still require=
d after the query finishes
executing=2E

This first change does not solve t=
he admission control problem for two
reasons:
* We don't release the "admit=
ted" memory on the coordinator until
  the query is unregistered=2E
* Admis=
sion control still considers the memory reserved until the
  query memtrack=
er is unregistered, which happens only when the
  QueryState is destroyed: =
see MemTracker::GetPoolMemReserved()=2E

The flow is mostly similar to init=
ial_reservation_refcnt_, except the
coordinator also holds onto a reference=
 count, which is released
when either the final row is returned or cancella=
tion is initiated=2E
After the coordinator releases its refcount, the resou=
rces can be
freed as soon as local fragments also release their refcounts=
=2E

Also clean up Coordinator slightly by preventing runtime_state() from
=
leaking out to ClientRequestState - instead it's possible to log
the query =
MemTracker by following parent links in the MemTracker=2E

This patch is pa=
rtially based on Joe McDonnell's IMPALA-1575 patch=2E

Testing:
Ran core te=
sts=2E

Change-Id: I41ff374b0403f10a145f7fee9b3145953ee32341
---
M be/src/r=
untime/coordinator=2Ecc
M be/src/runtime/coordinator=2Eh
M be/src/runtime/m=
em-tracker=2Ecc
M be/src/runtime/mem-tracker=2Eh
M be/src/runtime/query-exe=
c-mgr=2Ecc
M be/src/runtime/query-state=2Ecc
M be/src/runtime/query-state=
=2Eh
M be/src/runtime/runtime-state=2Ecc
M be/src/runtime/test-env=2Ecc
M b=
e/src/service/client-request-state=2Ecc
10 files changed, 140 insertions(+)=
, 89 deletions(-)


  git pull ssh://gerrit=2Ecloudera=2Eorg:29418/Impala-A=
SF refs/changes/03/8303/15
-- 
To view, visit http://gerrit=2Ecloudera=2Eor=
g:8080/8303
To unsubscribe, visit http://gerrit=2Ecloudera=2Eorg:8080/setti=
ngs

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: n=
ewpatchset
Gerrit-Change-Id: I41ff374b0403f10a145f7fee9b3145953ee32341
Gerr=
it-Change-Number: 8303
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong <tar=
mstrong@cloudera=2Ecom>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera=2Ecom>
=
Gerrit-Reviewer: Joe McDonnell <joemcdonnell@cloudera=2Ecom>
Gerrit-Reviewe=
r: Sailesh Mukil <sailesh@cloudera=2Ecom>
Gerrit-Reviewer: Tim Armstrong <t=
armstrong@cloudera=2Ecom>

--AraBmqvlyOI=--