Page MenuHomePhabricator

Add compute logging watcher process to kill tail (and itself) if parent dies

Authored by prha on Oct 2 2019, 4:46 AM.



Kind of crappy that we're spawning two processes (one to tail, one to watch), but I like this better than having the ComputeLogManager do the bookkeeping on all of the spawned tail processes and killing off orphaned ones.

Test Plan

bk, also ran the dagster-graphql multiprocessing test and saw that the crashed solid did
not orphan a tail process

Diff Detail

R1 dagster
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

prha created this revision.Oct 2 2019, 4:46 AM
prha edited the summary of this revision. (Show Details)Oct 2 2019, 5:01 AM
prha added a reviewer: Restricted Project.
schrockn accepted this revision.Oct 2 2019, 1:41 PM
schrockn added a subscriber: schrockn.
schrockn added inline comments.

i will never not think of fork as weird


I think this all comment worthy

something like

# There are now two processes. The child process (indicated by the return value of os.fork being
# zero) will run until the parent process has terminated. Only then will it terminate the tail
# process, which otherwise would have been orphaned. The other branch occurs in the parent
# process (where the actual solid computation is happening)
This revision is now accepted and ready to land.Oct 2 2019, 1:41 PM
prha updated this revision to Diff 5386.Oct 2 2019, 4:17 PM

add comments, do some renaming for clarity

This revision was landed with ongoing or failed builds.Oct 2 2019, 4:30 PM
This revision was automatically updated to reflect the committed changes.