Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201711081237.vA8CbJUt007689@ip-10-146-233-104.ec2.internal>
Date: Wed, 8 Nov 2017 12:37:19 +0000
From: "Attila Jeges (Code Review)" <gerrit@cloudera.org>
To: Zoltan Borok-Nagy <boroknagyz@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Tim Armstrong <tarmstrong@cloudera.com>, Michael Ho <kwho@cloudera.com>,
        Laszlo Gaal <laszlo.gaal@cloudera.com>,
        Gabor Kaszab <gaborkaszab@cloudera.com>,
        Csaba Ringhofer <csringhofer@cloudera.com>,
        Thomas Tauber-Marshall <tmarshall@cloudera.com>
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-2248=3A_Make_idle_session_timeout_a_query_option=0A?=
In-Reply-To: <gerrit.1510064019000.I32e2775f80da387b0df4195fe2c5435b3f8e585e@gerrit.cloudera.org>
References: <gerrit.1510064019000.I32e2775f80da387b0df4195fe2c5435b3f8e585e@gerrit.cloudera.org>
Reply-To: attilaj@cloudera.com, impala-cr@cloudera.com,
        boroknagyz@cloudera.com, gaborkaszab@cloudera.com, marcelk@gmail.com,
        tarmstrong@cloudera.com, kwho@cloudera.com, laszlo.gaal@cloudera.com,
        csringhofer@cloudera.com, tmarshall@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="z/ruxdDbJxQ="; charset=UTF-8
archived-at: Wed, 08 Nov 2017 12:37:29 -0000

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

Attila Jeges has posted comments on this change=2E ( http://gerrit=2Ecloude=
ra=2Eorg:8080/8490 )

Change subject: IMPALA-2248: Make idle_session_timeou=
t a query option
=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


Patch Set 3:

(5 comments)

http://gerrit=2Ecloudera=2Eorg:8080/#/c/8=
490/3/be/src/service/client-request-state=2Ecc
File be/src/service/client-r=
equest-state=2Ecc:

http://gerrit=2Ecloudera=2Eorg:8080/#/c/8490/3/be/src/s=
ervice/client-request-state=2Ecc@211
PS3, Line 211: }
             :       =
  else {
single line


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8490/3/be/sr=
c/service/impala-server=2Eh
File be/src/service/impala-server=2Eh:

http://=
gerrit=2Ecloudera=2Eorg:8080/#/c/8490/3/be/src/service/impala-server=2Eh@40=
3
PS3, Line 403: 'timeout'
nit: formal parameter is 'requested_timeout'


h=
ttp://gerrit=2Ecloudera=2Eorg:8080/#/c/8490/3/be/src/service/impala-server=
=2Ecc
File be/src/service/impala-server=2Ecc:

http://gerrit=2Ecloudera=2Eo=
rg:8080/#/c/8490/3/be/src/service/impala-server=2Ecc@1119
PS3, Line 1119:  =
if (session_timeout > 0) {
              :     lock_guard<mutex> l(session_=
timeout_lock_);
              :     multiset<int32_t>::const_iterator itr =
=3D session_timeout_set_=2Efind(session_timeout);
              :     DCHEC=
K(itr !=3D session_timeout_set_=2Eend());
              :     session_timeo=
ut_set_=2Eerase(itr);
              :     session_timeout_cv_=2Enotify_one(=
);
              :   }
Can we call UnregisterSessionTimeout() instead here?=


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8490/3/be/src/service/impala-ser=
ver=2Ecc@1236
PS3, Line 1236:       
nit: 4 character indents on line conti=
nuations


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8490/3/fe/src/test/java/=
org/apache/impala/service/JdbcTest=2Ejava
File fe/src/test/java/org/apache/=
impala/service/JdbcTest=2Ejava:

http://gerrit=2Ecloudera=2Eorg:8080/#/c/84=
90/3/fe/src/test/java/org/apache/impala/service/JdbcTest=2Ejava@657
PS3, Li=
ne 657: connections=2Eget(0)=2Eclose();
Maybe you can be more explicit here=
 about the expected end-state, e=2Eg=2E:

assertNotNull("=2E=2E", connectio=
ns=2Eget(0));
assertNull("=2E=2E", connections=2Eget(1));
assertNull("=2E=
=2E", connections=2Eget(2));


-- 
To view, visit http://gerrit=2Ecloudera=
=2Eorg:8080/8490
To unsubscribe, visit http://gerrit=2Ecloudera=2Eorg:8080/=
settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageTy=
pe: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Ger=
rit-Change-Number: 8490
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy =
<boroknagyz@cloudera=2Ecom>
Gerrit-Reviewer: Attila Jeges <attilaj@cloudera=
=2Ecom>
Gerrit-Reviewer: Csaba Ringhofer <csringhofer@cloudera=2Ecom>
Gerri=
t-Reviewer: Gabor Kaszab <gaborkaszab@cloudera=2Ecom>
Gerrit-Reviewer: Lasz=
lo Gaal <laszlo=2Egaal@cloudera=2Ecom>
Gerrit-Reviewer: Michael Ho <kwho@cl=
oudera=2Ecom>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera=
=2Ecom>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera=2Ecom>
Gerrit-R=
eviewer: Zoltan Borok-Nagy <boroknagyz@cloudera=2Ecom>
Gerrit-Comment-Date:=
 Wed, 08 Nov 2017 12:37:19 +0000
Gerrit-HasComments: Yes

--z/ruxdDbJxQ=--