Fixes and cleanups for the multinode setting. (#143)

* Add function for driver to get address info from Redis.

* Use Redis address instead of Redis port.

* Configure Redis to run in unprotected mode.

* Add method for starting Ray processes on non-head node.

* Pass in correct node ip address to start_plasma_manager.

* Script for starting Ray processes.

* Handle the case where an object already exists in the store. Maybe this should also compare the object hashes.

* Have driver get info from Redis when start_ray_local=False.

* Fix.

* Script for killing ray processes.

* Catch some errors when the main_loop in a worker throws an exception.

* Allow redirecting stdout and stderr to /dev/null.

* Wrap start_ray.py in a shell script.

* More helpful error messages.

* Fixes.

* Wait for redis server to start up before configuring it.

* Allow seeding of deterministic object ID generation.

* Small change.

src/common/redis_module/ray_redis_module.c

@@ -61,14 +61,25 @@ RedisModuleKey *OpenPrefixedKey(RedisModuleCtx *ctx,
 /**
  * Register a client with Redis. This is called from a client with the command:
  *
- *     RAY.CONNECT <client type> <address> <ray client id> <aux address>
+ *     RAY.CONNECT <ray client id> <node ip address> <client type> <field 1>
+ *         <value 1> <field 2> <value 2> ...
+ *
+ * The command can take an arbitrary number of pairs of field names and keys,
+ * and these will be stored in a hashmap associated with this client. Several
+ * fields are singled out for special treatment:
+ *
+ *     address: This is provided by plasma managers and it should be an address
+ *         like "127.0.0.1:1234". It is returned by RAY.GET_CLIENT_ADDRESS so
+ *         that other plasma managers know how to fetch objects.
+ *     aux_address: This is provided by local schedulers and should be the
+ *         address of the plasma manager that the local scheduler is connected
+ *         to. This is published to the "db_clients" channel by the RAY.CONNECT
+ *         command and is used by the global scheduler to determine which plasma
+ *         managers and local schedulers are connected.
  *
- * @param client_type The type of the client (e.g., plasma_manager).
- * @param address The address of the client.
  * @param ray_client_id The db client ID of the client.
- * @param aux_address An auxiliary address. This is currently just used by the
- *        local scheduler to record the address of the plasma manager that it is
- *        connected to.
+ * @param node_ip_address The IP address of the node the client is on.
+ * @param client_type The type of the client (e.g., plasma_manager).
  * @return OK if the operation was successful.
  */
 int Connect_RedisCommand(RedisModuleCtx *ctx,
@@ -95,7 +106,8 @@ int Connect_RedisCommand(RedisModuleCtx *ctx,
       RedisModule_CreateString(ctx, "aux_address", strlen("aux_address"));
 
   RedisModule_HashSet(db_client_table_key, REDISMODULE_HASH_CFIELDS,
-                      "node_ip_address", node_ip_address, NULL);
+                      "ray_client_id", ray_client_id, "node_ip_address",
+                      node_ip_address, "client_type", client_type, NULL);
 
   for (int i = 4; i < argc; i += 2) {
     RedisModuleString *key = argv[i];

src/common/state/redis.c

@@ -160,11 +160,19 @@ db_handle *db_connect(const char *db_address,
                       "could not establish synchronous connection to redis "
                       "%s:%d",
                       db_address, db_port);
-  /* Enable keyspace events. */
-  reply = redisCommand(context, "CONFIG SET notify-keyspace-events AKE");
+  /* Configure Redis to generate keyspace notifications for list events. This
+   * should only need to be done once (by whoever started Redis), but since
+   * Redis may be started in multiple places (e.g., for testing or when starting
+   * processes by hand), it is easier to do it multiple times. */
+  reply = redisCommand(context, "CONFIG SET notify-keyspace-events Kl");
   CHECKM(reply != NULL, "db_connect failed on CONFIG SET");
   freeReplyObject(reply);
-  /* Add new client using optimistic locking. */
+  /* Also configure Redis to not run in protected mode, so clients on other
+   * hosts can connect to it. */
+  reply = redisCommand(context, "CONFIG SET protected-mode no");
+  CHECKM(reply != NULL, "db_connect failed on CONFIG SET");
+  freeReplyObject(reply);
+  /* Create a client ID for this client. */
   db_client_id client = globally_unique_id();
 
   /* Construct the argument arrays for RAY.CONNECT. */

src/global_scheduler/lib/python/global_scheduler_services.py

@@ -6,7 +6,7 @@ import os
 import subprocess
 import time
 
-def start_global_scheduler(redis_address, use_valgrind=False, use_profiler=False):
+def start_global_scheduler(redis_address, use_valgrind=False, use_profiler=False, redirect_output=False):
   """Start a global scheduler process.
 
   Args:
@@ -15,6 +15,8 @@ def start_global_scheduler(redis_address, use_valgrind=False, use_profiler=False
       of valgrind. If this is True, use_profiler must be False.
     use_profiler (bool): True if the global scheduler should be started inside a
       profiler. If this is True, use_valgrind must be False.
+    redirect_output (bool): True if stdout and stderr should be redirected to
+      /dev/null.
 
   Return:
     The process ID of the global scheduler process.
@@ -23,13 +25,16 @@ def start_global_scheduler(redis_address, use_valgrind=False, use_profiler=False
     raise Exception("Cannot use valgrind and profiler at the same time.")
   global_scheduler_executable = os.path.join(os.path.abspath(os.path.dirname(__file__)), "../../build/global_scheduler")
   command = [global_scheduler_executable, "-r", redis_address]
-  if use_valgrind:
-    pid = subprocess.Popen(["valgrind", "--track-origins=yes", "--leak-check=full", "--show-leak-kinds=all", "--error-exitcode=1"] + command)
-    time.sleep(1.0)
-  elif use_profiler:
-    pid = subprocess.Popen(["valgrind", "--tool=callgrind"] + command)
-    time.sleep(1.0)
-  else:
-    pid = subprocess.Popen(command)
-    time.sleep(0.1)
+  with open(os.devnull, "w") as FNULL:
+    stdout = FNULL if redirect_output else None
+    stderr = FNULL if redirect_output else None
+    if use_valgrind:
+      pid = subprocess.Popen(["valgrind", "--track-origins=yes", "--leak-check=full", "--show-leak-kinds=all", "--error-exitcode=1"] + command, stdout=stdout, stderr=stderr)
+      time.sleep(1.0)
+    elif use_profiler:
+      pid = subprocess.Popen(["valgrind", "--tool=callgrind"] + command, stdout=stdout, stderr=stderr)
+      time.sleep(1.0)
+    else:
+      pid = subprocess.Popen(command, stdout=stdout, stderr=stderr)
+      time.sleep(0.1)
   return pid

src/photon/photon/photon_services.py

@@ -10,7 +10,7 @@ import time
 def random_name():
   return str(random.randint(0, 99999999))
 
-def start_local_scheduler(plasma_store_name, plasma_manager_name=None, plasma_address=None, node_ip_address="127.0.0.1", redis_address=None, use_valgrind=False, use_profiler=False):
+def start_local_scheduler(plasma_store_name, plasma_manager_name=None, plasma_address=None, node_ip_address="127.0.0.1", redis_address=None, use_valgrind=False, use_profiler=False, redirect_output=False):
   """Start a local scheduler process.
 
   Args:
@@ -29,6 +29,8 @@ def start_local_scheduler(plasma_store_name, plasma_manager_name=None, plasma_ad
       valgrind. If this is True, use_profiler must be False.
     use_profiler (bool): True if the local scheduler should be started inside a
       profiler. If this is True, use_valgrind must be False.
+    redirect_output (bool): True if stdout and stderr should be redirected to
+      /dev/null.
 
   Return:
     A tuple of the name of the local scheduler socket and the process ID of the
@@ -47,13 +49,16 @@ def start_local_scheduler(plasma_store_name, plasma_manager_name=None, plasma_ad
     command += ["-r", redis_address]
   if plasma_address is not None:
     command += ["-a", plasma_address]
-  if use_valgrind:
-    pid = subprocess.Popen(["valgrind", "--track-origins=yes", "--leak-check=full", "--show-leak-kinds=all", "--error-exitcode=1"] + command)
-    time.sleep(1.0)
-  elif use_profiler:
-    pid = subprocess.Popen(["valgrind", "--tool=callgrind"] + command)
-    time.sleep(1.0)
-  else:
-    pid = subprocess.Popen(command)
-    time.sleep(0.1)
+  with open(os.devnull, "w") as FNULL:
+    stdout = FNULL if redirect_output else None
+    stderr = FNULL if redirect_output else None
+    if use_valgrind:
+      pid = subprocess.Popen(["valgrind", "--track-origins=yes", "--leak-check=full", "--show-leak-kinds=all", "--error-exitcode=1"] + command, stdout=stdout, stderr=stderr)
+      time.sleep(1.0)
+    elif use_profiler:
+      pid = subprocess.Popen(["valgrind", "--tool=callgrind"] + command, stdout=stdout, stderr=stderr)
+      time.sleep(1.0)
+    else:
+      pid = subprocess.Popen(command, stdout=stdout, stderr=stderr)
+      time.sleep(0.1)
   return local_scheduler_name, pid

src/photon/photon_scheduler.c

@@ -29,6 +29,7 @@ local_scheduler_state *init_local_scheduler(
     event_loop *loop,
     const char *redis_addr,
     int redis_port,
+    const char *local_scheduler_socket_name,
     const char *plasma_store_socket_name,
     const char *plasma_manager_socket_name,
     const char *plasma_manager_address,
@@ -40,19 +41,24 @@ local_scheduler_state *init_local_scheduler(
   utarray_new(state->workers, &worker_icd);
   /* Connect to Redis if a Redis address is provided. */
   if (redis_addr != NULL) {
-    int num_args = 0;
+    int num_args;
     const char **db_connect_args = NULL;
     if (plasma_manager_address != NULL) {
+      num_args = 4;
+      db_connect_args = malloc(sizeof(char *) * num_args);
+      db_connect_args[0] = "local_scheduler_socket_name";
+      db_connect_args[1] = local_scheduler_socket_name;
+      db_connect_args[2] = "aux_address";
+      db_connect_args[3] = plasma_manager_address;
+    } else {
       num_args = 2;
       db_connect_args = malloc(sizeof(char *) * num_args);
-      db_connect_args[0] = "aux_address";
-      db_connect_args[1] = plasma_manager_address;
+      db_connect_args[0] = "local_scheduler_socket_name";
+      db_connect_args[1] = local_scheduler_socket_name;
     }
     state->db = db_connect(redis_addr, redis_port, "photon", node_ip_address,
                            num_args, db_connect_args);
-    if (num_args != 0) {
-      free(db_connect_args);
-    };
+    free(db_connect_args);
     db_attach(state->db, loop, false);
   } else {
     state->db = NULL;
@@ -295,10 +301,10 @@ void start_server(const char *node_ip_address,
                   bool global_scheduler_exists) {
   int fd = bind_ipc_sock(socket_name, true);
   event_loop *loop = event_loop_create();
-  g_state =
-      init_local_scheduler(node_ip_address, loop, redis_addr, redis_port,
-                           plasma_store_socket_name, plasma_manager_socket_name,
-                           plasma_manager_address, global_scheduler_exists);
+  g_state = init_local_scheduler(
+      node_ip_address, loop, redis_addr, redis_port, socket_name,
+      plasma_store_socket_name, plasma_manager_socket_name,
+      plasma_manager_address, global_scheduler_exists);
 
   /* Register a callback for registering new clients. */
   event_loop_add_file(loop, fd, EVENT_LOOP_READ, new_client_connection,

src/photon/photon_scheduler.h

@@ -69,6 +69,7 @@ local_scheduler_state *init_local_scheduler(
     event_loop *loop,
     const char *redis_addr,
     int redis_port,
+    const char *local_scheduler_socket_name,
     const char *plasma_manager_socket_name,
     const char *plasma_store_socket_name,
     const char *plasma_manager_address,

src/photon/test/photon_tests.c

@@ -59,6 +59,7 @@ photon_mock *init_photon_mock() {
   CHECK(mock->plasma_fd >= 0 && mock->photon_fd >= 0);
   mock->photon_state = init_local_scheduler(
       "127.0.0.1", mock->loop, redis_addr, redis_port,
+      utstring_body(photon_socket_name),
       utstring_body(plasma_manager_socket_name),
       utstring_body(plasma_store_socket_name), NULL, false);
   /* Connect a Photon client. */

src/plasma/plasma/plasma.py

@@ -234,7 +234,7 @@ DEFAULT_PLASMA_STORE_MEMORY = 10 ** 9
 def random_name():
   return str(random.randint(0, 99999999))
 
-def start_plasma_store(plasma_store_memory=DEFAULT_PLASMA_STORE_MEMORY, use_valgrind=False, use_profiler=False):
+def start_plasma_store(plasma_store_memory=DEFAULT_PLASMA_STORE_MEMORY, use_valgrind=False, use_profiler=False, redirect_output=False):
   """Start a plasma store process.
 
   Args:
@@ -242,6 +242,8 @@ def start_plasma_store(plasma_store_memory=DEFAULT_PLASMA_STORE_MEMORY, use_valg
       valgrind. If this is True, use_profiler must be False.
     use_profiler (bool): True if the plasma store should be started inside a
       profiler. If this is True, use_valgrind must be False.
+    redirect_output (bool): True if stdout and stderr should be redirected to
+      /dev/null.
 
   Return:
     A tuple of the name of the plasma store socket and the process ID of the
@@ -252,25 +254,31 @@ def start_plasma_store(plasma_store_memory=DEFAULT_PLASMA_STORE_MEMORY, use_valg
   plasma_store_executable = os.path.join(os.path.abspath(os.path.dirname(__file__)), "plasma_store")
   plasma_store_name = "/tmp/plasma_store{}".format(random_name())
   command = [plasma_store_executable, "-s", plasma_store_name, "-m", str(plasma_store_memory)]
-  if use_valgrind:
-    pid = subprocess.Popen(["valgrind", "--track-origins=yes", "--leak-check=full", "--show-leak-kinds=all", "--error-exitcode=1"] + command)
-    time.sleep(1.0)
-  elif use_profiler:
-    pid = subprocess.Popen(["valgrind", "--tool=callgrind"] + command)
-    time.sleep(1.0)
-  else:
-    pid = subprocess.Popen(command)
-    time.sleep(0.1)
+  with open(os.devnull, "w") as FNULL:
+    stdout = FNULL if redirect_output else None
+    stderr = FNULL if redirect_output else None
+    if use_valgrind:
+      pid = subprocess.Popen(["valgrind", "--track-origins=yes", "--leak-check=full", "--show-leak-kinds=all", "--error-exitcode=1"] + command, stdout=stdout, stderr=stderr)
+      time.sleep(1.0)
+    elif use_profiler:
+      pid = subprocess.Popen(["valgrind", "--tool=callgrind"] + command, stdout=stdout, stderr=stderr)
+      time.sleep(1.0)
+    else:
+      pid = subprocess.Popen(command, stdout=stdout, stderr=stderr)
+      time.sleep(0.1)
   return plasma_store_name, pid
 
-def start_plasma_manager(store_name, redis_address, num_retries=20, use_valgrind=False, run_profiler=False):
+def start_plasma_manager(store_name, redis_address, node_ip_address="127.0.0.1", num_retries=20, use_valgrind=False, run_profiler=False, redirect_output=False):
   """Start a plasma manager and return the ports it listens on.
 
   Args:
     store_name (str): The name of the plasma store socket.
     redis_address (str): The address of the Redis server.
+    node_ip_address (str): The IP address of the node.
     use_valgrind (bool): True if the Plasma manager should be started inside of
       valgrind and False otherwise.
+    redirect_output (bool): True if stdout and stderr should be redirected to
+      /dev/null.
 
   Returns:
     A tuple of the Plasma manager socket name, the process ID of the Plasma
@@ -291,15 +299,18 @@ def start_plasma_manager(store_name, redis_address, num_retries=20, use_valgrind
     command = [plasma_manager_executable,
                "-s", store_name,
                "-m", plasma_manager_name,
-               "-h", "127.0.0.1",
+               "-h", node_ip_address,
                "-p", str(port),
                "-r", redis_address]
-    if use_valgrind:
-      process = subprocess.Popen(["valgrind", "--track-origins=yes", "--leak-check=full", "--show-leak-kinds=all", "--error-exitcode=1"] + command)
-    elif run_profiler:
-      process = subprocess.Popen(["valgrind", "--tool=callgrind"] + command)
-    else:
-      process = subprocess.Popen(command)
+    with open(os.devnull, "w") as FNULL:
+      stdout = FNULL if redirect_output else None
+      stderr = FNULL if redirect_output else None
+      if use_valgrind:
+        process = subprocess.Popen(["valgrind", "--track-origins=yes", "--leak-check=full", "--show-leak-kinds=all", "--error-exitcode=1"] + command, stdout=stdout, stderr=stderr)
+      elif run_profiler:
+        process = subprocess.Popen(["valgrind", "--tool=callgrind"] + command, stdout=stdout, stderr=stderr)
+      else:
+        process = subprocess.Popen(command, stdout=stdout, stderr=stderr)
     # This sleep is critical. If the plasma_manager fails to start because the
     # port is already in use, then we need it to fail within 0.1 seconds.
     time.sleep(0.1)