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 <t...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 07 Mar 2018 20:34:51 +0000
Gerrit-HasComments: Yes

Reply via email to