Sailesh Mukil 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 5: (5 comments) 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). Done 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: // TODO: Display these metrics in the debug webpage. IMPALA-6269 : // 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 numb Done http://gerrit.cloudera.org:8080/#/c/8472/4/be/src/rpc/impala-service-pool.h@75 PS4, Line 75: > comment on what that synchronizes Done. Since the lock theoretically seems unnecessary, I've left a TODO to consider removing it later. 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: : METRIC_DEFINE_histogram(server, impala_unused, : "RPC Queue Time", : kudu::MetricUnit::kMic > include names.h instead Done 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: > why do we have that now? It's not needed. Removed. -- 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: 5 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 21:35:20 +0000 Gerrit-HasComments: Yes