[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has abandoned this change.

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..


Abandoned

This change was merged so I need to abandon the change I'll address the review 
comments as a part of another change/commit#

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I827a5fa2246e7b05b718fb8b4563b8488c91a1d9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has abandoned this change.

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..


Abandoned

The previous commit was merged so I have to abandon this change

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I72625dcc72764c1b89eb4ecb8725c51c9ca7835b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has posted comments on this change.

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7791/1//COMMIT_MSG
Commit Message:

PS1, Line 9: 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
> given that the original change was already merged (I wasn't aware of that s
I had reduced MAX_IDLE_TIMEOUT_MS from 5 sec to 4 sec because the session 
timeout is 1 sec for all the five sessions, so the delay of 5 sec appeared to 
be a longer delay, waiting for sessions to expire. Since the expiration thread 
is checking for the sessions to be expired every sec.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I72625dcc72764c1b89eb4ecb8725c51c9ca7835b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change.

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7791/1//COMMIT_MSG
Commit Message:

PS1, Line 9: 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
given that the original change was already merged (I wasn't aware of that since 
it showed up as two separate reviews), this text doesn't apply to this 
particular change. Just say that this is a follow up cleanup change.

I'm still curious as to the reason the MAX_IDLE_TIMEOUT_MS was reduced from 5 
to 4 seconds.


http://gerrit.cloudera.org:8080/#/c/7791/1/be/src/service/session-expiry-test.cc
File be/src/service/session-expiry-test.cc:

PS1, Line 88:  
nit: nuke that trailing whitespace


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I72625dcc72764c1b89eb4ecb8725c51c9ca7835b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change.

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..


Patch Set 1:

> The previous commit was merged - it will need a new change ID.

Oh weird, now I see 7709, so this is the third.  Pranay, please fix your 
workflow as to not generate new change-ids when you upload a new patch for the 
same change.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I72625dcc72764c1b89eb4ecb8725c51c9ca7835b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..


Patch Set 1:

The previous commit was merged - it will need a new change ID.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I72625dcc72764c1b89eb4ecb8725c51c9ca7835b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has uploaded a new change for review.

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

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..

IMPALA-5108: idle_session_timeout kicks in later than expected

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: I72625dcc72764c1b89eb4ecb8725c51c9ca7835b
---
M be/src/service/impala-server.cc
M be/src/service/session-expiry-test.cc
2 files changed, 11 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I72625dcc72764c1b89eb4ecb8725c51c9ca7835b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has posted comments on this change.

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7729/1//COMMIT_MSG
Commit Message:

PS1, Line 17: reduce
> what's the reason to reduce this time?
Since the frequency of polling is session-timeout/2  where session-timeout is 
1sec, 4 sec should be good enough a tight upper bound so I used 4 sec as idle 
session timeout


Line 18: of max_idle_timeout_ms
> let's generally keep the messages left justified (exception inside formatte
OK


http://gerrit.cloudera.org:8080/#/c/7729/1/be/src/service/session-expiry-test.cc
File be/src/service/session-expiry-test.cc:

PS1, Line 68: gets one session eac
> since we're really trying to create multiple sessions (and just using clien
Done


PS1, Line 83: 10
> that looks like it should be expressed in terms of NUM_CLIENTS.
Done


PS1, Line 87: 5s
> 5s should be updated if you're changing that value.
Done


PS1, Line 88: 5L
> isn't that NUM_CLIENTS?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I827a5fa2246e7b05b718fb8b4563b8488c91a1d9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change.

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..


Patch Set 1:

(8 comments)

Great to see this one get fixed.

http://gerrit.cloudera.org:8080/#/c/7729/1//COMMIT_MSG
Commit Message:

PS1, Line 17: reduce
what's the reason to reduce this time?


Line 18: of max_idle_timeout_ms
let's generally keep the messages left justified (exception inside formatted 
structures e.g. bulleted lists).


http://gerrit.cloudera.org:8080/#/c/7729/1/be/src/service/session-expiry-test.cc
File be/src/service/session-expiry-test.cc:

PS1, Line 68: gets one session eac
since we're really trying to create multiple sessions (and just using clients 
to do that) maybe NUM_CLIENTS should be called NUM_SESSIONS?


PS1, Line 83: 10
that looks like it should be expressed in terms of NUM_CLIENTS.


PS1, Line 87: 5s
5s should be updated if you're changing that value.


PS1, Line 87: 10L
same


PS1, Line 88: 5L
isn't that NUM_CLIENTS?


