Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/9375 )
Change subject: KUDU-2297 (part 5): dump stacks into diagnostics log on service queue overflow ...................................................................... Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/9375/5/src/kudu/rpc/service_pool.cc File src/kudu/rpc/service_pool.cc: http://gerrit.cloudera.org:8080/#/c/9375/5/src/kudu/rpc/service_pool.cc@136 PS5, Line 136: if (too_busy_hook_) { : too_busy_hook_(); : } You may get a less noisy stack if you call this before responding to the RPC. Since responding to the RPC would start off some reactor thread tasks asynchronously. But the trade off is that you'll reply to this RPC a little later. Not a big deal, but I just thought I'd bring it up. http://gerrit.cloudera.org:8080/#/c/9375/5/src/kudu/server/diagnostics_log.cc File src/kudu/server/diagnostics_log.cc: http://gerrit.cloudera.org:8080/#/c/9375/5/src/kudu/server/diagnostics_log.cc@118 PS5, Line 118: MutexLock l(lock_); Why bother locking this? It seems to me like when RunThread() gives up the lock to do the actual logging, the 'dump_stacks_now_reason_' can be overwritten anyway by another thread calling DumpStacksNow(). Unless I'm missing something. -- To view, visit http://gerrit.cloudera.org:8080/9375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62019c71121d7ca4037cab86a7c2ea048a261ad8 Gerrit-Change-Number: 9375 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Wed, 07 Mar 2018 20:34:51 +0000 Gerrit-HasComments: Yes
