[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-10 Thread Zoltan Borok-Nagy (Code Review) 
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8490 )

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8490/3/fe/src/test/java/org/apache/impala/service/JdbcTest.java
File fe/src/test/java/org/apache/impala/service/JdbcTest.java:

http://gerrit.cloudera.org:8080/#/c/8490/3/fe/src/test/java/org/apache/impala/service/JdbcTest.java@657
PS3, Line 657: }
> Maybe you can be more explicit here about the expected end-state, e.g.:
Done



--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Fri, 10 Nov 2017 15:58:30 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-10 Thread Zoltan Borok-Nagy (Code Review) 
Hello Michael Ho, Thomas Tauber-Marshall, Laszlo Gaal, Gabor Kaszab, Attila 
Jeges, Tim Armstrong, Csaba Ringhofer,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8490

to look at the new patch set (#6).

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..

IMPALA-2248: Make idle_session_timeout a query option

This commit makes idle_session_timeout a query option.
idle_session_timeout currently can be set as a command line
option, which will be the default timeout for sessions.
HS2 sessions can override it with a smaller value by setting
it in the configuration overlay of HS2 OpenSession().

However, we can't override idle_session_timeout for JDBC/ODBC
connections, because we cannot put this in the connection string.

This commit is a workaround for this problem, it allows JDBC/ODBC
connections to set the session timeout as a query option
with the SET statement.

I created an automated test case in JdbcTest.java based on
test_hs2.py::test_concurrent_session_mixed_idle_timeout.

Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
A fe/src/test/java/org/apache/impala/util/Metrics.java
11 files changed, 262 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/8490/6
--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-08 Thread Zoltan Borok-Nagy (Code Review) 
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8490 )

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc@211
PS3, Line 211:   SetResultSet({}, {});
 : }
> single line
Done


http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/impala-server.cc@1119
PS3, Line 1119:discard_result(UnregisterQuery(query_id, false, ));
  :   }
  :   // Reconfigure the poll period of session_timeout_thread_ if 
necessary.
  :   UnregisterSessionTimeout(session_state->session_timeout);
  :   return Status::OK();
  : }
  : 
> Can we call UnregisterSessionTimeout() instead here?
Nice catch! Yes, and we should! Done.


http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/impala-server.cc@1236
PS3, Line 1236:
> nit: 4 character indents on line continuations
Done



--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 08 Nov 2017 13:25:05 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-08 Thread Zoltan Borok-Nagy (Code Review) 
Hello Michael Ho, Thomas Tauber-Marshall, Laszlo Gaal, Gabor Kaszab, Attila 
Jeges, Tim Armstrong, Csaba Ringhofer,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8490

to look at the new patch set (#5).

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..

IMPALA-2248: Make idle_session_timeout a query option

This commit makes idle_session_timeout a query option.
idle_session_timeout currently can be set as a command line
option, which will be the default timeout for sessions.
HS2 sessions can override it with a smaller value by setting
it in the configuration overlay of HS2 OpenSession().

However, we can't override idle_session_timeout for JDBC/ODBC
connections, because we cannot put this in the connection string.

This commit is a workaround for this problem, it allows JDBC/ODBC
connections to set the session timeout as a query option
with the SET statement.

I created an automated test case in JdbcTest.java based on
test_hs2.py::test_concurrent_session_mixed_idle_timeout.

Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
A fe/src/test/java/org/apache/impala/util/Metrics.java
11 files changed, 256 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/8490/5
--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-08 Thread Attila Jeges (Code Review) 
Attila Jeges has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8490 )

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc@211
PS3, Line 211: }
 : else {
single line


http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/impala-server.h@403
PS3, Line 403: 'timeout'
nit: formal parameter is 'requested_timeout'


http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/impala-server.cc@1119
PS3, Line 1119:  if (session_timeout > 0) {
  : lock_guard l(session_timeout_lock_);
  : multiset::const_iterator itr = 
session_timeout_set_.find(session_timeout);
  : DCHECK(itr != session_timeout_set_.end());
  : session_timeout_set_.erase(itr);
  : session_timeout_cv_.notify_one();
  :   }
Can we call UnregisterSessionTimeout() instead here?


http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/impala-server.cc@1236
PS3, Line 1236:
nit: 4 character indents on line continuations


http://gerrit.cloudera.org:8080/#/c/8490/3/fe/src/test/java/org/apache/impala/service/JdbcTest.java
File fe/src/test/java/org/apache/impala/service/JdbcTest.java:

http://gerrit.cloudera.org:8080/#/c/8490/3/fe/src/test/java/org/apache/impala/service/JdbcTest.java@657
PS3, Line 657: connections.get(0).close();
Maybe you can be more explicit here about the expected end-state, e.g.:

assertNotNull("..", connections.get(0));
assertNull("..", connections.get(1));
assertNull("..", connections.get(2));



--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 08 Nov 2017 12:37:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-08 Thread Zoltan Borok-Nagy (Code Review) 
Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8490 )

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..


