Add pending task dependencies to active object IDs #6054

edoakes · 2019-10-30T23:43:50Z

Why are these changes needed?

Part of the stop-gap GC solution.

Related issue number

Checks

I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://ray.readthedocs.io/en/latest/.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failure rates at https://ray-travis-tracker.herokuapp.com/.

AmplabJenkins · 2019-10-30T23:43:59Z

Can one of the admins verify this patch?

stephanie-wang

Can we keep track of the dependencies from the worker instead and pass it along with the current active object IDs? That way we can reuse that logic when we implement ref counting.

src/ray/raylet/node_manager.cc

AmplabJenkins · 2019-10-31T05:08:03Z

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

AmplabJenkins · 2019-11-01T21:36:48Z

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

AmplabJenkins · 2019-11-01T23:50:11Z

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

AmplabJenkins · 2019-11-03T03:27:46Z

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

AmplabJenkins · 2019-11-04T11:51:27Z

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

src/ray/core_worker/core_worker.cc

src/ray/core_worker/core_worker.h

AmplabJenkins · 2019-11-04T21:51:38Z

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

AmplabJenkins · 2019-11-06T22:01:28Z

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

AmplabJenkins · 2019-11-06T22:17:46Z

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

AmplabJenkins · 2019-11-07T00:14:19Z

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

stephanie-wang

Nice! I like the new class :)

Is there a way we can also check that the reference counter is empty after a non-actor task completes?

src/ray/core_worker/reference_count_test.cc

src/ray/core_worker/reference_count.cc

stephanie-wang · 2019-11-07T01:58:23Z

src/ray/core_worker/reference_count.cc

+}
+
+void ReferenceCounter::SetDependencies(
+    const ObjectID &object_id, std::shared_ptr<std::vector<ObjectID>> dependencies) {


Why shared_ptr instead of unique_ptr or &&?

These can be shared among multiple entries - we set the same dependencies for all return values for functions that have multiple returns.

I see, can you add a comment about that in the header?

src/ray/core_worker/reference_count.cc

AmplabJenkins · 2019-11-07T04:19:15Z

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

stephanie-wang

Looks great!

AmplabJenkins · 2019-11-07T20:42:13Z

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

AmplabJenkins · 2019-11-08T01:50:48Z

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

AmplabJenkins · 2019-11-08T05:09:59Z

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

edoakes requested review from stephanie-wang and pcmoritz October 30, 2019 23:43

stephanie-wang reviewed Oct 31, 2019

View reviewed changes

ericl reviewed Oct 31, 2019

View reviewed changes

src/ray/raylet/node_manager.cc Outdated Show resolved Hide resolved

Change to ref counts

22032d0

edoakes force-pushed the deps-ref-count branch from 3fe80c7 to 22032d0 Compare November 1, 2019 18:08

Track deps

c5b6b51

stephanie-wang self-assigned this Nov 2, 2019

Add comments

24973ed

fix duplicate

d901bea

stephanie-wang reviewed Nov 4, 2019

View reviewed changes

src/ray/core_worker/core_worker.cc Outdated Show resolved Hide resolved

src/ray/core_worker/core_worker.cc Outdated Show resolved Hide resolved

src/ray/core_worker/core_worker.h Outdated Show resolved Hide resolved

src/ray/core_worker/core_worker.h Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/master' into deps-ref-count

3661bd1

edoakes added 3 commits November 6, 2019 12:30

Add ReferenceCounter class

bf36214

Fix test

870cd5f

Remove num_references for simplicity

b50a5b1

edoakes added 2 commits November 6, 2019 14:28

More tests

a420bab

Merge branch 'master' into deps-ref-count

29c3840

stephanie-wang reviewed Nov 7, 2019

View reviewed changes

comments

9fdab03

Warning for dangling refs

0235076

stephanie-wang approved these changes Nov 7, 2019

View reviewed changes

Merge branch 'master' into deps-ref-count

92c3ff6

edoakes mentioned this pull request Nov 8, 2019

Enable stopgap GC #6109

Merged

3 tasks

edoakes merged commit ca53af4 into ray-project:master Nov 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pending task dependencies to active object IDs #6054

Add pending task dependencies to active object IDs #6054

edoakes commented Oct 30, 2019

AmplabJenkins commented Oct 30, 2019

stephanie-wang left a comment

AmplabJenkins commented Oct 31, 2019

AmplabJenkins commented Nov 1, 2019

AmplabJenkins commented Nov 1, 2019

AmplabJenkins commented Nov 3, 2019

AmplabJenkins commented Nov 4, 2019

AmplabJenkins commented Nov 4, 2019

AmplabJenkins commented Nov 6, 2019

AmplabJenkins commented Nov 6, 2019

AmplabJenkins commented Nov 7, 2019

stephanie-wang left a comment •

edited

Loading

stephanie-wang Nov 7, 2019

edoakes Nov 7, 2019

stephanie-wang Nov 7, 2019

AmplabJenkins commented Nov 7, 2019

stephanie-wang left a comment

AmplabJenkins commented Nov 7, 2019

AmplabJenkins commented Nov 8, 2019

AmplabJenkins commented Nov 8, 2019

Add pending task dependencies to active object IDs #6054

Add pending task dependencies to active object IDs #6054

Conversation

edoakes commented Oct 30, 2019

Why are these changes needed?

Related issue number

Checks

AmplabJenkins commented Oct 30, 2019

stephanie-wang left a comment

Choose a reason for hiding this comment

AmplabJenkins commented Oct 31, 2019

AmplabJenkins commented Nov 1, 2019

AmplabJenkins commented Nov 1, 2019

AmplabJenkins commented Nov 3, 2019

AmplabJenkins commented Nov 4, 2019

AmplabJenkins commented Nov 4, 2019

AmplabJenkins commented Nov 6, 2019

AmplabJenkins commented Nov 6, 2019

AmplabJenkins commented Nov 7, 2019

stephanie-wang left a comment • edited Loading

Choose a reason for hiding this comment

stephanie-wang Nov 7, 2019

Choose a reason for hiding this comment

edoakes Nov 7, 2019

Choose a reason for hiding this comment

stephanie-wang Nov 7, 2019

Choose a reason for hiding this comment

AmplabJenkins commented Nov 7, 2019

stephanie-wang left a comment

Choose a reason for hiding this comment

AmplabJenkins commented Nov 7, 2019

AmplabJenkins commented Nov 8, 2019

AmplabJenkins commented Nov 8, 2019

stephanie-wang left a comment •

edited

Loading