You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2017/08/21 14:55:39 UTC
[2/3] incubator-impala git commit: IMPALA-5108: idle_session_timeout
kicks in later than expected
IMPALA-5108: idle_session_timeout kicks in later than expected
Fix: The issue was caused because sessions could get closed
some time after their timeout expires, because the session
expiration thread only woke up every session-timeout/2
seconds. This fix changes the logic, now the session expiration
thread will wakeup every 1 sec to check whether sessions can
be expired.
The test changes in session-expiry-test is based on introducing
variable names for the numeric constants and to reduce the time
of max_idle_timeout_ms.
Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Reviewed-on: http://gerrit.cloudera.org:8080/7709
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/3b5c460a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/3b5c460a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/3b5c460a
Branch: refs/heads/master
Commit: 3b5c460ac27e81f1169b5babe0b28e664bdd237e
Parents: 3e77040
Author: Pranay <ps...@cloudera.com>
Authored: Thu Aug 17 11:15:55 2017 -0700
Committer: Henry Robinson <he...@cloudera.com>
Committed: Sun Aug 20 22:23:43 2017 +0000
----------------------------------------------------------------------
be/src/service/impala-server.cc | 7 +++----
be/src/service/session-expiry-test.cc | 10 ++++++----
2 files changed, 9 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/3b5c460a/be/src/service/impala-server.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index e173c84..b440845 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -1814,10 +1814,9 @@ void ImpalaServer::RegisterSessionTimeout(int32_t session_timeout) {
if (session_timeout_set_.empty()) {
session_timeout_cv_.wait(timeout_lock);
} else {
- // Sleep for half the minimum session timeout; the maximum delay between a session
- // expiring and this method picking it up is equal to the size of this sleep.
- int64_t session_timeout_min_ms = *session_timeout_set_.begin() * 1000 / 2;
- system_time deadline = get_system_time() + milliseconds(session_timeout_min_ms);
+ // Sleep for a second before checking whether an active session can be expired.
+ const int64_t SLEEP_TIME_MS = 1000;
+ system_time deadline = get_system_time() + milliseconds(SLEEP_TIME_MS);
session_timeout_cv_.timed_wait(timeout_lock, deadline);
}
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/3b5c460a/be/src/service/session-expiry-test.cc
----------------------------------------------------------------------
diff --git a/be/src/service/session-expiry-test.cc b/be/src/service/session-expiry-test.cc
index 81271a4..a0a493c 100644
--- a/be/src/service/session-expiry-test.cc
+++ b/be/src/service/session-expiry-test.cc
@@ -42,6 +42,8 @@ DECLARE_int32(beeswax_port);
// that doesn't depend upon being rescheduled in a timely fashion.
TEST(SessionTest, TestExpiry) {
+ const int NUM_CLIENTS = 5;
+ const int MAX_IDLE_TIMEOUT_MS = 4000;
FLAGS_idle_session_timeout = 1;
InProcessStatestore* ips = InProcessStatestore::StartWithEphemeralPorts();
InProcessImpalaServer* impala =
@@ -60,11 +62,11 @@ TEST(SessionTest, TestExpiry) {
EXPECT_EQ(beeswax_session_metric->value(), 0L);
{
- scoped_ptr<ThriftClient<ImpalaServiceClient>> beeswax_clients[5];
- scoped_ptr<ThriftClient<ImpalaHiveServer2ServiceClient>> hs2_clients[5];
+ scoped_ptr<ThriftClient<ImpalaServiceClient>> beeswax_clients[NUM_CLIENTS];
+ scoped_ptr<ThriftClient<ImpalaHiveServer2ServiceClient>> hs2_clients[NUM_CLIENTS];
// Create five Beeswax clients and five HS2 clients (each HS2 gets one session each)
- for (int i = 0; i < 5; ++i) {
+ for (int i = 0; i < NUM_CLIENTS; ++i) {
beeswax_clients[i].reset(new ThriftClient<ImpalaServiceClient>(
"localhost", impala->beeswax_port()));
EXPECT_OK(beeswax_clients[i]->Open());
@@ -78,7 +80,7 @@ TEST(SessionTest, TestExpiry) {
}
int64_t start = UnixMillis();
- while (expired_metric->value() != 10 && UnixMillis() - start < 5000) {
+ while (expired_metric->value() != 10 && UnixMillis() - start < MAX_IDLE_TIMEOUT_MS) {
SleepForMs(100);
}