Mike Percy has posted comments on this change.
Change subject: Add integration tests for replay cache with writes
Patch Set 24:
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?
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
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
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 @ " <<
how about append the last status as well?
Line 1181: TEST_F(RaftConsensusITest,
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-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>