Feature/vert timeouts #11
Conversation
56889d4
to
0b44dd2
Compare
This works is by changing exec:run/2 to exec:run_link/2, causing erlexec to kill the OS pid when the erlang Pid exits.
0b44dd2
to
cee8854
Compare
cee8854
to
df659f6
Compare
test/wrek_tests.erl
Outdated
| end, | ||
|
|
||
| #{count := Count} = Map, | ||
|
|
There was a problem hiding this comment.
Any particular reason you are using indirect matching to get Count here? I mean, for other test cases you match directly against #{count := Cnt} -> Cnt in the receive clause.
|
|
||
| FailMsgs = lists:filter(fun | ||
| (#wrek_event{type = {wrek, error}, msg = {vert, _}}) -> true; | ||
| (#wrek_event{type = {vert, done}, msg = timeout}) -> true; |
There was a problem hiding this comment.
So, if any vertex times out, then we will have an error event for the graph, plus a done event with timeout message for the corresponding vertex, right?
There was a problem hiding this comment.
That's right. The done event with timeout is sent by a wrek_vert process, and after shutting down with {shutdown, timeout}, which does not match {shutdown, {ok, _}}, a wrek process will emit a error event.
test/wrek_tests.erl
Outdated
| gen_event:stop(EvMgr), | ||
|
|
||
| % {wrek, start} + | ||
| % ({starting_vert}, {vert, start}, {vert, done}) * #ok_verts + | ||
| % ({starting_vert}, {vert, start}, {exec, exit_status}, {vert, done}) * #bad_verts + | ||
| % {wrek, done} = | ||
| % (3 * #ok_verts) + (4 * #bad_verts) + 2 | ||
| ?assertEqual((3 * maps:size(VertMap)) + 3, Count). | ||
| ?assertEqual((3 * maps:size(VertMap)) + 2, Count). |
There was a problem hiding this comment.
It is my understanding that in this test case, there are 3 ok vertices and 1 bad vertex. If that is true, then it seems like the assertion does not comply with the formula in the comments.
There was a problem hiding this comment.
This is a bad test. I guess there's no time like the present to rewrite it!
| } = State, | ||
| wrek_event:vert_done(EvMgr, Id, timeout), | ||
| {stop, {shutdown, timeout}, State}; | ||
|
|
There was a problem hiding this comment.
So, with total failure mode, this will terminate the whole graph, whereas with partial failure mode this will terminate this vertex only (and any others that depend on it), right? I mean once this is merged.
If the above is correct, then this timeout feature is essentially a timeout for individual vertices (which is good).
There was a problem hiding this comment.
Yes. Timeouts are meant to be set on individual vertices. It would be possible to have a timeout for an entire wrek, but right now it is implicitly the sum of the vertex timeouts in the critical path.
| @@ -24,13 +24,14 @@ exec(Dir0, Cmd0, Env, EventFun) -> | |||
| sync, | |||
| {cd, Dir}, | |||
| {env, Env}, | |||
| {kill_timeout, 0}, | |||
There was a problem hiding this comment.
kill_timeout is an option for erlexec that governs how long to wait for a child OS process to exit gracefully after a SIGTERM before sending a SIGKILL.
- Replace lists:filter calls with list comprehensions - Split test that counts number of events
Fix #10.