Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8134 )

Change subject: KUDU-2155: disable failure detector around elections
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8134/4/src/kudu/consensus/raft_consensus.h
File src/kudu/consensus/raft_consensus.h:

http://gerrit.cloudera.org:8080/#/c/8134/4/src/kudu/consensus/raft_consensus.h@642
PS4, Line 642:  This typically occurs at startup
             :   // and when the local peer steps down as leader.
> Also while you're here, it might be nice to standardize some terminology w.
Will update the comment.

I'd prefer to use armed/disarmed instead of enabled/disabled, but I'm not in 
the mood to rename these functions and make backports harder. So I'll make sure 
we're using enabled/disabled consistently.


http://gerrit.cloudera.org:8080/#/c/8134/3/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

http://gerrit.cloudera.org:8080/#/c/8134/3/src/kudu/consensus/raft_consensus.cc@2217
PS3, Line 2217:   }
> What if ServiceUnavailable is returned not because of the pool shutting dow
This pool is configured with an upper bound on the number of worker threads, 
but no upper bound on the queue size itself. Therefore what you're suggesting 
can only happen if we've queued 2^31-1 callbacks to the pool. Such a case seems 
pathological to me; I can't imagine the server not having been killed by the 
oom killer already.


http://gerrit.cloudera.org:8080/#/c/8134/4/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

http://gerrit.cloudera.org:8080/#/c/8134/4/src/kudu/consensus/raft_consensus.cc@673
PS4, Line 673: !s.ok()
> nit: PREDICT_FALSE? same below
Done


http://gerrit.cloudera.org:8080/#/c/8134/4/src/kudu/consensus/raft_consensus.cc@675
PS4, Line 675: "failed to submit failure detected task"
> nit: this string literal is used twice in this scope; maybe create a variab
Done



--
To view, visit http://gerrit.cloudera.org:8080/8134
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idcd311cee028c48e908f290d60c474e8a4557d97
Gerrit-Change-Number: 8134
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Tue, 04 Feb 2020 17:29:56 +0000
Gerrit-HasComments: Yes

Reply via email to