Michael Ho 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 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/12579/5/be/src/rpc/TAcceptQueueServer.cpp File be/src/rpc/TAcceptQueueServer.cpp: http://gerrit.cloudera.org:8080/#/c/12579/5/be/src/rpc/TAcceptQueueServer.cpp@198 PS5, Line 198: if (entry->expiration_time_ != 0) : wait_time = entry->expiration_time_ - MonotonicMillis(); styling nit: if (entry->expiration_time_ != 0) { wait_time = entry->expiration_time_ - MonotonicMillis(); } http://gerrit.cloudera.org:8080/#/c/12579/5/be/src/rpc/TAcceptQueueServer.cpp@200 PS5, Line 200: LOG(INFO) << "All " << maxTasks_ << " server threads are in use. " : << "Waiting for " << wait_time << " msecs."; Does it make sense to log only if we are timing out a request below ? http://gerrit.cloudera.org:8080/#/c/12579/5/be/src/rpc/TAcceptQueueServer.cpp@263 PS5, Line 263: MonotonicMillis() + FLAGS_accepted_cnxn_timeout * MILLIS_PER_SEC; nit: indent 4 http://gerrit.cloudera.org:8080/#/c/12579/5/be/src/rpc/TAcceptQueueServer.cpp@267 PS5, Line 267: connection_setup_pool.Offer( In theory, this could block too but it's still counted towards the expiration_time_. So, is it possible for the code to reach line 199 above and compute a negative wait_time value ? http://gerrit.cloudera.org:8080/#/c/12579/5/tests/custom_cluster/test_frontend_connection_limit.py File tests/custom_cluster/test_frontend_connection_limit.py: http://gerrit.cloudera.org:8080/#/c/12579/5/tests/custom_cluster/test_frontend_connection_limit.py@1 PS5, Line 1: nit: blank line -- 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: 5 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: Fri, 01 Mar 2019 23:30:56 +0000 Gerrit-HasComments: Yes
