Fix multi-thread problem of function manager and Jenkins test #3648

guoyuhong · 2018-12-27T09:52:55Z

What do these changes do?

The Jenkins test mnist_example.py uses multi-thread in actor and we found frequent Jenkins test failures recently.

Pattern 1

Pattern 2

Pattern 1 is caused by NIL driver ID in multi-thread actor exporting. In this case, exporting is called before worker.task_driver_id is set in main thread. Finally, load_actor won't get the right actor key from GCS.

File "/usr/local/lib/python3.6/threading.py", line 884, in _bootstrap
    self._bootstrap_inner()
File "/usr/local/lib/python3.6/threading.py", line 916, in _bootstrap_inner
    self.run()
File "/home/admin/yuhong/ray/python/ray/tune/function_runner.py", line 80, in run
    self._entrypoint(*self._entrypoint_args)
File "python/ray/experimental/sgd/mnist_example.py", line 112, in train_mnist
    strategy=args.strategy)
File "/home/admin/yuhong/ray/python/ray/experimental/sgd/sgd.py", line 106, in __init__
    all_reduce_alg=all_reduce_alg))
File "/home/admin/yuhong/ray/python/ray/actor.py", line 329, in remote
    return self._remote(args=args, kwargs=kwargs)
File "/home/admin/yuhong/ray/python/ray/actor.py", line 392, in _remote
    self._checkpoint_interval)
File "/home/admin/yuhong/ray/python/ray/function_manager.py", line 520, in export_actor_class
    for line in traceback.format_stack():
 export_actor_class: FunctionDescriptor:ray.experimental.sgd.sgd_worker.SGDWorker.__init__., driver id: ObjectID(fffffffffffffffffffffffffffffffffffffff)

Pattern 2 is because that worker.task_driver_id becomes NIL in _publish_actor_class_to_key. The actor key is right, but driver_id information is not correct. Actually, in this case, worker continues with following failures, but push_error_to_driver suppresses this error message and the task is reconstructed indefinitely.

Traceback (most recent call last):
  File "/home/admin/yuhong/ray/python/ray/workers/default_worker.py", line 106, in <module>
    ray.worker.global_worker.main_loop()
  File "/home/admin/yuhong/ray/python/ray/worker.py", line 963, in main_loop
    self._wait_for_and_process_task(task)
  File "/home/admin/yuhong/ray/python/ray/worker.py", line 891, in _wait_for_and_process_task
    driver_id, function_descriptor)
  File "/home/admin/yuhong/ray/python/ray/function_manager.py", line 453, in get_execution_info
    raise KeyError(message)
KeyError: "Error occurs in get_execution_info: driver_id: 44b1341be3f9ee0bdfb8d249f82eaa4e9cab6930, function_descriptor: FunctionDescriptor:ray.experimental.sgd.sgd_worker.SGDWorker.__init__.. Message: b'j\\xceZ\\xbd\\xba\\x98\\xf0\\x91\\xc6\\xa7m\\x80|\\xfd\\x16m$\\x94\\xf6\\xac'"

In this PR, the following changes are included,

The function manager will wait the driver ID and driver ID will be kept as soon as it is not NIL.
actor_class_info will has the driver_id item at the beginning. Otherwise, if actor_class_info is put to _actors_to_export the driver_id infor will be missing.
Add error log to push_error_to_driver.

Related issue number

N/A

raulchen · 2018-12-27T10:15:45Z

python/ray/function_manager.py

Assume I have an actor that only creates background threads to do work. Its main thread never receives tasks.
Sleeping here will block forever.
Instead of sleeping here, I think an easier solution is not resetting task_driver_id for actors. Because:

If a worker is an actor, all the tasks should have the same driver id.

