Adar Dembo has posted comments on this change. Change subject: disk failure: tests for disk failure recovery ......................................................................
Patch Set 10: (8 comments) http://gerrit.cloudera.org:8080/#/c/7031/7/src/kudu/integration-tests/disk_failure-itest.cc File src/kudu/integration-tests/disk_failure-itest.cc: PS7, Line 68: > Done Not done? Also apply this change to your other testing change. Line 233: // servers with EIOs would have been shut down entirely. > Applying sequentially means there are only MRS flushes. OK, could you add a comment explaining that? http://gerrit.cloudera.org:8080/#/c/7031/10/src/kudu/integration-tests/disk_failure-itest.cc File src/kudu/integration-tests/disk_failure-itest.cc: PS10, Line 58: const vector<string> kDefaultFlags = { : // Flush frequently to trigger writes. : "--flush_threshold_mb=1", : "--flush_threshold_secs=1", : : // Ensure a tablet will only store data on a single disk. : "--fs_target_data_dirs_per_tablet=1", : }; Nit: move these to GetDefaultFlags() (i.e. closer to where they're actually used). Line 73: void SetupDefaultTables() { It's creating just one table though. So SetupDefaultTable() maybe? Line 93: if (GetParam() == FBM) { How about place the string representation of the gflag directly in the enum, then you won't need this conversion. Line 107: CHECK_EQ(3, tablet_servers_.size()); Nit: compare with FLAGS_num_tablet_servers, so that if we want to update that value, there are two places in the code to update instead of one. Line 332: SetServerSurvivalFlags(ext_tservers); NO_FATALS. Line 350: write_workload.set_write_pattern(TestWorkload::WritePattern::INSERT_SEQUENTIAL_ROWS); Add comment justifying SEQUENTIAL. -- To view, visit http://gerrit.cloudera.org:8080/7031 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8ff63ec71ab718866484b9f3ec7264bc72ecfe97 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
