ORM: ProcessNode.is_valid_cache is False for unsealed nodes #6147
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `Sealable` mixin is used by the `ProcessNode` which allows it to be sealed. By having the `seal` method return `self`, which will be the `ProcessNode` instance, it brings the behavior on par with the `store` method.
When a `ProcessNode` is not yet sealed, it has not officially been terminated. At this point it cannot yet be considered a valid cache source. However, this condition was not considered by the property `ProcessNode.is_valid_cache`. This bug manifested itself in very rare situations where a race condition could lead to a process being cached from an unsealed node. When a node is stored from the cache, it copies all the attribute except for the sealed key and adds the outputs. The sealing is then left to the engine which will complete the cached process as if it had run normally. The problem arises when the cache source was not yet sealed, and so the outputs had not yet been added. The cached node will then miss the output nodes.
sphuber
force-pushed
the
fix/6100/caching-unsealed
branch
from
c99ef8a to
2c2c095
Compare
|
Thanks @sphuber, I'll take a look later today. |
unkcpz
approved these changes
Oct 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #6100
When a
ProcessNodeis not yet sealed, it has not officially beenterminated. At this point it cannot yet be considered a valid cache
source. However, this condition was not considered by the property
ProcessNode.is_valid_cache.This bug manifested itself in very rare situations where a race
condition could lead to a process being cached from an unsealed node.
When a node is stored from the cache, it copies all the attribute except
for the sealed key and adds the outputs. The sealing is then left to the
engine which will complete the cached process as if it had run normally.
The problem arises when the cache source was not yet sealed, and so the
outputs had not yet been added. The cached node will then miss the
output nodes.