Patch Set 4:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc@204
PS3, Line 204: if (iequals(key, "idle_session_timeout")) {
> Its unfortunate this is special cased here. Could you add a comment explain
Done


http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc@205
PS3, Line 205:   // IMPALA-2248: For some client, we need to enable to 
set
> Could you extract the body of the 'if' to a new function in order to keep t
Done


http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc@213
PS3, Line 213:
 :   } else {
> single line
Done


http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/impala-server.cc@189
PS3, Line 189: DEFINE_int32(idle_session_timeout, 0, "The time, in seconds, 
that a session may be idle"
> Note how this interacts with the query option.
Done


http://gerrit.cloudera.org:8080/#/c/8490/3/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:

http://gerrit.cloudera.org:8080/#/c/8490/3/common/thrift/ImpalaService.thrift@292
PS3, Line 292:   // The time, in seconds, that a session may be idle for before 
it is closed (and all
> Note how this interacts with the flag.
Done


http://gerrit.cloudera.org:8080/#/c/8490/3/fe/src/test/java/org/apache/impala/service/JdbcTest.java
File fe/src/test/java/org/apache/impala/service/JdbcTest.java:

http://gerrit.cloudera.org:8080/#/c/8490/3/fe/src/test/java/org/apache/impala/service/JdbcTest.java@594
PS3, Line 594:
> "0, 5, 10"'s and "timeout * 1.5"'s relation could be more explicit, for exa
Partly done.
I don't feel that the asList part is necessary, but I introduced a 
timeoutTolerance variable.


http://gerrit.cloudera.org:8080/#/c/8490/3/fe/src/test/java/org/apache/impala/util/Metrics.java
File fe/src/test/java/org/apache/impala/util/Metrics.java:

http://gerrit.cloudera.org:8080/#/c/8490/3/fe/src/test/java/org/apache/impala/util/Metrics.java@26
PS3, Line 26: /**
> Could you please write a short comment to describe the purpose of this clas
Done



--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 08 Nov 2017 11:49:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-08 Thread Zoltan Borok-Nagy (Code Review) 
Hello Michael Ho, Thomas Tauber-Marshall, Laszlo Gaal, Gabor Kaszab, Attila 
Jeges, Tim Armstrong, Csaba Ringhofer,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8490

to look at the new patch set (#4).

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..

IMPALA-2248: Make idle_session_timeout a query option

This commit makes idle_session_timeout a query option.
idle_session_timeout currently can be set as a command line
option, which will be the default timeout for sessions.
HS2 sessions can override it with a smaller value by setting
it in the configuration overlay of HS2 OpenSession().

However, we can't override idle_session_timeout for JDBC/ODBC
connections, because we cannot put this in the connection string.

This commit is a workaround for this problem, it allows JDBC/ODBC
connections to set the session timeout as a query option
with the SET statement.

I created an automated test case in JdbcTest.java based on
test_hs2.py::test_concurrent_session_mixed_idle_timeout.

Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
A fe/src/test/java/org/apache/impala/util/Metrics.java
11 files changed, 253 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/8490/4
--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-07 Thread Csaba Ringhofer (Code Review) 
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8490 )

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8490/3/fe/src/test/java/org/apache/impala/service/JdbcTest.java
File fe/src/test/java/org/apache/impala/service/JdbcTest.java:

http://gerrit.cloudera.org:8080/#/c/8490/3/fe/src/test/java/org/apache/impala/service/JdbcTest.java@594
PS3, Line 594:
"0, 5, 10"'s and "timeout * 1.5"'s relation could be more explicit, for example:
float timeout_tolerance = 0.5;
... Arrays.asList(0, 5, 5*(1 + 2*timeout_tolerance))
...
int sleepPeriod = (int)(timeout * (1 + timeout_tolerance));



--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 08 Nov 2017 00:21:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-07 Thread Gabor Kaszab (Code Review) 
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8490 )

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc@205
PS3, Line 205:   int32_t requested_timeout = atoi(value.c_str());
Could you extract the body of the 'if' to a new function in order to keep this 
part more verbose?


http://gerrit.cloudera.org:8080/#/c/8490/3/fe/src/test/java/org/apache/impala/util/Metrics.java
File fe/src/test/java/org/apache/impala/util/Metrics.java:

http://gerrit.cloudera.org:8080/#/c/8490/3/fe/src/test/java/org/apache/impala/util/Metrics.java@26
PS3, Line 26: public class Metrics {
Could you please write a short comment to describe the purpose of this class?



--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 07 Nov 2017 18:36:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-07 Thread Thomas Tauber-Marshall (Code Review) 
Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8490 )

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc@204
PS3, Line 204: if (iequals(key, "idle_session_timeout")) {
Its unfortunate this is special cased here. Could you add a comment explaining 
why that's necessary.


http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/client-request-state.cc@213
PS3, Line 213: key, value,
 :   _->set_query_options,
single line


http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/8490/3/be/src/service/impala-server.cc@189
PS3, Line 189: DEFINE_int32(idle_session_timeout, 0, "The time, in seconds, 
that a session may be idle"
Note how this interacts with the query option.


http://gerrit.cloudera.org:8080/#/c/8490/3/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:

http://gerrit.cloudera.org:8080/#/c/8490/3/common/thrift/ImpalaService.thrift@292
PS3, Line 292:   // The time, in seconds, that a session may be idle for before 
it is closed (and all
Note how this interacts with the flag.



--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 07 Nov 2017 17:03:55 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-07 Thread Zoltan Borok-Nagy (Code Review) 
Hello Michael Ho, Laszlo Gaal, Gabor Kaszab, Attila Jeges, Tim Armstrong, Csaba 
Ringhofer,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8490

to look at the new patch set (#3).

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..

IMPALA-2248: Make idle_session_timeout a query option

This commit makes idle_session_timeout a query option.
idle_session_timeout currently can be set as a command line
option, which will be the default timeout for sessions.
HS2 sessions can override it with a smaller value by setting
it in the configuration overlay of HS2 OpenSession().

However, we can't override idle_session_timeout for JDBC/ODBC
connections, because we cannot put this in the connection string.

This commit is a workaround for this problem, it allows JDBC/ODBC
connections to set the session timeout as a query option
with the SET statement.

I created an automated test case in JdbcTest.java based on
test_hs2.py::test_concurrent_session_mixed_idle_timeout.

Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
---
M be/src/service/client-request-state.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
A fe/src/test/java/org/apache/impala/util/Metrics.java
10 files changed, 236 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/8490/3
--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-07 Thread Zoltan Borok-Nagy (Code Review) 
Zoltan Borok-Nagy has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/8490 )

Change subject: IMPALA-2248: Make idle_session_timeout a query option
..

IMPALA-2248: Make idle_session_timeout a query option

This commit makes idle_session_timeout a query option.
idle_session_timeout currently can be set as a command line
option, which will be the default timeout for sessions.
HS2 sessions can override it with a smaller value by setting
it in the configuration overlay of HS2 OpenSession().

However, we can't override idle_session_timeout for JDBC/ODBC
connections, because we cannot put this in the connection string.

This commit is a workaround for this problem, it allows JDBC/ODBC
connections to set the session timeout as a query option
with the SET statement.

I created an automated test case in JdbcTest.java.

Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
---
M be/src/service/client-request-state.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
A fe/src/test/java/org/apache/impala/util/Metrics.java
10 files changed, 236 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/8490/2
--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy

[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option

2017-11-07 Thread Zoltan Borok-Nagy (Code Review) 
Zoltan Borok-Nagy has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8490


Change subject: IMPALA-2248: Make idle_session_timeout a query option
..

IMPALA-2248: Make idle_session_timeout a query option

This commit makes idle_session_timeout a query option.
idle_session_timeout currently can be set as a command line
option, which will be the default timeout for sessions.
HS2 sessions can override it with a smaller value by setting
it in the configuration overlay of HS2 OpenSession().

However, we can't override idle_session_timeout for JDBC/ODBC
connections, because we cannot put this in the connection string.

This commit is a workaround for this problem, it allows JDBC/ODBC
connections to set the session timeout as a query option
with the SET statement.

I created an automated test case in JdbcTest.java.

Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
---
M be/src/service/client-request-state.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
A fe/src/test/java/org/apache/impala/util/Metrics.java
10 files changed, 238 insertions(+), 23 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/8490/1
--
To view, visit http://gerrit.cloudera.org:8080/8490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Gerrit-Change-Number: 8490
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy