Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14300 )
Change subject: KUDU-2800 draft 2 ...................................................................... Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/14300/7/src/kudu/integration-tests/raft_config_change-itest.cc File src/kudu/integration-tests/raft_config_change-itest.cc: http://gerrit.cloudera.org:8080/#/c/14300/7/src/kudu/integration-tests/raft_config_change-itest.cc@461 PS7, Line 461: SlowBootstrapTest nit: maybe, SlowTabletBootstrapTest would be more precise? http://gerrit.cloudera.org:8080/#/c/14300/7/src/kudu/integration-tests/raft_config_change-itest.cc@508 PS7, Line 508: TServerDetails* leader = nullptr; : ASSERT_OK(GetLeaderReplicaWithRetries(tablet_id_, &leader)); : consensus::ConsensusStatePB cstate; : ASSERT_OK(GetConsensusState(leader, tablet_id_, kTimeout, : EXCLUDE_HEALTH_REPORT, &cstate)); : // Check that new replica has not been added. : ASSERT_FALSE(IsRaftConfigMember(no_replica_servers.front(), cstate.committed_config())); : // Check that the replica at the tablet servers that was restarted is not removed. : ASSERT_TRUE(IsRaftConfigMember(replica_servers.front(), cstate.committed_config())); Does it make sense to make the newly introduced ValidateConsensusState() to take care of this case as well? http://gerrit.cloudera.org:8080/#/c/14300/7/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: http://gerrit.cloudera.org:8080/#/c/14300/7/src/kudu/tserver/ts_tablet_manager.cc@144 PS7, Line 144: hidden Change to 'unsafe'. That way it's automatically hidden from the documentation and requires `--unlock_unsafe_flags` to use. Test-only flags which introduce such stuff are tagged 'unsafe' usually. http://gerrit.cloudera.org:8080/#/c/14300/7/src/kudu/tserver/ts_tablet_manager.cc@1077 PS7, Line 1077: "Injecting " << FLAGS_tablet_open_bootstrap_inject_latency_ms : << "ms delay in tablet bootstrapping" nit: maybe, use strings::Substitute() to format the message for easier readability in the code -- To view, visit http://gerrit.cloudera.org:8080/14300 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie1fee42053194f51d7a869ce14788095d6627ed9 Gerrit-Change-Number: 14300 Gerrit-PatchSet: 7 Gerrit-Owner: Volodymyr Verovkin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Comment-Date: Wed, 02 Oct 2019 19:32:39 +0000 Gerrit-HasComments: Yes
