Adar Dembo has posted comments on this change. Change subject: Add fault injection of EIOs ......................................................................
Patch Set 8: (9 comments) http://gerrit.cloudera.org:8080/#/c/6881/8/src/kudu/util/env-test.cc File src/kudu/util/env-test.cc: Line 1009: FLAGS_env_bad_dir_regex = Substitute("($0)", kTestRWPath1); Why do we need to surround the pathname with parens? Line 1016: FLAGS_env_bad_dir_regex = "(neither_path)"; And here too? http://gerrit.cloudera.org:8080/#/c/6881/8/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 107: #define MAYBE_INJECT_EIO(filename_expr, error_expr) do { \ Can we call this MAYBE_RETURN_EIO to emphasize the similarity with MAYBE_RETURN_FAILURE? Line 150: DEFINE_double(env_inject_eio, 0.0, Why can't we reuse env_inject_io_error? I think it's reasonable for env_inject_io_error to always add EIO posix codes, if that would help. Line 153: DEFINE_string(env_bad_dir_regex, "", Should rename to emphasize symmetry with env_inject_eio/env_inject_io_error (i.e. "env_inject_eio_dir_regex" or some such). Line 154: "ECMAScript regex specifying bad directories. If empty, all " Nit: Instead of "bad" just explain how this ties into error injection. Line 156: TAG_FLAG(env_inject_eio, hidden); Nit: it's easy to tag the wrong flags, so let's colocate the flag tags and definitions (i.e. don't have the definitions first and tags next). http://gerrit.cloudera.org:8080/#/c/6881/8/src/kudu/util/fault_injection.h File src/kudu/util/fault_injection.h: Line 46: // Note that 'status_expr' is evaluated even if 'fraction_flag' is zero. Can this be addressed? Seems like you could use MaybeTrue() on fraction_flag now, before expanding status_eval? PS8, Line 51: desribed described -- To view, visit http://gerrit.cloudera.org:8080/6881 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5190d5e7f296cf27b65f1ecd2c16f3315cc46a39 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: 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
