Initialize client_id_ in ObjectManager constructor #3403
Conversation
|
Test FAILed. |
|
Test FAILed. |
|
jenkins retest this please |
|
Test FAILed. |
|
@xutianming thanks for submitting this! Were you running into crashes or bugs because of this? If so, what was the scenario that failed? |
In fact, no. This constructor is currently not invoked. |
There was a problem hiding this comment.
@xutianming, thanks for the PR. I think this looks okay as-is.
It's okay to add a method to get the local client id from the object directory, even though it may seem more consistent with the existing design to pass in the client_id via the ObjectManager constructor. We ought to eventually remove the object manager constructor which internally instantiates ObjectDirectory: It will be easier to read and maintain the object manager with a single constructor, and I don't think much would be lost.
| @@ -104,6 +104,11 @@ class ObjectDirectoryInterface { | |||
| virtual ray::Status ReportObjectRemoved(const ObjectID &object_id, | |||
| const ClientID &client_id) = 0; | |||
|
|
|||
| /// Get Local client id | |||
There was a problem hiding this comment.
(nit) Please update the documentation to the following: "Get local client id."
|
jenkins retest this please |
…fined ObjectDirectory
|
Test FAILed. |
|
Test FAILed. |
|
Thanks @xutianming! |
Initialize client_id_ in ObjectManager constructor that takes user-defined ObjectDirectory.
What do these changes do?
In the ObjectManager constructor that takes user-defined ObjectDirectoryInterface implementation,
client_id_ is not initialized. But several member functions rely on client_id_, such as NotifyDirectoryObjectDeleted and HandleObjectAdded.
This pull request initializes client_id_ by invoking GetLocalClientID of ObjectDirectoryInterface, which retrieves local client id through gcs_client.
Related issue number
No