Adar Dembo has posted comments on this change.

Change subject: [consensus] KUDU-1613: Fix replica eviction failure for 
WRONG_SERVER_UUID
......................................................................


Patch Set 5:

(5 comments)

I'm deferring to Mike/David since I'm less familiar with this code.

http://gerrit.cloudera.org:8080/#/c/5111/5/src/kudu/consensus/consensus_peers.cc
File src/kudu/consensus/consensus_peers.cc:

Line 238:   switch (ps.response) {
Maybe add a default case that calls LOG(FATAL) on the (unknown) response type?


http://gerrit.cloudera.org:8080/#/c/5111/5/src/kudu/consensus/consensus_peers.h
File src/kudu/consensus/consensus_peers.h:

PS5, Line 140: respond with appropriate response status.
Nit: technically, this method does not "respond" (in the sense of sending an 
RPC response).

Perhaps "and return the appropriate response status"?


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

Line 2770
Why was this comment removed? If it's no longer true, shouldn't the entire 
DeleteTablet() call be thrown out too?


PS5, Line 2782: // These tests exhibit the replica eviction behaviors when a 
follower
              : // returns WRONG_SERVER_UUID error code. Tests verify that 
followers
              : // returning these error codes are evicted from consensus after 
a
              : // specified --follower_unavailable_considered_failed_sec.
Nit: this is talking about tests in the plural, but it's just a single test, 
right?


Line 2795:   int leader_indx = -1;
Nit: conventionally, we use 'idx' as an infix for naming an index, not 'indx'.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d3f84fd297a8e4760208a213c1ee393e92499a3
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Dinesh Bhat <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to