Andrew Wong has posted comments on this change.

Change subject: Add fault injection of EIOs
......................................................................


Patch Set 11:

(7 comments)

Haven't addressed everything, marked some "will do"s. Pushed after rebasing as 
along with the EMC patch

http://gerrit.cloudera.org:8080/#/c/6881/8/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

Line 107:   const string& f_ = (filename_expr); \
> Can we call this MAYBE_RETURN_EIO to emphasize the similarity with MAYBE_RE
Done


Line 150:               "Fraction of the time that operations on certain files 
will fail "
> Why can't we reuse env_inject_io_error? I think it's reasonable for env_inj
We can, although there's the subtlety that suicide_on_eio should be switched to 
false in tests that use the current env_inject_io_error.
Will try it out.


Line 153: DEFINE_string(env_inject_eio_regex, "",
> Should rename to emphasize symmetry with env_inject_eio/env_inject_io_error
Done


Line 154:               "ECMAScript regex specifying files on which I/O will 
fail. If "
> Nit: Instead of "bad" just explain how this ties into error injection.
Done


Line 156: TAG_FLAG(env_inject_eio_regex, hidden);
> Nit: it's easy to tag the wrong flags, so let's colocate the flag tags and 
Done


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_fla
Yeah that's MAYBE_EVAL_AND_RETURN_FAILURE. Will replace the implementation 
since it seems like MAYBE_EVAL_AND_RETURN_FAILURE's behavior is expected.


PS8, Line 51: describe
> described
Done


-- 
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: 11
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

Reply via email to