Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8245 )
Change subject: WIP: KUDU-2048. consensus: only evict unresponsive nodes if remaining voters are viable ...................................................................... Patch Set 1: (1 comment) rev 3 has a pretty substantial cleanup (as described in the comment). I also tested rev 3 on a cluster and found that it resolved the problem well. Will update the commit message and do a bit of cleanup and then re-post, but worth taking a short look at the direction if anyone has time. http://gerrit.cloudera.org:8080/#/c/8245/1/src/kudu/consensus/consensus_queue.cc File src/kudu/consensus/consensus_queue.cc: http://gerrit.cloudera.org:8080/#/c/8245/1/src/kudu/consensus/consensus_queue.cc@383 PS1, Line 383: auto unreachable_time = now - peer->last_successful_communication_time; > How about requiring a strict majority of the peers not being evicted to hav thought about this a bit. I think using just the committed index is not quite enough since it won't solve the problem for tablets that aren't actively receiving writes. However it does make sense to _also_ use the committed index in addition to a time bound, for the reasons you mentioned. In that case I think we can relax the time bound a bit. I'll use the consensus RPC timeout since that's now 30 seconds. I also did some closer reading of the code and found that last_successful_communication_time gets updated even in cases where replication is not successful (eg due to TABLET_NOT_FOUND or BOOTSTRAPPING state) which isn't really ideal here. Additionally, I ran some tests and realized that the time-based heuristic is missing one element: if the follower has actually crashed and is returning "Connection refused" errors, we'd not distinguish that from a replica which is just being slow over the last 15 seconds. In order to solve the above problems, I went through and did some cleanup which ended up in this patch. I still think a simple approach like proposed here might be a good thing to backport to prior branches, though, rather than the more invasive cleanup. -- To view, visit http://gerrit.cloudera.org:8080/8245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I673f5b8a58b3954ea28066ecb334b3fdd60e7adb Gerrit-Change-Number: 8245 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Thu, 19 Oct 2017 16:35:09 +0000 Gerrit-HasComments: Yes
