Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201710280245.v9S2jZxc006373@ip-10-146-233-104.ec2.internal>
Date: Sat, 28 Oct 2017 02:45:35 +0000
From: "Impala Public Jenkins (Code Review)" <gerrit@cloudera.org>
To: Tim Armstrong <tarmstrong@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: impala-cr@cloudera.com, tarmstrong@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="F7pEODJsMFs="; charset=UTF-8
archived-at: Sat, 28 Oct 2017 02:45:41 -0000

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

Impala Public Jenkins has submitted this change and it was merged=2E ( http=
://gerrit=2Ecloudera=2Eorg:8080/8303 )

Change subject: IMPALA-1575: Part 1=
: eagerly 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 r=
esources

Release of backend resources for query execution on the coordinat=
or
daemon should occur eagerly as soon as query execution is finished
or ca=
ncelled=2E Before this patch some resources managed by QueryState,
like scr=
atch files, were only released when the query was closed
and all of the que=
ry's control structures were torn down=2E

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

This first change does not solv=
e the admission control problem for two
reasons:
* We don't release the "ad=
mitted" memory on the coordinator until
  the query is unregistered=2E
* Ad=
mission control still considers the memory reserved until the
  query memtr=
acker is unregistered, which happens only when the
  QueryState is destroye=
d: see MemTracker::GetPoolMemReserved()=2E

The flow is mostly similar to i=
nitial_reservation_refcnt_, except the
coordinator also holds onto a refere=
nce count, which is released
when either the final row is returned or cance=
llation is initiated=2E
After the coordinator releases its refcount, the re=
sources can be
freed as soon as local fragments also release their refcount=
s=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 p=
artially based on Joe McDonnell's IMPALA-1575 patch=2E

Testing:
Ran core t=
ests=2E

Change-Id: I41ff374b0403f10a145f7fee9b3145953ee32341
Reviewed-on: =
http://gerrit=2Ecloudera=2Eorg:8080/8303
Reviewed-by: Tim Armstrong <tarmst=
rong@cloudera=2Ecom>
Tested-by: Impala Public Jenkins
---
M be/src/runtime/=
coordinator=2Ecc
M be/src/runtime/coordinator=2Eh
M be/src/runtime/mem-trac=
ker=2Ecc
M be/src/runtime/mem-tracker=2Eh
M be/src/runtime/query-exec-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 be/src/s=
ervice/client-request-state=2Ecc
10 files changed, 140 insertions(+), 89 de=
letions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved
  Impala=
 Public Jenkins: Verified

-- 
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: m=
erged
Gerrit-Change-Id: I41ff374b0403f10a145f7fee9b3145953ee32341
Gerrit-Ch=
ange-Number: 8303
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong <tarmstro=
ng@cloudera=2Ecom>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera=2Ecom>
Gerri=
t-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <joemcdonn=
ell@cloudera=2Ecom>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera=2Ecom>=

Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera=2Ecom>

--F7pEODJsMFs=--