Page MenuHomePhabricator

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

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

Details

Summary

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

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

Event Timeline

prha created this revision.Wed, Oct 2, 4:46 AM
prha edited the summary of this revision. (Show Details)Wed, Oct 2, 5:01 AM
prha added a reviewer: Restricted Project.
schrockn accepted this revision.Wed, Oct 2, 1:41 PM
schrockn added a subscriber: schrockn.
schrockn added inline comments.
python_modules/dagster/dagster/core/execution/compute_logs.py
81–108

i will never not think of fork as weird

82

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.Wed, Oct 2, 1:41 PM
prha updated this revision to Diff 5386.Wed, Oct 2, 4:17 PM

add comments, do some renaming for clarity

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