David Ribeiro Alves has posted comments on this change. Change subject: disk failure: basic test coverage for disk failure ......................................................................
Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/7031/3//COMMIT_MSG Commit Message: PS3, Line 16: salvagable typo http://gerrit.cloudera.org:8080/#/c/7031/3/src/kudu/integration-tests/disk_failure-itest.cc File src/kudu/integration-tests/disk_failure-itest.cc: PS3, Line 61: int num_rows, int payload_size) { nit: indent PS3, Line 62: client::sp::shared_ptr nit: using... PS3, Line 82: // This test is set up so each tablet occupies its own disk on each tserver. : // The '|' represents a separation of disks. : // ts-0 ts-1 ts-2 : // [ a | b ] [ b | a ] [ a | b ] : // : // EIOs are triggered on two disks. : // ts-0 ts-1 ts-2 : // [ X | b ] [ X | a ] [ a | b ] : // : // With improper disk-failure handling, this scenario alone would have been : // enough to leave the server with only a single copy of data, as the two : // servers with EIOs would have been shut down entirely. : // : // With proper disk-failure handling, this scenario would be salvagable, and : // data would be replicated on the remaining disks. : // ts-0 ts-1 ts-2 : // [ X | ba ] [ X | ab ] [ a | b ] nit move this comment to before the test declaration, or at least give an intro there PS3, Line 150: if (files.size() == 1) { : // "Empty" directories will only have the instance file. : path_without_data = data_path; : } else { : path_with_data = data_path; : num_servers_with_data++; : } this is a bit funky (depending on the number of files) any way we can be more precise? also does this work for both block managers? PS3, Line 214: tablets tablet servers -- 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: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
