Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13607 )
Change subject: IMPALA-7802: Close connections of idle client sessions ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/13607/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13607/1//COMMIT_MSG@28 PS1, Line 28: If so, the connection will be closed. This allows the service threads > I was trying to understand this scenario: If there are no open sessions associated with a connection, the connection_to_session state map shouldn't find an entry if I understand the code correctly. In that case, we will by default mark the connection as not idle. Please see https://gerrit.cloudera.org/#/c/13607/1/be/src/service/impala-server.cc@2072 Given the above, I am not sure I understand the concern about setting it to a very low value. http://gerrit.cloudera.org:8080/#/c/13607/1//COMMIT_MSG@30 PS1, Line 30: > One thing I didn't understand about the scope of this - are we trying to ha It's the former for sure. Will update commit message to make it clearer. http://gerrit.cloudera.org:8080/#/c/13607/1/be/src/rpc/TAcceptQueueServer.cpp File be/src/rpc/TAcceptQueueServer.cpp: http://gerrit.cloudera.org:8080/#/c/13607/1/be/src/rpc/TAcceptQueueServer.cpp@78 PS1, Line 78: if (!processor_->process(input_, output_, connectionContext) || > Is it possible that the thread can get stuck in either the process() or pro Yes but that's not the intention of this patch to address those cases. It's hard to tell between a slow network/client vs a stuck client so setting a timeout here may sometimes lead to false positive and prematurely closes a client's connection. A better thing to do instead of a socket timeout is to use TCP keepalive or something similar to TCP_USER_TIMEOUT to check the remote end is still alive. http://gerrit.cloudera.org:8080/#/c/13607/1/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/13607/1/be/src/service/impala-server.cc@221 PS1, Line 221: idle_session_timeout > So if idle_session_timeout is set to 10 minutes and idle_client_poll_period To the first point, yes, that's potentially the latest time the connection of an idle session will be closed. Nope. If idle_session_timeout is not set for a session, a session cannot become idle by definition. Please keep in mind that idle_session_timeout can be set per session during session establishment via query option. The global flag --idle_session_timeout is mostly there to set the default value. -- To view, visit http://gerrit.cloudera.org:8080/13607 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I97c4fb8e1b741add273f8a913fb0967303683e38 Gerrit-Change-Number: 13607 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Anonymous Coward (443) Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoram Thanga <[email protected]> Gerrit-Comment-Date: Fri, 14 Jun 2019 04:46:08 +0000 Gerrit-HasComments: Yes
