David Ribeiro Alves has posted comments on this change. Change subject: Add integration tests for replay cache with writes ......................................................................
Patch Set 32: (4 comments) http://gerrit.cloudera.org:8080/#/c/3519/30/src/kudu/integration-tests/exactly_once_writes-itest.cc File src/kudu/integration-tests/exactly_once_writes-itest.cc: Line 62: Random random(1234); > How about initialize an int seed_ = SeedRandom() in the SetUp() function an Done http://gerrit.cloudera.org:8080/#/c/3519/32/src/kudu/integration-tests/exactly_once_writes-itest.cc File src/kudu/integration-tests/exactly_once_writes-itest.cc: Line 37: void DoTestWritesWithExactlyOnceSemantics(const vector<string>& ts_flags, > nit: Put method declarations in same order as definitions (just switch them Done http://gerrit.cloudera.org:8080/#/c/3519/32/src/kudu/integration-tests/ts_itest-base.h File src/kudu/integration-tests/ts_itest-base.h: Line 101: // TODO remove this once we have ResultTracker GC > Mind adding a JIRA# to this TODO, if there is one? There's no JIRA, the GC patch is up for review. Will remove these comments as I enable this by default when it lands. Line 493: void AssertNoTabletServersCrashed() { > This is already implemented as ExternalMiniCluster::AssertNoCrashes() This is just a move from the child class. the raft itest uses this. Happy to remove this and swap to the other method in a follow up. -- 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: 32 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