Repository navigation
[core][__on_object_freed__][2/n] Run the __on_object_freed__ hook through an OnObjectFreed RPC - #67308
[core][__on_object_freed__][2/n] Run the __on_object_freed__ hook through an OnObjectFreed RPC#67308karticam wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements the backend and frontend support for running an actor's on_object_freed hook when an object it created is freed. It introduces the OnObjectFreed RPC, adds scheduling logic to run the hook on the actor's executor (respecting concurrency groups and event loops), and includes comprehensive unit tests. A review comment identified an issue in _raylet.pyx where the arguments for PythonFunctionDescriptor are swapped, which would cause incorrect metadata representation.
| hook_descriptor = PythonFunctionDescriptor( | ||
| creation_descriptor.module_name, "__on_object_freed__", | ||
| creation_descriptor.class_name) |
There was a problem hiding this comment.
The arguments passed to PythonFunctionDescriptor are swapped. The constructor signature is PythonFunctionDescriptor(module_name, class_name, function_name), but here "__on_object_freed__" (the function name) is passed as the second argument, and creation_descriptor.class_name (the class name) is passed as the third argument. This will cause incorrect metadata representation in dashboard, logging, or tracing.
hook_descriptor = PythonFunctionDescriptor(creation_descriptor.module_name, creation_descriptor.class_name, "__on_object_freed__")
78e4b3f to
5698b6e
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5698b6e. Configure here.
|
|
||
| virtual void HandleOnObjectFreed(OnObjectFreedRequest request, | ||
| OnObjectFreedReply *reply, | ||
| SendReplyCallback send_reply_callback) = 0; |
There was a problem hiding this comment.
Missing Doxygen on new functions
Low Severity
/** ... */ block comments using @ tags (@brief, and @param/@return where applicable).
Additional Locations (2)
Triggered by project rule: Bugbot Rules
Reviewed by Cursor Bugbot for commit 5698b6e. Configure here.
5698b6e to
3eb9cc8
Compare
Signed-off-by: kartica.modi <kartica.modi@anyscale.com>
3eb9cc8 to
f869a59
Compare


This is PR 2 of a series of PRs that add the
__on_object_freed__hook: an actor that defines the hook will be told when an object it created is freed. This PR adds the RPC that runs the hook on the actor's worker. Nothing sends it yet: a later PR makes the raylet send it when the last copy of such an object is deleted.The complete design doc is here: https://docs.google.com/document/d/1ZJhgRmwVyKfNCR21CWf6xMcc0B6HJwSPQsfebTRvnnE/edit?tab=t.0#heading=h.7wcod1f9afpg
Changes
CoreWorkerService.OnObjectFreedRPC. The request carries the freed object's ID and the ID of the worker it is meant for. A worker that reuses the address of a dead one rejects it, likePushTask. The client side (CoreWorkerClient::OnObjectFreed) is added too, for the raylet to use later.HandleOnObjectFreedschedules the hook and replies right away: the raylet doesn't use the result, and a hook waiting behind a long actor method shouldn't keep the RPC open. If the worker dies before the hook runs, the hook doesn't run, which is the same as for a dead worker.TaskReceiver::RunOnActorExecutorpicks the group's fiber executor for an asyncio actor, the group's thread pool for a threaded actor, and the task execution thread otherwise, where the hook runs between the actor's methods. The hook's flag and group from [core][__on_object_freed__][1/n] Detect the __on_object_freed__ hook on actor classes #67232, and the actor's executors, are read on the task execution thread, which sets them during actor creation, so the hook never runs before the actor's__init__has finished (since the hook is also queued on the task execution service).async defand sync hooks run on the group's event loop, as actor methods do. AnExceptionraised by the hook is logged and the actor keeps running. A test-onlyCoreWorker._schedule_on_object_freed_hook_for_testingruns the same path as the RPC, so the hook can be tested before the raylet sends the RPC.