Raylet events subscription support #3557

suquark · 2018-12-17T11:09:38Z

Related issue number

Changes include:

Implement raylet events subscription. (https://docs.google.com/document/d/11WJ6eQs_IC_Ne1VUjf7Tx8KQaPbD4aR7ewom6XiTdRA/edit?usp=sharing)
Introduce a new socket called raylet_events.
Implemented SimpleConnection for raylet client. We can replace the legacy io.h methods with this.
Reuse RegisterClientReply to sync RegisterClientRequest. RegisterClientReply was slightly modified to remove the legacy gpu_ids.
Implemented RayletEventTransport. Now async_api can be initialized synchronously.
Use new RayletEventProtocol to replace the old PlasmaProtocol. Improve performance using bytearray.

suquark · 2018-12-21T23:26:11Z

The CI test still failing on Linux builds (macos is ok). I cannot reproduce it in local machine/ubuntu docker. When shutting down a pytest, it will print

*** Error in `python -m pytest -v --durations=10 python/ray/tune/test/trial_scheduler_test.py': corrupted size vs. prev_size: 0x000055aaeebc32b0 ***
/home/travis/.travis/job_stages: line 104: 15098 Aborted                 (core dumped) python -m pytest -v --durations=10 python/ray/tune/test/trial_scheduler_test.py
The command "python -m pytest -v --durations=10 python/ray/tune/test/trial_scheduler_test.py" exited with 134.

All failing tests are related to rllibs & raytune, and I cannot find where my implementation includes something could also cause a memory leak. So I suspect that if there is something wrong in rllibs & raytune.

richardliaw · 2018-12-22T01:01:47Z

Is there a longer stack trace? Maybe temporary use -s on the pytest

suquark · 2018-12-22T01:08:18Z

OK. I have temporarily changed the pytest. It could take some time to see the results.

suquark · 2019-01-01T05:26:28Z

I squashed these commits because there were too many conflicts between this branch and upstream/master due to the interface change.

AmplabJenkins · 2019-01-02T10:08:35Z

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

AmplabJenkins · 2019-01-02T11:55:20Z

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

AmplabJenkins · 2019-01-02T12:47:40Z

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

suquark · 2019-01-03T02:41:22Z

I have removed some jenkins comments since related commits have been squashed.

AmplabJenkins · 2019-01-03T04:19:44Z

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

AmplabJenkins · 2019-01-07T07:36:34Z

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

AmplabJenkins · 2019-01-07T10:25:25Z

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

AmplabJenkins · 2019-01-07T10:32:03Z

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

AmplabJenkins · 2019-01-07T10:48:06Z

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

AmplabJenkins · 2019-01-07T16:07:16Z

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

robertnishihara · 2019-01-07T22:51:48Z

.travis.yml

I read through the TF issue, but it's not clear to me why we need this. What goes wrong without it?

I will get something like

*** Error in `python -m pytest -svvv python/ray/tune/test/ray_trial_executor_test.py -k test_start_stopu': corrupted size vs. prev_size: 0x000055bd463f1f00 *** Fatal Python error: Aborted Stack (most recent call first): /home/travis/.travis/job_stages: line 104: 17219 Aborted (core dumped) python -m pytest -svvv python/ray/tune/test/ray_trial_executor_test.py -k test_start_stopu The command "python -m pytest -svvv python/ray/tune/test/ray_trial_executor_test.py -k test_start_stopu" exited with 134.

when exiting a pytest like test_start_stopu.

The test test_start_stopu only includes:

def test_start_stopu(): import ray.rllib.models import scipy.signal ray.init() ray.shutdown()

It only happens when:

Use trusty linux under travis CI

Use both tensorflow and scipy.signal

I have checked my code and profiled with valgrind but I cannot see any memory leaks in my code, so I suppose it could come from tensorflow. Use libtcmalloc-minimal4 in the tensorflow issue solves this problem.

python/ray/experimental/async_api.py

robertnishihara · 2019-01-07T23:08:03Z

src/ray/raylet/lib/python/common_extension.cc

assuming #3674 is merged soon, we should remove this and instead use ray.ObjectID.from_random().

src/ray/raylet/lib/python/raylet_extension.cc

python/ray/experimental/test/async_test.py

src/ray/raylet/event/events.h

src/ray/common/client_connection.h

src/ray/raylet/node_manager.cc

robertnishihara · 2019-01-08T00:34:39Z

src/ray/raylet/event/events.h

I don't understand all of this graph terminology. What is an "edge"? Does the node manager really need anything more complex than a mapping from raylet client to a set of ObjectIDs that that client is subscribed to?

suquark · 2019-01-08T16:50:06Z

I have rebased this branch due to upstream updates. It could take me sometime to simplify or remove the "graph" stuff. It seems to be a excessive abstraction.

AmplabJenkins · 2019-01-08T18:52:55Z

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

AmplabJenkins · 2019-01-08T20:03:33Z

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

event socket support integrate events manager into node manager maintaining local objects Unsubscribe events on finishing create new API use LocalClientConnection more robust connection receiving events from the event socket use C++ random ID & use async write Make API init sync. Fix bugs. add tests Implement `unsubscribe` cleanup files cleanup python code use bytearray for message buffering fix memory leak remove unused headers fix client table; use generated flatbuffer config remove comment fix tempfile test implement a simple client enable futures return ObjectIDs only improve event transport remove legacy code fix java fix doc

suquark · 2019-01-09T10:43:01Z

@robertnishihara I have rethought about the graph terminology. My conclusion is that it is still necessary for more general event processing. Under the event processing context, what I need to do is:

The interface for event subscription & cancelling. That's the RayletEvents class.
Classes to manage different kind of events (so later we could properly implement a panel in our webui to watch these events). That's the subclasses of EventsManager.
The classes represent the concrete event, that's the subclasses of EventNode.
The classes represent the dependency/interaction of events, that's the subclasses of EventEdge.

These classes are necessary, and we cannot replace them with functions. This is because we need to know which events to finish(typically a top-down process) and which events to cancel (typically a bottom-up process). This requires class instances (lambda closures won't work here because of their lifetime).

Another benefit of the graph representation is that we only need to focus on the local stuff. For example, if we need to create a chain of events that one event is triggered by another, we only need to create the last event so that the whole chain will be built based on the graph dependency. This will happen when we move more components into the event module (like ray.wait, ray.get, Pull).

The last but not the least, the graph representation allows us to use multiple threads to process events in the future (parallel graph walking).

Some details are included in
https://docs.google.com/document/d/11WJ6eQs_IC_Ne1VUjf7Tx8KQaPbD4aR7ewom6XiTdRA/edit?usp=sharing

I am also waiting for #3674 to use the new ObjectID interface.

AmplabJenkins · 2019-01-09T12:07:11Z

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

AmplabJenkins · 2019-03-11T07:10:59Z

Can one of the admins verify this patch?

AmplabJenkins · 2019-03-11T21:49:57Z

Can one of the admins verify this patch?

AmplabJenkins · 2019-10-08T19:22:09Z

Can one of the admins verify this patch?

suquark changed the title ~~Raylet events subscription support~~ [WIP] Raylet events subscription support Dec 17, 2018

suquark self-assigned this Dec 18, 2018

suquark changed the title ~~[WIP] Raylet events subscription support~~ Raylet events subscription support Dec 19, 2018

suquark force-pushed the subscription branch from 718b982 to cc80408 Compare December 21, 2018 11:48

suquark requested review from atumanov, pcmoritz and robertnishihara December 21, 2018 23:19

suquark mentioned this pull request Dec 22, 2018

[WIP][DO NOT MERGE] Debug memory leaks #3606

Closed

suquark force-pushed the subscription branch 3 times, most recently from d23fe22 to 6da980e Compare January 1, 2019 05:22

suquark force-pushed the subscription branch from 6da980e to abdf1b1 Compare January 1, 2019 05:31

ray-project deleted a comment from AmplabJenkins Jan 3, 2019

suquark force-pushed the subscription branch 2 times, most recently from 1438e6f to 2d6499c Compare January 7, 2019 08:57

suquark force-pushed the subscription branch from 2d6499c to ff4b4c7 Compare January 7, 2019 13:36

robertnishihara reviewed Jan 8, 2019

View reviewed changes

suquark force-pushed the subscription branch from ff4b4c7 to 47a6d0f Compare January 8, 2019 16:28

suquark added 2 commits January 9, 2019 17:23

Free event clients properly

5d7d7dd

suquark force-pushed the subscription branch from 7b494d1 to 5d7d7dd Compare January 9, 2019 09:30

suquark mentioned this pull request Jan 15, 2019

WIP: Replace legacy socket io code #3784

Closed

stephanie-wang assigned robertnishihara and unassigned suquark Jan 30, 2019

ericl closed this Dec 29, 2019

suquark deleted the subscription branch July 9, 2020 18:28

Raylet events subscription support #3557

Raylet events subscription support #3557

Uh oh!

Conversation

suquark commented Dec 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related issue number

Uh oh!

suquark commented Dec 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richardliaw commented Dec 22, 2018

Uh oh!

suquark commented Dec 22, 2018

Uh oh!

suquark commented Jan 1, 2019

Uh oh!

AmplabJenkins commented Jan 2, 2019

Uh oh!

AmplabJenkins commented Jan 2, 2019

Uh oh!

AmplabJenkins commented Jan 2, 2019

Uh oh!

suquark commented Jan 3, 2019

Uh oh!

AmplabJenkins commented Jan 3, 2019

Uh oh!

AmplabJenkins commented Jan 7, 2019

Uh oh!

AmplabJenkins commented Jan 7, 2019

Uh oh!

AmplabJenkins commented Jan 7, 2019

Uh oh!

AmplabJenkins commented Jan 7, 2019

Uh oh!

AmplabJenkins commented Jan 7, 2019

Uh oh!

robertnishihara Jan 7, 2019

Choose a reason for hiding this comment

Uh oh!

suquark Jan 8, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robertnishihara Jan 7, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

robertnishihara Jan 8, 2019

Choose a reason for hiding this comment

Uh oh!

suquark commented Jan 8, 2019

Uh oh!

AmplabJenkins commented Jan 8, 2019

Uh oh!

AmplabJenkins commented Jan 8, 2019

Uh oh!

suquark commented Jan 9, 2019

Uh oh!

AmplabJenkins commented Jan 9, 2019

Uh oh!

AmplabJenkins commented Mar 11, 2019

Uh oh!

AmplabJenkins commented Mar 11, 2019

Uh oh!

AmplabJenkins commented Oct 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

suquark commented Dec 17, 2018 •

edited

Loading

suquark commented Dec 21, 2018 •

edited

Loading