PS1, Line 90: 5L
same


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I827a5fa2246e7b05b718fb8b4563b8488c91a1d9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dan Hecht 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5108: idle_session_timeout kicks in later than expected
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1098/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has uploaded a new patch set (#3).

Change subject: 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
---
M be/src/service/impala-server.cc
M be/src/service/session-expiry-test.cc
2 files changed, 9 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has uploaded a new change for review.

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

Change subject: 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: I827a5fa2246e7b05b718fb8b4563b8488c91a1d9
---
M be/src/service/impala-server.cc
M be/src/service/session-expiry-test.cc
2 files changed, 9 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I827a5fa2246e7b05b718fb8b4563b8488c91a1d9
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh

[Impala-ASF-CR] IMPALA-5108: idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Hello Henry Robinson,

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

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

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

Change subject: 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
---
M be/src/service/impala-server.cc
M be/src/service/session-expiry-test.cc
2 files changed, 9 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Patch Set 2: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7709/2//COMMIT_MSG
Commit Message:

Line 7: IMPALA-5108:idle_session_timeout kicks in later than expected
nit: put a space after ':'


PS2, Line 11: ~1 min
that's only a default - would be clearer to the reader to say "Sessions could 
get closed some time after their timeout expires, because the session 
expiration thread only woke up every session-timeout/2 seconds."


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

PS2, Line 1817: // Sleep for a second before checking whether an active session
  : // can be expired or not.
nit: can you wrap this to 90 chars?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Henry Robinson (Code Review)]

Henry Robinson has abandoned this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Abandoned

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has posted comments on this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7712/1//COMMIT_MSG
Commit Message:

PS1, Line 20: Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
> please remove this line and resubmit - Gerrit uses this as its unique ID fo
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has uploaded a new patch set (#2).

Change subject: 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 polling frequency to check for
the session to be expired was half of session timeout, which is
around ~1 min. The polling frequency has been increased, it's changed
to be around 1 sec with this change.

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
---
M be/src/service/impala-server.cc
M be/src/service/session-expiry-test.cc
2 files changed, 10 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has uploaded a new patch set (#2).

Change subject: 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 polling frequency to check for
the session to be expired was half of session timeout, which is
around ~1 min. The polling frequency has been increased, it's changed
to be around 1 sec with this change.

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
---
M be/src/service/impala-server.cc
M be/src/service/session-expiry-test.cc
2 files changed, 10 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7712/1//COMMIT_MSG
Commit Message:

PS1, Line 20: Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
please remove this line and resubmit - Gerrit uses this as its unique ID for 
patch sets, so it thinks that there's two different patches for this one issue.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Henry Robinson 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has uploaded a new change for review.

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

Change subject: 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 polling frequency to check for
the session to be expired was half of session timeout, which is
around ~1 min. The polling frequency has been increased, it's changed
to be around 1 sec with this change.

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

Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
---
M be/src/service/impala-server.cc
M be/src/service/session-expiry-test.cc
2 files changed, 10 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I81505654d796b7cba59206e40cbe54b6e946794f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has posted comments on this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Patch Set 1:

(5 comments)

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

PS1, Line 1817: that
> whether
that replaced by whether, done


PS1, Line 1817:   
> double space
Double space Removed


PS1, Line 1819: int64_t session_timeout_min_ms = 1000;
> Rename this to something like sleep_time_ms , and make it a constant - e.g.
Made it a constant and changed it to uppercase


PS1, Line 1844: FLAGS_use_local_tz_for_unix_timestamp_conversions = true;
> This is very dangerous: what if some other thread reads FLAGS_use_local_tz_
Removed it


http://gerrit.cloudera.org:8080/#/c/7709/1/be/src/service/session-expiry-test.cc
File be/src/service/session-expiry-test.cc:

PS1, Line 45: num_clients
> constants should be ALL_CAPS
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Pranay Singh
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Henry Robinson (Code Review)]

Henry Robinson has posted comments on this change.

Change subject: IMPALA-5108:idle_session_timeout kicks in later than expected
..


Patch Set 1:

(5 comments)

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

PS1, Line 1817:   
double space


PS1, Line 1817: that
whether


PS1, Line 1819: int64_t session_timeout_min_ms = 1000;
Rename this to something like sleep_time_ms , and make it a constant - e.g.:

  const int64_t SLEEP_TIME_MS = 1000;


PS1, Line 1844: FLAGS_use_local_tz_for_unix_timestamp_conversions = true;
This is very dangerous: what if some other thread reads 
FLAGS_use_local_tz_for_unix_timestamp_conversions concurrently?

I'd leave this change out for now.


http://gerrit.cloudera.org:8080/#/c/7709/1/be/src/service/session-expiry-test.cc
File be/src/service/session-expiry-test.cc:

PS1, Line 45: num_clients
constants should be ALL_CAPS


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5108:idle session timeout kicks in later than expected

[Pranay Singh (Code Review)]

Pranay Singh has uploaded a new change for review.

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

Change subject: 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 polling frequency to check for
the session to be expired was half of session timeout, which is
around ~1 min. The polling frequency has been increased, it's changed
to be around 1 sec with this change.

In addition to the above change, the time that displays the Expiry
time of session is displayed in local time zone, this is to better
co-relate with the time detail in the logs.

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
---
M be/src/service/impala-server.cc
M be/src/service/session-expiry-test.cc
2 files changed, 14 insertions(+), 8 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic62f1ece6c128a80676c6eb331ef190a038bee76
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Pranay Singh