[xray] Monitor for Raylet processes #1831

stephanie-wang · 2018-04-04T23:25:02Z

What do these changes do?

This adds a C++ monitor, similar to monitor.py that listens for heartbeats from Raylet processes through the GCS. Raylet processes that do not respond within a timeout are marked as dead.

robertnishihara · 2018-04-04T23:42:41Z

python/ray/services.py

@@ -1112,11 +1115,35 @@ def start_monitor(redis_address, node_ip_address, stdout_file=None,
        command.append("--autoscaling-config=" + str(autoscaling_config))
    p = subprocess.Popen(command, stdout=stdout_file, stderr=stderr_file)
    if cleanup:
-        all_processes[PROCESS_TYPE_WORKER].append(p)
+        all_processes[PROCESS_TYPE_MONITOR].append(p)


nice catch!

robertnishihara · 2018-04-04T23:44:29Z

python/ray/services.py

+# Location of the raylet executables.
+RAYLET_MONITOR_EXECUTABLE = os.path.join(
+    os.path.abspath(os.path.dirname(__file__)),
+    "core/src/ray/raylet/raylet_monitor")


I believe the executable also needs to be added to the ray_files = [ list in python/setup.py.

robertnishihara · 2018-04-04T23:45:46Z

src/ray/gcs/tables.cc

  };
  return Subscribe(JobID::nil(), client_id_, notification_callback,
                   subscription_callback);
 }

 Status ClientTable::Disconnect() {
  auto data = std::make_shared<ClientTableDataT>(local_client_);
-  data->is_insertion = true;
+  data->is_insertion = false;


Hmm this was a bug right?

Yeah, apparently this path wasn't covered in the tests :( I can add a regression unit test.

robertnishihara · 2018-04-04T23:46:30Z

src/ray/gcs/tables.h

@@ -435,6 +435,10 @@ class ClientTable : private Log<UniqueID, ClientTableData> {
  /// \return Status
  ray::Status Disconnect();

+  /// Mark a different client as disconnected. The client ID should never be
+  /// reused for a new client.
+  ray::Status MarkDisconnected(const ClientID &dead_client_id);


Can we include the \param and \return?

robertnishihara

Looks good to me. I left some comments.

robertnishihara · 2018-04-04T23:47:19Z

src/ray/raylet/monitor.h

+
+class Monitor {
+ public:
+  // Create a Raylet monitor attached to the given GCS address and port.


// -> /// for some places in this file, right?

Ah sorry, thanks!

robertnishihara · 2018-04-04T23:48:07Z

src/ray/raylet/monitor.h

+
+namespace raylet {
+
+class Monitor {


could we include some documentation about the purpose of this class?

robertnishihara · 2018-04-04T23:51:21Z

src/ray/raylet/monitor.h

+  gcs::AsyncGcsClient gcs_client_;
+  int64_t heartbeat_timeout_;
+  boost::asio::deadline_timer heartbeat_timer_;
+  std::unordered_map<ClientID, int64_t, UniqueIDHasher> heartbeats_;


Can you document the interpretation of the int64_t?

robertnishihara · 2018-04-04T23:52:28Z

src/ray/raylet/monitor.cc

+    it->second--;
+    if (it->second == 0) {
+      if (dead_clients_.count(it->first) == 0) {
+        RAY_LOG(WARNING) << "Client timed out: " << it->first.hex();


just it->first (the hex() happens automatically)

robertnishihara · 2018-04-04T23:55:48Z

python/ray/services.py

+        if use_raylet:
+            start_raylet_monitor(redis_address,
+                                 stdout_file=monitor_stdout_file,
+                                 stderr_file=monitor_stderr_file,


do we want these to go to the same log files as monitor.py or to their own log file?

I think it makes sense to go to the same file as monitor.py, since only one of the monitors should ever be active at a given time (except for the autoscaler part of monitor.py).

AmplabJenkins · 2018-04-04T23:56:50Z

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

AmplabJenkins · 2018-04-05T01:16:54Z

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

robertnishihara · 2018-04-05T22:20:13Z

This test failure looks potentially related.

$ python test/monitor_test.py

testCleanupOnDriverExitManyRedisShards (__main__.MonitorTest) ... ok

testCleanupOnDriverExitSingleRedisShard (__main__.MonitorTest) ... Process Process-3:

Traceback (most recent call last):

  File "/Users/travis/miniconda/lib/python2.7/multiprocessing/process.py", line 267, in _bootstrap

    self.run()

  File "/Users/travis/miniconda/lib/python2.7/multiprocessing/process.py", line 114, in run

    self._target(*self._args, **self._kwargs)

  File "test/monitor_test.py", line 47, in Driver

    if (2, 1, summary_start[2]) != StateSummary():

  File "test/monitor_test.py", line 31, in StateSummary

    obj_tbl_len = len(ray.global_state.object_table())

  File "/Users/travis/.local/lib/python2.7/site-packages/ray-0.4.0-py2.7-macosx-10.6-x86_64.egg/ray/experimental/state.py", line 236, in object_table

    self._object_table(binary_to_object_id(object_id_binary)))

  File "/Users/travis/.local/lib/python2.7/site-packages/ray-0.4.0-py2.7-macosx-10.6-x86_64.egg/ray/experimental/state.py", line 200, in _object_table

    result_table_response, 0)

  File "/Users/travis/.local/lib/python2.7/site-packages/ray-0.4.0-py2.7-macosx-10.6-x86_64.egg/ray/core/generated/ResultTableReply.py", line 12, in GetRootAsResultTableReply

    n = flatbuffers.encode.Get(flatbuffers.packer.uoffset, buf, offset)

  File "/Users/travis/.local/lib/python2.7/site-packages/flatbuffers-2015.12.22.1-py2.7.egg/flatbuffers/encode.py", line 24, in Get

    return packer_type.unpack_from(memoryview_type(buf), head)[0]

TypeError: cannot make memory view because object does not have the buffer interface

FAIL



======================================================================

FAIL: testCleanupOnDriverExitSingleRedisShard (__main__.MonitorTest)

----------------------------------------------------------------------

Traceback (most recent call last):

  File "test/monitor_test.py", line 91, in testCleanupOnDriverExitSingleRedisShard

    self._testCleanupOnDriverExit(num_redis_shards=1)

  File "test/monitor_test.py", line 82, in _testCleanupOnDriverExit

    self.assertEqual((0, 1), StateSummary()[:2])

AssertionError: Tuples differ: (0, 1) != (2, 2)



First differing element 0:

0

2



- (0, 1)

+ (2, 2)



----------------------------------------------------------------------

Ran 2 tests in 30.064s



FAILED (failures=1)

stephanie-wang · 2018-04-05T22:39:58Z

Hmm, I'm sort of confused by this error because I don't think adding the new C++ monitor should mess with the original one (especially when the use_raylet flag isn't turned on). That error looks like it may be because there's an object in the object table, but no corresponding result table entry?

robertnishihara · 2018-04-05T22:48:08Z

Ah, looks like the error was already reported in #1332.

AmplabJenkins · 2018-04-05T23:16:35Z

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

stephanie-wang added 3 commits April 4, 2018 15:49

Add raylet monitor script to timeout Raylet heartbeats

1cd014d

Unit test for removing a different client from the client table

041dc86

Set node manager heartbeat according to global config

1984f45

stephanie-wang requested a review from robertnishihara April 4, 2018 23:28

robertnishihara reviewed Apr 4, 2018

View reviewed changes

stephanie-wang added 2 commits April 4, 2018 17:09

Doc and fixes

c515301

Add regression test for client table disconnect, refactor client table

1801b31

Fix linting.

ef0b180

robertnishihara mentioned this pull request Apr 5, 2018

Test failure in monitor_test.py::test_cleanup_on_driver_exit_many_redis_shards. #1332

Closed

robertnishihara approved these changes Apr 6, 2018

View reviewed changes

robertnishihara merged commit cbf3181 into ray-project:master Apr 6, 2018

robertnishihara deleted the xray-monitor branch April 6, 2018 03:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[xray] Monitor for Raylet processes #1831

[xray] Monitor for Raylet processes #1831

stephanie-wang commented Apr 4, 2018

robertnishihara Apr 4, 2018

robertnishihara Apr 4, 2018

stephanie-wang Apr 4, 2018

robertnishihara Apr 4, 2018

stephanie-wang Apr 4, 2018 •

edited

Loading

robertnishihara Apr 4, 2018

robertnishihara left a comment

robertnishihara Apr 4, 2018

stephanie-wang Apr 4, 2018

robertnishihara Apr 4, 2018

robertnishihara Apr 4, 2018

robertnishihara Apr 4, 2018

robertnishihara Apr 4, 2018

stephanie-wang Apr 5, 2018

AmplabJenkins commented Apr 4, 2018

AmplabJenkins commented Apr 5, 2018

robertnishihara commented Apr 5, 2018

stephanie-wang commented Apr 5, 2018

robertnishihara commented Apr 5, 2018

AmplabJenkins commented Apr 5, 2018

[xray] Monitor for Raylet processes #1831

[xray] Monitor for Raylet processes #1831

Conversation

stephanie-wang commented Apr 4, 2018

What do these changes do?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephanie-wang Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertnishihara left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented Apr 4, 2018

AmplabJenkins commented Apr 5, 2018

robertnishihara commented Apr 5, 2018

stephanie-wang commented Apr 5, 2018

robertnishihara commented Apr 5, 2018

AmplabJenkins commented Apr 5, 2018

stephanie-wang Apr 4, 2018 •

edited

Loading