[core] Fix bug in dynamic generator tasks in object location handling #29082
Conversation
Signed-off-by: Stephanie Wang <[email protected]>
Signed-off-by: Stephanie Wang <[email protected]>
Signed-off-by: Stephanie Wang <[email protected]>
python/ray/tests/test_generators.py
Outdated
| time.sleep(0.1) | ||
|
|
||
| @ray.remote | ||
| def read(gen): |
There was a problem hiding this comment.
Ah thanks, will remove.
| def dynamic_generator(num_returns): | ||
| for i in range(num_returns): | ||
| yield np.ones(1_000_000, dtype=np.int8) * i | ||
| time.sleep(0.1) |
There was a problem hiding this comment.
how often does this test reproduce the bug? If low, would increase the sleep increase the likelyhood of reproducibility?
There was a problem hiding this comment.
It reproduces reliably on my laptop! So I think it's probably safe enough on CI.
Not sure of a good way to do this since the relevant logic is all in the CoreWorker handler. |
Signed-off-by: Stephanie Wang <[email protected]>
Signed-off-by: Stephanie Wang <[email protected]>
Seems in the future, we should consider how to refactor the code so it's more unit-testable. |
For sure, it's been a long-standing issue :( |
…ray-project#29082) When dynamically generated objects are put into the object store, we need to notify the owner of the new location. However, currently this message can reach the owner before the owner knows about the object, since the task reply can arrive much later. Then, the owner will think that the object has already gone out of scope and will not add the location, leading to an ObjectFetchTimedOutError once any worker on a different node tries to read the object. This PR fixes the bug by adding the object to the ref counter if it was returned by a dynamic generator task. The owner does this by checking whether the task spec had num_returns="dynamic" set. Related issue number Closes ray-project#28911. Signed-off-by: Stephanie Wang <[email protected]> Signed-off-by: Weichen Xu <[email protected]>
Why are these changes needed?
When dynamically generated objects are put into the object store, we need to notify the owner of the new location. However, currently this message can reach the owner before the owner knows about the object, since the task reply can arrive much later. Then, the owner will think that the object has already gone out of scope and will not add the location, leading to an ObjectFetchTimedOutError once any worker on a different node tries to read the object.
This PR fixes the bug by adding the object to the ref counter if it was returned by a dynamic generator task. The owner does this by checking whether the task spec had num_returns="dynamic" set.
Related issue number
Closes #28911.
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.