tvalentyn opened a new pull request, #40441:
URL: https://github.com/apache/beam/pull/40441
RunInference framework uses _ModelStatus objects to store a state of the
model, like whether the model is valid or should be reloaded. We need to store
one _ModelStatus object per one instance of RunInference. SInce there can be
more than one such object, we key them by a tag: `{model_tag}_model_status`.
To share these objects within the SDK process, we rely on `shared.Shared()`,
which also has a concept of tags and is a tool to share a singleton object
across multiple DoFn instances.
However, the usage `return shared.Shared().acquire(lambda:
_ModelStatus(False), tag=tag)` is incorrect since this creates a *new shared
handle* every time get_model_status is called. The subsequent `acquire` call,
creates a new, uninitialized _ModelStatus instance.
This confusion likely is caused by similar Beam APIs: shared vs
multiprocess_shared, which use tags slightly differently.
For MultiProcessShared, tag is the key. The same tag from any process gives
the same object.
For shared.Shared, it is a version label. The key is the handle's own
internal uuid. Acquiring through the same handle with a different tag throws
away the old object and builds a new one, but the same tag through a *new*
handle gives a new object.
The `Shared` handle is generally not expected to be created more than once
*per stage* of the pipeline (recall that this was originally written for Batch
pipelines in TFX; with streaming it gets more complicated since we have all
stages running at once).
Given that a) RunInference used 2 shared handles, one for the model, one for
the ModelStatus, and b) the Shared machinery has a single
`_SharedMap._keepalive` slot for entire process, calling `load_model_status`
invalidated the last existing reference to the model in some cases, causing
#40440.
Because we instantiated a new ModelStatus object every time, we didn't store
the state of previously loaded models properly, so invalidating a model might
have not caused the reload. However, this would only happen if we are NOT
sharing the model across processes, so impact should be limited.
fixes: #40440
--
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]
