Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/12579 )
Change subject: IMPALA-7800: Reject new connections after --fe_service_threads ...................................................................... Patch Set 6: (6 comments) Addressed your comments, Michael. Please have another look. Thanks. http://gerrit.cloudera.org:8080/#/c/12579/6/be/src/rpc/TAcceptQueueServer.h File be/src/rpc/TAcceptQueueServer.h: http://gerrit.cloudera.org:8080/#/c/12579/6/be/src/rpc/TAcceptQueueServer.h@88 PS6, Line 88: void CleanupAndClose(const std::string& error, > Please add a brief comment about what this function does and what those par Done http://gerrit.cloudera.org:8080/#/c/12579/6/be/src/rpc/TAcceptQueueServer.cpp File be/src/rpc/TAcceptQueueServer.cpp: http://gerrit.cloudera.org:8080/#/c/12579/6/be/src/rpc/TAcceptQueueServer.cpp@29 PS6, Line 29: DEFINE_int64(accepted_cnxn_timeout, 300, : "(Advanced) The amount of time in seconds an accepted connection will wait in the " : "queue before we time it out and reject the connection request. A value of 0 " : "means there is no timeout."); > Sorry but now that I realize this may affect all existing Thrift based serv Changed the implementation to pass the timeout value via the constructor chain from ImpalaServer. Now only HS2 and Beeswax servers have the timeout. http://gerrit.cloudera.org:8080/#/c/12579/6/be/src/rpc/TAcceptQueueServer.cpp@195 PS6, Line 195: wait_time > nit: coding convention in Impala usually keeps the unit as the suffix of th Done http://gerrit.cloudera.org:8080/#/c/12579/6/be/src/rpc/TAcceptQueueServer.cpp@205 PS6, Line 205: msecs > nit: we either use ms or milliseconds. Done http://gerrit.cloudera.org:8080/#/c/12579/6/be/src/rpc/TAcceptQueueServer.cpp@207 PS6, Line 207: (wait_result == THRIFT_ETIMEDOUT) > Do we care about any potential non-zero result value ? Should we bail out i It does not seem to return any other non-zero value besides THRIFT_TIMEDOUT. https://github.com/apache/thrift/blob/master/lib/cpp/src/thrift/concurrency/Monitor.cpp#L78 http://gerrit.cloudera.org:8080/#/c/12579/6/be/src/rpc/TAcceptQueueServer.cpp@265 PS6, Line 265: if (FLAGS_accepted_cnxn_timeout != 0) : entry->expiration_time_ = : MonotonicMillis() + FLAGS_accepted_cnxn_timeout * MILLIS_PER_SEC; > nit styling: Done -- To view, visit http://gerrit.cloudera.org:8080/12579 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb345c1d84cc2f691f54ded467f253e758f87e64 Gerrit-Change-Number: 12579 Gerrit-PatchSet: 6 Gerrit-Owner: Zoram Thanga <zo...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Thomas Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com> Gerrit-Comment-Date: Tue, 19 Mar 2019 02:36:16 +0000 Gerrit-HasComments: Yes
