tvalentyn commented on issue #40673:
URL: https://github.com/apache/beam/issues/40673#issuecomment-6299314798
I think reasons contributing to this deadlock are
1) hosting the model in the same process that operates it via proxies.
2) multiprocessing internals, specifically the fact that finalizers on proxy
objects require a blocking call to the server, and they run on whichever thread
the GC happens to run on, which can be one of the server's own threads. I am
not sure if multiprocessing necessarily forbids using proxies in the same
process, so this might warrant a CPython issue.
Ideas to remediate this (with each of the below, the issue is not
reproducible):
A) use `always_proxy=False`.
B) use `spawn_process=True`
C) monkey-patch multiprocessing internals to make finalizers not blocking
A: We added `always_proxy=True` in
https://github.com/apache/beam/pull/27989 with the motivation to reduce
side-effects of an SDK process crash on other harnesses. The intent of that PR
is not fully accomplished through `always_proxy` since we still don't launch a
separate process with `always_proxy`, more recently we added
`spawn_process=True` for that. In the latter case, the separate process is
still attached to a parent process via a `lifeline` - IIUC if a process that
created a shared MPS handle crashes, the process owning the handle
self-destructs:
https://github.com/apache/beam/blob/f014f5aa0539175141902a9216d4285c120e689c/sdks/python/apache_beam/utils/multi_process_shared.py#L238
.
B: This is a more recent addition. We use it in Model Manager path, and
there were several other issues we recently fixed with model manager, so that
path is not as tested as the main, but still spawning separate processes might
be working well, I haven't tested it beyond confirming that it solves the
deadlock problem in the simple repro above. Likely, this is not a one-line
change and might impact other things, like metrics. It might be better if
multiprocessing makes some assumptions that each shared handle gets a server in
a dedicated process. There are some signs that it does make such assumptions.
C: I'd rather not go this route. It looks like we are already messing with
internals:
https://github.com/apache/beam/blob/7bdc6fff5c94e83538e76f41c419825fadede039/sdks/python/apache_beam/utils/multi_process_shared.py#L57
and that patch might increase the likelihood of such deadlocks, but doesn't
seem to be the root cause for this particular instance I encountered. The
LLM-generated patch doesn't look pretty, and I'd rather revert the existing
patches of this kind if we no longer need them on currently supported Python
versions.
A) could be the easiest remediation for now if it doesn't make things worse.
Though a deadlock might still be possible in a scenario when two SDK processes
each have proxies for objects owned by the opposite process, and then both
proxies are garbage-collected at the exact same instant.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
