Todd Lipcon has posted comments on this change.

Change subject: KUDU-1469. Fix handling of fully-deduped requests after a 
leader change
......................................................................


Patch Set 3:

(5 comments)

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

Line 1192:     state_->UpdateLastReceivedOpIdUnlocked(last_from_leader);
> should we update this method's name? now that it does't always update the l
I amended the method doc comment for this method, but I'm not sure of a better 
name. UpdateLastReceivedOpIdIfNecessaryAndAlsoLastReceivedFromCurrentLeader 
seems a bit verbose ;-) Any suggestion?


http://gerrit.cloudera.org:8080/#/c/3228/3/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:

Line 201:   void InsertPayloadExpectTimeout(int start_row, int num_rows, int 
payload_size);
> this naming of this method is weird because it doesn't actually expect the 
Done


PS3, Line 2314: leader
> leader follower?
Done


PS3, Line 2363: replicatoin
> typo
Done


PS3, Line 2395: first
> nit: line the names up?
Yea, it doesnt matter that they were both NO_OPs, just that they diverge. i.e 
that the old leader has replicated one more operation that wasn't replicated to 
the new leader.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <david.al...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to