[kudu-CR] Add fault injection of EIOs

["Adar Dembo (Code Review)" <ge...@cloudera.org>]

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 <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes