Distributed ref counting for serialized ObjectIDs by stephanie-wang · Pull Request #6945 · ray-project/ray

stephanie-wang · 2020-01-28T22:56:17Z

Why are these changes needed?

This implements distributed reference counting for object IDs that are serialized and passed to another process. It also implements reference counting for object IDs that get nested in another object.

There should be no change in memory management behavior from the user's perspective. By default, all workers will maintain these new ref counts, but they will not be considered when deciding whether an object is still in scope or not. Therefore, the current behavior is that if the creator of an object ID still has a local reference to the object, then at least one copy of the object will be available in the cluster. The creator of an object ID is the process that called ray.put() or that submitted the task that returns the object ID. The creator's local reference count includes:

Local Python references to the ObjectID.
Number of pending tasks submitted by the creator that depend on the object.

The new reference counts added by this PR include:

Number of objects that contain the ObjectID that are still in scope.
List of processes that have a local reference to the ObjectID.

Eventually, object pinning will also consider the new reference counts when deciding when an object stored in plasma can be evicted. The guarantee will be that if any process in a cluster has a reference to the object ID or an object that transitively contains the object ID, then at least one copy of the object will be available in the cluster. To turn on this behavior, set the distributed_ref_counting_enabled flag to 1 in the internal config passed to ray.init(), like so:

ray.init(_internal_config=json.dumps({
    "distributed_ref_counting_enabled": 1,
  }))

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/.

…ted task pending on the inner ID

AmplabJenkins · 2020-01-28T22:56:38Z

Can one of the admins verify this patch?

AmplabJenkins · 2020-01-29T03:14:00Z

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

…rrower's ref count goes to 0

- keep a sentinel ref count for task argument IDs - keep contained_in_borrowed in addition to contained_in_owned

…at a time

…o unset contained_in when popping refs

AmplabJenkins · 2020-02-03T02:38: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/21259/
Test FAILed.

skip adding ownership info if we already have it to handle duplicate refs

AmplabJenkins · 2020-02-04T10:20:41Z

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

…ng-full

- register handler for WaitForRefRemoved - don't create a python reference for arg IDs - pass in client factory into ReferenceCounter - fix bad decrement in PopBorrowerRefs

- don't decrement for IDs on dependency resolution, wait until task finished - add object IDs that were inlined when building the arguments to the task spec, pin these on the task executor until task finishes

src/ray/core_worker/reference_count_test.cc

Co-Authored-By: Edward Oakes <ed.nmi.oakes@gmail.com>

AmplabJenkins · 2020-02-18T02:41:27Z

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

AmplabJenkins · 2020-02-18T02:59:24Z

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

AmplabJenkins · 2020-02-18T03:10: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/22057/
Test FAILed.

AmplabJenkins · 2020-02-18T04:08: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/22060/
Test FAILed.

AmplabJenkins · 2020-02-18T19:52:14Z

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

AmplabJenkins · 2020-02-19T00:29:17Z

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

stephanie-wang · 2020-02-19T01:29:23Z

@kfstorm, we're trying to merge this ASAP for 0.8.2, but I believe it's breaking a java test on Travis. Can you take a look when you get a chance? Thanks!

Logs: https://api.travis-ci.com/v3/job/288659419/log.txt

kfstorm · 2020-02-19T02:12:47Z

@stephanie-wang I've noticed that the Java CI is not stable recently. testInWorker is one of the flaky tests. Did you rerun it? If you retried several times and still coundn't get it pass, please just ignore it. I'll spend some time investigating the these flaky tests.

stephanie-wang · 2020-02-19T02:22:57Z

@stephanie-wang I've noticed that the Java CI is not stable recently. testInWorker is one of the flaky tests. Did you rerun it? If you retried several times and still coundn't get it pass, please just ignore it. I'll spend some time investigating the these flaky tests.

Good to know, thanks! I didn't look through all the runs, but I think it's been failing pretty consistently on one test, and it looks related to this PR. Here is the relevant output from that run:

FAILED: 
org.ray.api.exception.RayWorkerException: The worker died unexpectedly while executing this task.
	at org.ray.runtime.object.ObjectSerializer.deserialize(ObjectSerializer.java:48)
	at org.ray.runtime.object.ObjectStore.get(ObjectStore.java:99)
	at org.ray.runtime.AbstractRayRuntime.get(AbstractRayRuntime.java:78)
	at org.ray.runtime.AbstractRayRuntime.get(AbstractRayRuntime.java:72)
	at org.ray.api.Ray.get(Ray.java:71)
	at org.ray.runtime.object.RayObjectImpl.get(RayObjectImpl.java:37)
	at org.ray.api.test.MultiThreadingTest.testInWorker(MultiThreadingTest.java:134)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:86)
	at org.testng.internal.Invoker.invokeMethod(Invoker.java:643)
	at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:820)
	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1128)
	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
	at org.testng.TestRunner.privateRun(TestRunner.java:782)
	at org.testng.TestRunner.run(TestRunner.java:632)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:366)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:361)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:319)
	at org.testng.SuiteRunner.run(SuiteRunner.java:268)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1244)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1169)
	at org.testng.TestNG.run(TestNG.java:1064)
	at org.testng.TestNG.privateMain(TestNG.java:1385)
	at org.testng.TestNG.main(TestNG.java:1354)

stephanie-wang added 4 commits January 27, 2020 17:26

Skeleton plus a unit test for simple borrower case

ab3a8be

First unit test passes - forward an ID and task returns with 1 submit…

6f74f53

…ted task pending on the inner ID

Invariant for contained_in

9b1899a

Unit test passes for testing task return without creating a borrower

6b60ecc

stephanie-wang added 11 commits January 30, 2020 11:25

Wrap ref count functionality in test case

c7002b3

Fix bad delete

362a2d7

Unit test and fix for borrowers creating more borrowers

6ed0a0e

Unit test and fix for simple borrowing, but owner sends call after bo…

0ca7863

…rrower's ref count goes to 0

Refactor:

645ff49

- keep a sentinel ref count for task argument IDs - keep contained_in_borrowed in addition to contained_in_owned

Unit test for nested IDs passes

97b4abb

Refactor so that an object ID can only be contained in 1 borrowed ID …

4382854

…at a time

Add check

ffd8c8c

Fix

4e1f5b2

Unit test (passes) to test nesting object IDs but no borrowers created

20d8176

Unit test for nested objects from different owners passes, refactor t…

296a02e

…o unset contained_in when popping refs

stephanie-wang and others added 5 commits February 3, 2020 15:15

Unit tests for borrowers receiving an ObjectID from multiple sources,

8bf9534

skip adding ownership info if we already have it to handle duplicate refs

Unit test for returning object ID passes

67c6576

More unit tests for returning object IDs pass

ac3a5e7

Add serialized ID tests

ee9d672

fix serialization issue

a285e86

stephanie-wang added 6 commits February 4, 2020 10:11

remove swap

2db2c77

Merge remote-tracking branch 'upstream/master' into ref-counting-full

cfef48b

It builds!

4911491

Merge remote-tracking branch 'eoakes/ref-count-tests' into ref-counti…

b0532c7

…ng-full

debugging and some fixes:

b18be4c

- register handler for WaitForRefRemoved - don't create a python reference for arg IDs - pass in client factory into ReferenceCounter - fix bad decrement in PopBorrowerRefs

Fix accounting for serialized IDs:

9044a2d

- don't decrement for IDs on dependency resolution, wait until task finished - add object IDs that were inlined when building the arguments to the task spec, pin these on the task executor until task finishes

edoakes reviewed Feb 18, 2020

View reviewed changes

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

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

Add IDs contained in owned objects

e237c75

stephanie-wang force-pushed the ref-counting branch from a9828de to e237c75 Compare February 18, 2020 02:16

stephanie-wang and others added 8 commits February 17, 2020 18:21

Update src/ray/protobuf/core_worker.proto

5d55d3f

Co-Authored-By: Edward Oakes <ed.nmi.oakes@gmail.com>

Update src/ray/core_worker/reference_count.cc

73b6010

Co-Authored-By: Edward Oakes <ed.nmi.oakes@gmail.com>

Update src/ray/protobuf/core_worker.proto

31fae1a

Co-Authored-By: Edward Oakes <ed.nmi.oakes@gmail.com>

Update src/ray/protobuf/core_worker.proto

fd47662

Co-Authored-By: Edward Oakes <ed.nmi.oakes@gmail.com>

Update src/ray/core_worker/reference_count.h

7a2baab

Co-Authored-By: Edward Oakes <ed.nmi.oakes@gmail.com>

Update src/ray/core_worker/reference_count.h

42e55fc

Co-Authored-By: Edward Oakes <ed.nmi.oakes@gmail.com>

Update src/ray/core_worker/reference_count.cc

38bc04b

Co-Authored-By: Edward Oakes <ed.nmi.oakes@gmail.com>

Apply suggestions from code review

a5859ca

Co-Authored-By: Edward Oakes <ed.nmi.oakes@gmail.com>

update

e3b49ac

Merge remote-tracking branch 'upstream/master' into ref-counting

3c6eccd

Try to fix ::test_direct_call_serialized_id_eviction

9c679d9

edoakes mentioned this pull request Feb 19, 2020

Remove gc.collect() calls from reference counting tests #7218

Merged

3 tasks

stephanie-wang merged commit f76ce83 into ray-project:master Feb 19, 2020

stephanie-wang deleted the ref-counting branch February 19, 2020 02:23

stephanie-wang mentioned this pull request Feb 19, 2020

[core] Ref counting for returning object IDs created by a different process #7221

Merged

3 tasks

stephanie-wang mentioned this pull request Feb 24, 2020

[core] Add long running regression test for distributed ref counting and fix memory leak #7302

Merged

3 tasks

stephanie-wang mentioned this pull request Mar 4, 2020

[core] Ref counting for actor handles #7434

Merged

3 tasks

Conversation

stephanie-wang commented Jan 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

Checks

Uh oh!

AmplabJenkins commented Jan 28, 2020

Uh oh!

AmplabJenkins commented Jan 29, 2020

Uh oh!

AmplabJenkins commented Feb 3, 2020

Uh oh!

AmplabJenkins commented Feb 4, 2020

Uh oh!

Uh oh!

Uh oh!

AmplabJenkins commented Feb 18, 2020

Uh oh!

AmplabJenkins commented Feb 18, 2020

Uh oh!

AmplabJenkins commented Feb 18, 2020

Uh oh!

AmplabJenkins commented Feb 18, 2020

Uh oh!

AmplabJenkins commented Feb 18, 2020

Uh oh!

AmplabJenkins commented Feb 19, 2020

Uh oh!

stephanie-wang commented Feb 19, 2020

Uh oh!

kfstorm commented Feb 19, 2020

Uh oh!

stephanie-wang commented Feb 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

stephanie-wang commented Jan 28, 2020 •

edited

Loading