Let local clients connect with local manager via unix domain sockets (#11)

* let local clients connect with local manager via unix domain sockets

* fix clang-format

* fix tests

* fix stephanie's comments

src/photon/photon_scheduler.c

@@ -52,7 +52,7 @@ local_scheduler_state *init_local_scheduler(event_loop *loop,
   state->loop = loop;
   /* Connect to Plasma. This method will retry if Plasma hasn't started yet.
    * Pass in a NULL manager address and port. */
-  state->plasma_conn = plasma_connect(plasma_socket_name, NULL, 0);
+  state->plasma_conn = plasma_connect(plasma_socket_name, NULL);
   /* Subscribe to notifications about sealed objects. */
   int plasma_fd = plasma_subscribe(state->plasma_conn);
   /* Add the callback that processes the notification to the event loop. */

src/plasma/example.c

@@ -25,7 +25,7 @@ int main(int argc, char *argv[]) {
   while ((c = getopt(argc, argv, "s:cfg")) != -1) {
     switch (c) {
     case 's':
-      conn = plasma_connect(optarg, NULL, 0);
+      conn = plasma_connect(optarg, NULL);
       break;
     case 'c':
       assert(conn != NULL);

src/plasma/lib/python/plasma.py

@@ -64,19 +64,13 @@ class PlasmaClient(object):
   strings.
   """
 
-  def __init__(self, socket_name, addr=None, port=None):
+  def __init__(self, store_socket_name, manager_socket_name=None):
     """Initialize the PlasmaClient.
 
     Args:
-      socket_name (str): Name of the socket the plasma store is listening at.
-      addr (str): IPv4 address of plasma manager attached to the plasma store.
-      port (int): Port number of the plasma manager attached to the plasma store.
+      store_socket_name (str): Name of the socket the plasma store is listening at.
+      manager_socket_name (str): Name of the socket the plasma manager is listening at.
     """
-    if port is not None:
-      if not isinstance(port, int):
-        raise Exception("The 'port' argument must be an integer. The given argument has type {}.".format(type(port)))
-      if not 0 < port < 65536:
-        raise Exception("The 'port' argument must be greater than 0 and less than 65536. The given value is {}.".format(port))
 
     plasma_client_library = os.path.join(os.path.abspath(os.path.dirname(__file__)), "../../build/plasma_client.so")
     self.client = ctypes.cdll.LoadLibrary(plasma_client_library)
@@ -98,12 +92,12 @@ class PlasmaClient(object):
     self.buffer_from_read_write_memory.argtypes = [ctypes.c_void_p, ctypes.c_int64]
     self.buffer_from_read_write_memory.restype = ctypes.py_object
 
-    if addr is not None and port is not None:
+    if manager_socket_name is not None:
       self.has_manager_conn = True
-      self.plasma_conn = ctypes.c_void_p(self.client.plasma_connect(socket_name, addr, port))
+      self.plasma_conn = ctypes.c_void_p(self.client.plasma_connect(store_socket_name, manager_socket_name))
     else:
       self.has_manager_conn = False
-      self.plasma_conn = ctypes.c_void_p(self.client.plasma_connect(socket_name, None, 0))
+      self.plasma_conn = ctypes.c_void_p(self.client.plasma_connect(store_socket_name, None))
 
   def create(self, object_id, size, metadata=None):
     """Create a new buffer in the PlasmaStore for a particular object ID.

src/plasma/plasma_client.c

@@ -22,6 +22,7 @@
 
 /* Number of times we try connecting to a socket. */
 #define NUM_CONNECT_ATTEMPTS 50
+#define CONNECT_TIMEOUT 100
 
 typedef struct {
   /** Key that uniquely identifies the  memory mapped file. In practice, we
@@ -306,38 +307,36 @@ int plasma_subscribe(plasma_connection *conn) {
   return fd[0];
 }
 
-plasma_connection *plasma_connect(const char *store_socket_name,
-                                  const char *manager_addr,
-                                  int manager_port) {
-  CHECK(store_socket_name);
-  /* Try to connect to the Plasma store. If unsuccessful, retry several times.
-   */
+int socket_connect_retry(const char *socket_name,
+                         int num_retries,
+                         int64_t timeout) {
+  CHECK(socket_name);
   int fd = -1;
-  int connected_successfully = 0;
-  for (int num_attempts = 0; num_attempts < NUM_CONNECT_ATTEMPTS;
-       ++num_attempts) {
-    fd = connect_ipc_sock(store_socket_name);
+  for (int num_attempts = 0; num_attempts < num_retries; ++num_attempts) {
+    fd = connect_ipc_sock(socket_name);
     if (fd >= 0) {
-      connected_successfully = 1;
       break;
     }
-    /* Sleep for 100 milliseconds. */
-    usleep(100000);
+    /* Sleep for timeout milliseconds. */
+    usleep(timeout * 1000);
   }
-  /* If we could not connect to the Plasma store, exit. */
-  if (!connected_successfully) {
-    LOG_ERR("could not connect to store %s", store_socket_name);
+  /* If we could not connect to the socket, exit. */
+  if (fd == -1) {
+    LOG_ERR("could not connect to socket %s", socket_name);
     exit(-1);
   }
+  return fd;
+}
+
+plasma_connection *plasma_connect(const char *store_socket_name,
+                                  const char *manager_socket_name) {
   /* Initialize the store connection struct */
   plasma_connection *result = malloc(sizeof(plasma_connection));
-  result->store_conn = fd;
-  if (manager_addr != NULL) {
-    result->manager_conn = plasma_manager_connect(manager_addr, manager_port);
-    if (result->manager_conn < 0) {
-      LOG_ERR("Could not connect to Plasma manager %s:%d", manager_addr,
-              manager_port);
-    }
+  result->store_conn = socket_connect_retry(
+      store_socket_name, NUM_CONNECT_ATTEMPTS, CONNECT_TIMEOUT);
+  if (manager_socket_name != NULL) {
+    result->manager_conn = socket_connect_retry(
+        manager_socket_name, NUM_CONNECT_ATTEMPTS, CONNECT_TIMEOUT);
   } else {
     result->manager_conn = -1;
   }

src/plasma/plasma_client.h

@@ -36,21 +36,31 @@ plasma_request make_plasma_request(object_id object_id);
 plasma_request *make_plasma_multiple_request(int num_object_ids,
                                              object_id object_ids[]);
 
+/**
+ * Try to connect to the socket several times. If unsuccessful, fail.
+ *
+ * @param socket_name Name of the Unix domain socket to connect to.
+ * @param num_retries Number of retries.
+ * @param timeout Timeout in milliseconds.
+ * @return File descriptor of the socket.
+ */
+int socket_connect_retry(const char *socket_name,
+                         int num_retries,
+                         int64_t timeout);
+
 /**
  * Connect to the local plasma store and plasma manager. Return
  * the resulting connection.
  *
- * @param socket_name The name of the UNIX domain socket to use to connect to
- *        the Plasma Store.
- * @param manager_addr The IP address of the plasma manager to connect to. If
- *        this is NULL, then this function will not connect to a manager.
- * @param manager_port The port of the plasma manager to connect to. If
- *        manager_addr is NULL, then this argument is unused.
+ * @param store_socket_name The name of the UNIX domain socket to use to
+ *        connect to the Plasma store.
+ * @param manager_socket_name The name of the UNIX domain socket to use to
+ *        connect to the local Plasma manager. If this is NULL, then this
+ *        function will not connect to a manager.
  * @return The object containing the connection state.
  */
 plasma_connection *plasma_connect(const char *store_socket_name,
-                                  const char *manager_addr,
-                                  int manager_port);
+                                  const char *manager_socket_name);
 
 /**
  * Disconnect from the local plasma instance, including the local store and

src/plasma/plasma_manager.c

@@ -226,7 +226,7 @@ plasma_manager_state *init_plasma_manager_state(const char *store_socket_name,
                                                 int db_port) {
   plasma_manager_state *state = malloc(sizeof(plasma_manager_state));
   state->loop = event_loop_create();
-  state->plasma_conn = plasma_connect(store_socket_name, NULL, 0);
+  state->plasma_conn = plasma_connect(store_socket_name, NULL);
   state->manager_connections = NULL;
   state->fetch_connections = NULL;
   if (db_addr) {
@@ -710,6 +710,9 @@ void process_message(event_loop *loop,
   free(req);
 }
 
+/* TODO(pcm): Split this into two methods: new_worker_connection
+ * and new_manager_connection and also split client_connection
+ * into two structs, one for workers and one for other plasma managers. */
 client_connection *new_client_connection(event_loop *loop,
                                          int listener_sock,
                                          void *context,
@@ -724,7 +727,7 @@ client_connection *new_client_connection(event_loop *loop,
   conn->active_objects = NULL;
   conn->num_return_objects = 0;
   event_loop_add_file(loop, new_socket, EVENT_LOOP_READ, process_message, conn);
-  LOG_DEBUG("New plasma manager connection with fd %d", new_socket);
+  LOG_DEBUG("New client connection with fd %d", new_socket);
   return conn;
 }
 
@@ -740,6 +743,7 @@ int get_client_sock(client_connection *conn) {
 }
 
 void start_server(const char *store_socket_name,
+                  const char *manager_socket_name,
                   const char *master_addr,
                   int port,
                   const char *db_addr,
@@ -748,12 +752,16 @@ void start_server(const char *store_socket_name,
                                               port, db_addr, db_port);
   CHECK(g_manager_state);
 
-  int sock = bind_inet_sock(port);
-  CHECKM(sock >= 0, "Unable to bind to manager port");
+  int remote_sock = bind_inet_sock(port);
+  CHECKM(remote_sock >= 0, "Unable to bind to manager port");
+  int local_sock = bind_ipc_sock(manager_socket_name);
+  CHECKM(local_sock >= 0, "Unable to bind local manager socket");
 
   LOG_DEBUG("Started server connected to store %s, listening on port %d",
             store_socket_name, port);
-  event_loop_add_file(g_manager_state->loop, sock, EVENT_LOOP_READ,
+  event_loop_add_file(g_manager_state->loop, local_sock, EVENT_LOOP_READ,
+                      handle_new_client, g_manager_state);
+  event_loop_add_file(g_manager_state->loop, remote_sock, EVENT_LOOP_READ,
                       handle_new_client, g_manager_state);
   event_loop_run(g_manager_state->loop);
 }
@@ -775,25 +783,30 @@ int main(int argc, char *argv[]) {
   signal(SIGTERM, signal_handler);
   /* Socket name of the plasma store this manager is connected to. */
   char *store_socket_name = NULL;
+  /* Socket name this manager will bind to. */
+  char *manager_socket_name = NULL;
   /* IP address of this node. */
   char *master_addr = NULL;
   /* Port number the manager should use. */
-  int port;
+  int port = -1;
   /* IP address and port of state database. */
   char *db_host = NULL;
   int c;
-  while ((c = getopt(argc, argv, "s:m:p:d:")) != -1) {
+  while ((c = getopt(argc, argv, "s:m:h:p:r:")) != -1) {
     switch (c) {
     case 's':
       store_socket_name = optarg;
       break;
     case 'm':
+      manager_socket_name = optarg;
+      break;
+    case 'h':
       master_addr = optarg;
       break;
     case 'p':
       port = atoi(optarg);
       break;
-    case 'd':
+    case 'r':
       db_host = optarg;
       break;
     default:
@@ -807,19 +820,33 @@ int main(int argc, char *argv[]) {
         "switch");
     exit(-1);
   }
+  if (!manager_socket_name) {
+    LOG_ERR(
+        "please specify socket name of the manager's local socket with -m "
+        "switch");
+    exit(-1);
+  }
   if (!master_addr) {
     LOG_ERR(
         "please specify ip address of the current host in the format "
         "123.456.789.10 with -m switch");
     exit(-1);
   }
+  if (port == -1) {
+    LOG_ERR(
+        "please specify port the plasma manager shall listen to in the"
+        "format 12345 with -p switch");
+    exit(-1);
+  }
   char db_addr[16];
   int db_port;
   if (db_host) {
     parse_ip_addr_port(db_host, db_addr, &db_port);
-    start_server(store_socket_name, master_addr, port, db_addr, db_port);
+    start_server(store_socket_name, manager_socket_name, master_addr, port,
+                 db_addr, db_port);
   } else {
-    start_server(store_socket_name, master_addr, port, NULL, 0);
+    start_server(store_socket_name, manager_socket_name, master_addr, port,
+                 NULL, 0);
   }
 }
 #endif

src/plasma/test/manager_tests.c

@@ -18,6 +18,7 @@ SUITE(plasma_manager_tests);
 const char *manager_addr = "127.0.0.1";
 int manager_port = 12345;
 const char *store_socket_name = "/tmp/store12345";
+const char *manager_socket_name = "/tmp/manager12345";
 object_id oid;
 
 int test_done_handler(event_loop *loop, timer_id id, void *context) {
@@ -27,8 +28,12 @@ int test_done_handler(event_loop *loop, timer_id id, void *context) {
 
 typedef struct {
   int port;
-  int manager_fd;
+  /** Connection to the manager's TCP socket. */
+  int manager_remote_fd;
+  /** Connection to the manager's Unix socket. */
+  int manager_local_fd;
   int local_store;
+  int manager;
   plasma_manager_state *state;
   event_loop *loop;
   /* Accept a connection from the local manager on the remote manager. */
@@ -45,7 +50,8 @@ plasma_mock *init_plasma_mock(int port, plasma_mock *remote_mock) {
   plasma_mock *mock = malloc(sizeof(plasma_mock));
   /* Start listening on all the ports and initiate the local plasma manager. */
   mock->port = port;
-  mock->manager_fd = bind_inet_sock(port);
+  mock->manager_remote_fd = bind_inet_sock(port);
+  mock->manager_local_fd = bind_ipc_sock(manager_socket_name);
   mock->local_store = bind_ipc_sock(store_socket_name);
   mock->state =
       init_plasma_manager_state(store_socket_name, manager_addr, port, NULL, 0);
@@ -54,8 +60,8 @@ plasma_mock *init_plasma_mock(int port, plasma_mock *remote_mock) {
   if (remote_mock != NULL) {
     mock->write_conn =
         get_manager_connection(remote_mock->state, manager_addr, port);
-    mock->read_conn =
-        new_client_connection(mock->loop, mock->manager_fd, mock->state, 0);
+    mock->read_conn = new_client_connection(mock->loop, mock->manager_remote_fd,
+                                            mock->state, 0);
   } else {
     mock->write_conn = NULL;
     mock->read_conn = NULL;
@@ -70,10 +76,9 @@ plasma_mock *init_plasma_mock(int port, plasma_mock *remote_mock) {
 void add_mock_object_conn(plasma_mock *mock, object_id oid) {
   /* Connect a new client to the local plasma manager and mock a request to an
    * object. */
-  mock->plasma_conn =
-      plasma_connect(store_socket_name, manager_addr, mock->port);
+  mock->plasma_conn = plasma_connect(store_socket_name, manager_socket_name);
   mock->client_conn =
-      new_client_connection(mock->loop, mock->manager_fd, mock->state, 0);
+      new_client_connection(mock->loop, mock->manager_local_fd, mock->state, 0);
   mock->object_conn = add_object_connection(mock->client_conn, oid);
 }
 
@@ -88,7 +93,8 @@ void destroy_plasma_mock(plasma_mock *mock) {
   }
   destroy_plasma_manager_state(mock->state);
   close(mock->local_store);
-  close(mock->manager_fd);
+  close(mock->manager_local_fd);
+  close(mock->manager_remote_fd);
   free(mock);
 }
 

src/plasma/test/test.py

@@ -212,6 +212,8 @@ class TestPlasmaManager(unittest.TestCase):
     plasma_store_executable = os.path.join(os.path.abspath(os.path.dirname(__file__)), "../build/plasma_store")
     store_name1 = "/tmp/store{}".format(random.randint(0, 10000))
     store_name2 = "/tmp/store{}".format(random.randint(0, 10000))
+    manager_name1 = "/tmp/manager{}".format(random.randint(0, 10000))
+    manager_name2 = "/tmp/manager{}".format(random.randint(0, 10000))
     plasma_store_command1 = [plasma_store_executable, "-s", store_name1]
     plasma_store_command2 = [plasma_store_executable, "-s", store_name2]
 
@@ -233,7 +235,7 @@ class TestPlasmaManager(unittest.TestCase):
                                                "--port", str(redis_port)],
                                               stdout=FNULL)
       time.sleep(0.1)
-      manager_redis_args = ["-d", "{addr}:{port}".format(addr="127.0.0.1",
+      manager_redis_args = ["-r", "{addr}:{port}".format(addr="127.0.0.1",
                                                       port=redis_port)]
 
     # Start two PlasmaManagers.
@@ -242,11 +244,13 @@ class TestPlasmaManager(unittest.TestCase):
     plasma_manager_executable = os.path.join(os.path.abspath(os.path.dirname(__file__)), "../build/plasma_manager")
     plasma_manager_command1 = [plasma_manager_executable,
                                "-s", store_name1,
-                               "-m", "127.0.0.1",
+                               "-m", manager_name1,
+                               "-h", "127.0.0.1",
                                "-p", str(self.port1)] + manager_redis_args
     plasma_manager_command2 = [plasma_manager_executable,
                                "-s", store_name2,
-                               "-m", "127.0.0.1",
+                               "-m", manager_name2,
+                               "-h", "127.0.0.1",
                                "-p", str(self.port2)] + manager_redis_args
 
     if USE_VALGRIND:
@@ -257,8 +261,8 @@ class TestPlasmaManager(unittest.TestCase):
       self.p5 = subprocess.Popen(plasma_manager_command2)
 
     # Connect two PlasmaClients.
-    self.client1 = plasma.PlasmaClient(store_name1, "127.0.0.1", self.port1)
-    self.client2 = plasma.PlasmaClient(store_name2, "127.0.0.1", self.port2)
+    self.client1 = plasma.PlasmaClient(store_name1, manager_name1)
+    self.client2 = plasma.PlasmaClient(store_name2, manager_name2)
 
   def tearDown(self):
     # Kill the PlasmaStore and PlasmaManager processes.