Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8472 )
Change subject: IMPALA-4671: (part-2) Replace kudu::ServicePool with one that uses Impala threads ...................................................................... Patch Set 4: (5 comments) What testing did you do? Did you verify the threads show up on /threadz and things look right when krpc is enabled? http://gerrit.cloudera.org:8080/#/c/8472/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8472/4//COMMIT_MSG@33 PS4, Line 33: This patch however does not display instrumentation on the Webpages yet. : In a future patch, we will add that through the ImpalaServicePool and the : RpcMgr. can you file a JIRA for that (and you could reference it here). http://gerrit.cloudera.org:8080/#/c/8472/4/be/src/rpc/impala-service-pool.h File be/src/rpc/impala-service-pool.h: http://gerrit.cloudera.org:8080/#/c/8472/4/be/src/rpc/impala-service-pool.h@65 PS4, Line 65: // Number of RPCs that timed out while waiting in the service queue. : AtomicInt32 rpcs_timed_out_in_queue_; : : // Number of RPCs that were rejected due to the queue being full. : AtomicInt32 rpcs_queue_overflow_; these aren't used by anything yet, right? let's add a TODO with a JIRA number. Could do the same thing for "unused_histogram_". http://gerrit.cloudera.org:8080/#/c/8472/4/be/src/rpc/impala-service-pool.h@75 PS4, Line 75: boost::mutex shutdown_lock_; comment on what that synchronizes http://gerrit.cloudera.org:8080/#/c/8472/4/be/src/rpc/impala-service-pool.cc File be/src/rpc/impala-service-pool.cc: http://gerrit.cloudera.org:8080/#/c/8472/4/be/src/rpc/impala-service-pool.cc@40 PS4, Line 40: using boost::lock_guard; : using boost::mutex; : using std::shared_ptr; : using strings::Substitute; include names.h instead http://gerrit.cloudera.org:8080/#/c/8472/4/be/src/service/impalad-main.cc File be/src/service/impalad-main.cc: http://gerrit.cloudera.org:8080/#/c/8472/4/be/src/service/impalad-main.cc@58 PS4, Line 58: DECLARE_bool(use_krpc); why do we have that now? -- To view, visit http://gerrit.cloudera.org:8080/8472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia3e0fa9ae22c4803b0cea736eb35e333e2671448 Gerrit-Change-Number: 8472 Gerrit-PatchSet: 4 Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Comment-Date: Mon, 04 Dec 2017 18:26:26 +0000 Gerrit-HasComments: Yes