If I have a background thread whose lifecycle expands across multiple main-thread tasks, the worker must be an actor. (Normal tasks shouldn't create a thread whose lifecycle is longer than itself. We can raise an error in this case.)

That sounds right. If this is happening on an actor, then we should be able to figure out the correct driver ID. If it's happening on a non-actor worker, then we should raise an error.

AmplabJenkins · 2018-12-27T12:20: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/10446/
Test PASSed.

guoyuhong · 2018-12-29T09:59:41Z

@raulchen Thanks for the suggestion! I have added the logic to this PR. It also looks like that your test script in #3651 has also passed. Shall I add this script to runtest.py or you will have another PR to refine the multi-thread problem and add the test case later?

raulchen

@guoyuhong thanks! I'll do that in another PR. Something else needs to be fixed as well.

raulchen · 2018-12-29T11:21:07Z

python/ray/function_manager.py

this sleep should be removed now?
I think we can assert driver id != nil here.

Logically, yes. When a actor started another thread, worker.task_driver_id will not be nil.

python/ray/worker.py

AmplabJenkins · 2018-12-29T11:46: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/10497/
Test FAILed.

AmplabJenkins · 2019-01-02T06:36:23Z

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

guoyuhong · 2019-01-02T07:39:56Z

This ObjectID refactor code is put to #3674 .

raulchen · 2019-01-02T08:36:36Z

python/ray/function_manager.py

+        if (self._worker.task_driver_id.is_nil()):
+            logger.warning("export_actor_class with NIL task_driver_id, this "
+                           "may happen when export_actor_class runs not in "
+                           "the main thread. Will wait for the driver id.")


This warning message needs to be changed as well?

raulchen · 2019-01-02T08:37:25Z

python/ray/utils.py

    if driver_id is None:
        driver_id = ray_constants.NIL_JOB_ID.id()
    data = {} if data is None else data
+    logging.error("push_error_to_driver with message: %s" % message)


also print error_type?

AmplabJenkins · 2019-01-02T11:56:13Z

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

AmplabJenkins · 2019-01-02T13:31:49Z

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

guoyuhong requested review from ericl, raulchen and robertnishihara December 27, 2018 09:53

guoyuhong mentioned this pull request Dec 27, 2018

Add RayParams to refactor the parameters used by ray python. #3558

Merged

raulchen reviewed Dec 27, 2018

View reviewed changes

raulchen mentioned this pull request Dec 27, 2018

Issue when calling a remote function in a background thread #3651

Closed

guoyuhong force-pushed the fixMultithreadFM branch from 1fe0995 to 44e1614 Compare December 29, 2018 09:56

raulchen reviewed Dec 29, 2018

View reviewed changes

Yuhong Guo added 3 commits January 2, 2019 11:33

Fix multi-thread problem of function manager.

7c5e2fe

Avoid changing task_driver_id in actor

f1d8fa2

Remove sleep in function manager.

0f2f852

guoyuhong force-pushed the fixMultithreadFM branch from 44e1614 to 0f2f852 Compare January 2, 2019 04:21

raulchen reviewed Jan 2, 2019

View reviewed changes

Yuhong Guo and others added 2 commits January 2, 2019 17:46

Fix

c880839

small changes

768e554

raulchen approved these changes Jan 2, 2019

View reviewed changes

robertnishihara approved these changes Jan 3, 2019

View reviewed changes

raulchen merged commit 4b23a34 into ray-project:master Jan 3, 2019

Fix multi-thread problem of function manager and Jenkins test #3648

Fix multi-thread problem of function manager and Jenkins test #3648

Uh oh!

Conversation

guoyuhong commented Dec 27, 2018

What do these changes do?

Related issue number

Uh oh!

raulchen Dec 27, 2018

Choose a reason for hiding this comment

Uh oh!

robertnishihara Dec 27, 2018

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Dec 27, 2018

Uh oh!

guoyuhong commented Dec 29, 2018

Uh oh!

raulchen left a comment

Choose a reason for hiding this comment

Uh oh!

raulchen Dec 29, 2018

Choose a reason for hiding this comment

Uh oh!

guoyuhong Jan 2, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AmplabJenkins commented Dec 29, 2018

Uh oh!

AmplabJenkins commented Jan 2, 2019

Uh oh!

guoyuhong commented Jan 2, 2019

Uh oh!

raulchen Jan 2, 2019

Choose a reason for hiding this comment

Uh oh!

raulchen Jan 2, 2019

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Jan 2, 2019

Uh oh!

AmplabJenkins commented Jan 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants