Allow specifying a set of errnos to select from. #24
Conversation
Allow setting a set of errnos rather than just a particular errno or a random one from the entire set. Update the cookbook for random faults to exclude any errnos passed via extra arguments. My objective here is to be able to exclude a specific errno from random injection. The Go runtime gets confused by EAGAINs, which cause it to epoll_wait on the file descriptor. I'd like to exclude EAGAIN from the set of injected errors for my use case.
| void init_default_errnos() | ||
| { | ||
| int32_t errno; | ||
| for (errno = E2BIG; errno < EXFULL; errno++) { |
There was a problem hiding this comment.
I see where this range comes from, but it doesn't make a lot of sense to be honest.
As a follow up we should be more specific about the random errors and drop those that don't make sense, like ENOSYS or EL2NSYNC and OTOH add some that do make sense like EIO.
| { | ||
| std::random_device rd; | ||
| std::uniform_int_distribution<int> dist(E2BIG, EXFULL); | ||
| if (err_nos.empty() == 0) { |
There was a problem hiding this comment.
You mean if (err_nos.empty())?
If it's not empty why override it with default_errnos?
| } | ||
| std::random_device rd; | ||
| std::uniform_int_distribution<int> dist(0, err_nos.size()); | ||
| return int(err_nos[dist(rd)]); |
There was a problem hiding this comment.
As a follow up, we need to use a PRNG (default_random_engine) here rather than std::random_device.
The std::random_device that is often hardware based is too expensive and slow if used too frequently once its entropy pool is exhausted. It should be used just to get a seed for initializing the default_random_engine.
It would also be nice to add an option to provide a seed to have reproducible runs.
| @@ -125,7 +137,7 @@ static bool get_lucky(int probability) | |||
| if (dist(rd) > probability) { | |||
| return true; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
cosmetic changes should be done in cosmetic-only patches, that contain no functional change.
| @@ -139,16 +151,13 @@ int error_inject(volatile int in_flight, std::string path, std::string method) | |||
| return 0; | |||
| } | |||
|
|
|||
| // get the fault injection descritor | |||
| // get the fault injection descriptor | |||
There was a problem hiding this comment.
ditto, move to a patch with cosmetic changes only
| // get the err_no to inject | ||
| if (descr.err_no) { |
There was a problem hiding this comment.
Let's instead special case the err_nos.size() == 1 case here.
Although drawing a random number in dist(0, 1) is technically correct, there's no reason to waste cpu cycles on the random code if only one err_no is used.
| @@ -208,14 +217,14 @@ class server_handler: public serverIf { | |||
| } | |||
|
|
|||
| void set_fault(const std::vector<std::string>& methods, const bool random, | |||
| const int32_t err_no, const int32_t probability, | |||
| const std::vector<int32_t> & err_nos, const int32_t probability, | |||
There was a problem hiding this comment.
nit: & sticks to angular bracket please.
There was a problem hiding this comment.
Why not provide it by value and move it below rather than copying?
| set_fault(methods, random, err_no, probability, | ||
|
|
||
| set_fault(methods, random, err_nos, probability, |
There was a problem hiding this comment.
ditto, & coding style, and consider moving a vector rather than passing a reference around and eventually copy it, just to throw away the original vector.
| @@ -7,7 +7,7 @@ | |||
| service server { | |||
|
|
|||
| // Used to get the list of availables systems calls | |||
| list<string> get_methods(), | |||
| list<string> get_methods(), | |||
Allow setting a set of errnos rather than just a particular errno or a
random one from the entire set. Update the cookbook for random faults to
exclude any errnos passed via extra arguments.
My objective here is to be able to exclude a specific errno from
random injection. The Go runtime gets confused by EAGAINs, which cause
it to epoll_wait on the file descriptor. I'd like to exclude EAGAIN from
the set of injected errors for my use case.