Page MenuHomePhabricator

Clean up gRPC servers when there's an error in the constructor (for example, not being able to load the code on the server)
ClosedPublic

Authored by dgibson on Thu, Nov 12, 6:15 PM.

Details

Summary

This was causing memory leaks in tests and is generally a bad thing that we shouldn't do.

Test Plan

BK, in particular run test_bad_schedules_mixed_with_good_schedule 25 times, no longer OOMs

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

alangenfeld added inline comments.
python_modules/dagster/dagster/core/host_representation/handle.py
242–247

I *think* this is legit - but also worry that i dont understand __del__ in python well enough to be certain

This revision is now accepted and ready to land.Thu, Nov 12, 7:25 PM
python_modules/dagster/dagster/core/host_representation/handle.py
242–247

exceptions in destructors just get swallowed and log, so this can't mess things up too badly if it throws

python_modules/dagster/dagster/core/launcher/default_run_launcher.py
215

this shouldn't be needed, anything creating a managed handle should also be cleaning it up at this point