Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add delete_creating_tasks option for internal.free() #4588

Merged
merged 10 commits into from
Apr 12, 2019
Merged

Add delete_creating_tasks option for internal.free() #4588

merged 10 commits into from
Apr 12, 2019

Conversation

jovany-wang
Copy link
Contributor

@jovany-wang jovany-wang commented Apr 9, 2019 •
edited
Loading

What do these changes do?

Add an option delete_creating_tasks for free() to indicate whether also delete the obkects' creating tasks.

Linter

  • I've run scripts/format.sh to lint the changes in this PR.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13658/
Test FAILed.

raulchen
raulchen reviewed Apr 10, 2019
View reviewed changes
Copy link
Contributor

@raulchen raulchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a test?

python/ray/internal/internal_api.py
@@ -8,7 +8,7 @@
__all__ = ["free"]


def free(object_ids, local_only=False):
def free(object_ids, local_only=False, delete_creating_tasks=False):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document this new arg

guoyuhong
guoyuhong reviewed Apr 10, 2019
View reviewed changes
src/ray/raylet/lib/java/org_ray_runtime_raylet_RayletClientImpl.h
@@ -96,7 +96,7 @@ Java_org_ray_runtime_raylet_RayletClientImpl_nativeGenerateTaskId(JNIEnv *, jcla
JNIEXPORT void JNICALL
Java_org_ray_runtime_raylet_RayletClientImpl_nativeFreePlasmaObjects(JNIEnv *, jclass,
jlong, jobjectArray,
jboolean);
jboolean, jboolean);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also update the comment of the function signature in both .h and .cc files?

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13673/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13681/
Test FAILed.

@jovany-wang jovany-wang force-pushed the free_delete_creation_task branch from 5bd9e0f to d123dc5 Compare April 10, 2019 07:05
@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13688/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13695/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13697/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13716/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13715/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13727/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13749/
Test FAILed.

guoyuhong
guoyuhong approved these changes Apr 12, 2019
View reviewed changes
Copy link
Contributor

@guoyuhong guoyuhong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@guoyuhong guoyuhong merged commit fe07a5b into ray-project:master Apr 12, 2019
@guoyuhong guoyuhong deleted the free_delete_creation_task branch April 12, 2019 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raulchen raulchen raulchen approved these changes

@guoyuhong guoyuhong guoyuhong approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jovany-wang @AmplabJenkins @raulchen @guoyuhong