Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9136 )
Change subject: KUDU-1613: evict replicas with wrong server UUIDs ...................................................................... Patch Set 4: (9 comments) http://gerrit.cloudera.org:8080/#/c/9136/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9136/4//COMMIT_MSG@9 PS4, Line 9: rebuilt > what does "rebuilt" mean? Done, although this is documented here: http://kudu.apache.org/docs/administration.html#rebuilding_kudu http://gerrit.cloudera.org:8080/#/c/9136/4/src/kudu/consensus/consensus_peers.cc File src/kudu/consensus/consensus_peers.cc: http://gerrit.cloudera.org:8080/#/c/9136/4/src/kudu/consensus/consensus_peers.cc@320 PS4, Line 320: case TabletServerErrorPB::TABLET_FAILED: > add fallthrough expected or something Done http://gerrit.cloudera.org:8080/#/c/9136/4/src/kudu/consensus/consensus_peers.cc@321 PS4, Line 321: case TabletServerErrorPB::WRONG_SERVER_UUID: > nit: might make sense to reverse the order of the cases (have TabletServerE Done http://gerrit.cloudera.org:8080/#/c/9136/4/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: http://gerrit.cloudera.org:8080/#/c/9136/4/src/kudu/integration-tests/raft_consensus-itest.cc@2750 PS4, Line 2750: test that ensures when we start a new tablet server using the : // same ports, existing tablets that previously used those ports will be : // evicted and replicated elsewhere. > "tablets that used those ports"? this is confusing, please rephrase. maybe: Done http://gerrit.cloudera.org:8080/#/c/9136/4/src/kudu/integration-tests/raft_consensus-itest.cc@2755 PS4, Line 2755: is_3_4_3 > this is tied to a layout (3 replicas) could you just use the flag name inst Done http://gerrit.cloudera.org:8080/#/c/9136/4/src/kudu/integration-tests/raft_consensus-itest.cc@2805 PS4, Line 2805: env_->CreateDir > check status Done http://gerrit.cloudera.org:8080/#/c/9136/4/src/kudu/integration-tests/raft_consensus-itest.cc@2806 PS4, Line 2806: env_->CreateDir > check status Done http://gerrit.cloudera.org:8080/#/c/9136/4/src/kudu/integration-tests/raft_consensus-itest.cc@2809 PS4, Line 2809: new_ts->Start(); > check status, here and elsewhere Done http://gerrit.cloudera.org:8080/#/c/9136/4/src/kudu/integration-tests/raft_consensus-itest.cc@2810 PS4, Line 2810: LOG(INFO) << Substitute("Created new tablet server: $0 ($1)", : new_ts->uuid(), ts_opts.rpc_bind_address.ToString()); > did you mean to leave this. doesn't seem super informative... Done -- To view, visit http://gerrit.cloudera.org:8080/9136 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If0c7ae9dfbd99c2c9a38e8b1f7748c70882e06b4 Gerrit-Change-Number: 9136 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Wed, 31 Jan 2018 18:36:21 +0000 Gerrit-HasComments: Yes
