-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix bug in which actor classes are not exported multiple times. #4838
Merged
robertnishihara
merged 1 commit into
ray-project:master
from
robertnishihara:fixexportbug
May 23, 2019
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -186,8 +186,9 @@ class ActorClass(object): | |||||||||||
task. | ||||||||||||
_resources: The default resources required by the actor creation task. | ||||||||||||
_actor_method_cpus: The number of CPUs required by actor method tasks. | ||||||||||||
_exported: True if the actor class has been exported and false | ||||||||||||
otherwise. | ||||||||||||
_last_export_session: The index of the last session in which the remote | ||||||||||||
function was exported. This is used to determine if we need to | ||||||||||||
export the remote function again. | ||||||||||||
_actor_methods: The actor methods. | ||||||||||||
_method_decorators: Optional decorators that should be applied to the | ||||||||||||
method invocation function before invoking the actor methods. These | ||||||||||||
|
@@ -208,7 +209,7 @@ def __init__(self, modified_class, class_id, max_reconstructions, num_cpus, | |||||||||||
self._num_cpus = num_cpus | ||||||||||||
self._num_gpus = num_gpus | ||||||||||||
self._resources = resources | ||||||||||||
self._exported = False | ||||||||||||
self._last_export_session = None | ||||||||||||
|
||||||||||||
self._actor_methods = inspect.getmembers( | ||||||||||||
self._modified_class, ray.utils.is_function_or_method) | ||||||||||||
|
@@ -341,10 +342,14 @@ def _remote(self, | |||||||||||
*copy.deepcopy(args), **copy.deepcopy(kwargs)) | ||||||||||||
else: | ||||||||||||
# Export the actor. | ||||||||||||
if not self._exported: | ||||||||||||
if (self._last_export_session is None | ||||||||||||
or self._last_export_session < worker._session_index): | ||||||||||||
# If this actor class was exported in a previous session, we | ||||||||||||
# need to export this function again, because current GCS | ||||||||||||
# doesn't have it. | ||||||||||||
self._last_export_session = worker._session_index | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This more or less mimics ray/python/ray/remote_function.py Lines 112 to 116 in 1a39fee
|
||||||||||||
worker.function_actor_manager.export_actor_class( | ||||||||||||
self._modified_class, self._actor_method_names) | ||||||||||||
self._exported = True | ||||||||||||
|
||||||||||||
resources = ray.utils.resources_from_resource_arguments( | ||||||||||||
cpus_to_use, self._num_gpus, self._resources, num_cpus, | ||||||||||||
|
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We initialize this to
None
instead of toglobal_worker._session_index
because we don't actually export the actor class here.