Mike Percy has posted comments on this change. Change subject: Add integration tests for replay cache with writes ......................................................................
Patch Set 24: (7 comments) These look like slow tests. Can we move them out of raft_consensus-itest.cc into a new itest? There are a bunch of helpers that should make it easy, since you can just inherit from ExternalMiniClusterITestBase. See TabletReplacementITest for an example. Also, just so I understand what we expect to happen here. We are querying leaders and followers at the same time and the followers will only respond success to the write once it's replicated through to them. Right? So we don't expect them to return NOT_THE_LEADER? http://gerrit.cloudera.org:8080/#/c/3519/24/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: PS24, Line 1043: RestartAnyCrashedTabletServers() This method needs a flag indicating whether servers are allowed to crash in this test. We shouldn't be executing this line in TestWritesWithExactlyOnceSemanticsWithChurnyElections Line 1047: bool mismatched = false; Please add a comment here to summarize the purpose of this logic. It took me a few minutes to grok. PS24, Line 1079: (1234 s/1234/SeedRandom()/ Line 1125: status = controller.status(); Returning controller.status() is already what Write() does. See proxy.cc L99 Line 1141: FAIL() << "Couldn't write request to tablet server @ " << address.ToString(); how about append the last status as well? Line 1181: TEST_F(RaftConsensusITest, TestWritesWithExactlyOnceSemanticsWithChurnyElections) { Needs comment PS24, Line 1184: defined(__SANITIZE_ADDRESS__) || defined(ADDRESS_SANITIZER) we should only need one of these two, right? -- To view, visit http://gerrit.cloudera.org:8080/3519 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f Gerrit-PatchSet: 24 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
