schizophrenicmaniac opened a new pull request, #40500: URL: https://github.com/apache/beam/pull/40500
The `max_models_per_worker_hint` limit for a KeyedModelHandler with several model handlers was never actually enforced. `run_inference` created a new `threading.Lock()` on every call, so acquiring it always succeeded. Each deserialized copy of the handler (one per DoFn instance, plus new copies when bundle processors are recreated) still had the hint set, so every copy added it to the shared `_ModelHandlerManager` again. In the repro from the issue, five handler copies raised the limit to 5 and all three models stayed loaded with a hint of 1. This change makes `_ModelHandlerManager.increment_max_models` take an optional `process_id` and count each process only once. `KeyedModelHandler.run_inference` passes `os.getpid()`. The pid is computed on the caller's side because the manager is proxied from a separate MultiProcessShared process. Calls without a process id work as before. I added a unit test for the manager and one based on the repro in the issue (five pickled handler copies sharing one manager, hint of 1). The repro test fails on master and passes with this change. I also added a CHANGES.md entry. Fixes #40468 -- 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]
