Recreate actors when local schedulers die. (#804)

* Reconstruct actor state when local schedulers fail.

* Simplify construction of arguments to pass into default_worker.py from local scheduler.

* Remove deprecated ray.actor.

* Simplify actor reconstruction method.

* Fix linting.

* Small fixes.

src/common/state/actor_notification_table.h

@@ -14,6 +14,7 @@ typedef void (*actor_notification_table_subscribe_callback)(
     ActorID actor_id,
     WorkerID driver_id,
     DBClientID local_scheduler_id,
+    bool reconstruct,
     void *user_context);
 
 /**

src/common/state/redis.cc

@@ -1523,16 +1523,33 @@ void redis_actor_notification_table_subscribe_callback(redisAsyncContext *c,
     ActorID actor_id;
     WorkerID driver_id;
     DBClientID local_scheduler_id;
-    CHECK(sizeof(actor_id) + sizeof(driver_id) + sizeof(local_scheduler_id) ==
+    bool reconstruct;
+    CHECK(sizeof(actor_id) + sizeof(driver_id) + sizeof(local_scheduler_id) +
+              1 ==
           payload->len);
-    memcpy(&actor_id, payload->str, sizeof(actor_id));
-    memcpy(&driver_id, payload->str + sizeof(actor_id), sizeof(driver_id));
-    memcpy(&local_scheduler_id,
-           payload->str + sizeof(actor_id) + sizeof(driver_id),
-           sizeof(local_scheduler_id));
+    char *current_ptr = payload->str;
+    /* Parse the actor ID. */
+    memcpy(&actor_id, current_ptr, sizeof(actor_id));
+    current_ptr += sizeof(actor_id);
+    /* Parse the driver ID. */
+    memcpy(&driver_id, current_ptr, sizeof(driver_id));
+    current_ptr += sizeof(driver_id);
+    /* Parse the local scheduler ID. */
+    memcpy(&local_scheduler_id, current_ptr, sizeof(local_scheduler_id));
+    current_ptr += sizeof(local_scheduler_id);
+    /* Parse the reconstruct bit. */
+    if (*current_ptr == '1') {
+      reconstruct = true;
+    } else if (*current_ptr == '0') {
+      reconstruct = false;
+    } else {
+      LOG_FATAL("This code should be unreachable.");
+    }
+    current_ptr += 1;
+
     if (data->subscribe_callback) {
       data->subscribe_callback(actor_id, driver_id, local_scheduler_id,
-                               data->subscribe_context);
+                               reconstruct, data->subscribe_context);
     }
   } else if (strcmp(message_type->str, "subscribe") == 0) {
     /* The reply for the initial SUBSCRIBE command. */

src/local_scheduler/local_scheduler.cc

@@ -206,7 +206,13 @@ void LocalSchedulerState_free(LocalSchedulerState *state) {
  * @param state The state of the local scheduler.
  * @return Void.
  */
-void start_worker(LocalSchedulerState *state, ActorID actor_id) {
+void start_worker(LocalSchedulerState *state,
+                  ActorID actor_id,
+                  bool reconstruct) {
+  /* Non-actors can't be started in reconstruct mode. */
+  if (ActorID_equal(actor_id, NIL_ACTOR_ID)) {
+    CHECK(!reconstruct);
+  }
   /* We can't start a worker if we don't have the path to the worker script. */
   if (state->config.start_worker_command == NULL) {
     LOG_WARN("No valid command to start worker provided. Cannot start worker.");
@@ -223,24 +229,30 @@ void start_worker(LocalSchedulerState *state, ActorID actor_id) {
   /* Reset the SIGCHLD handler so that it doesn't influence the worker. */
   signal(SIGCHLD, SIG_DFL);
 
+  std::vector<const char *> command_vector;
+  for (int i = 0; state->config.start_worker_command[i] != NULL; i++) {
+    command_vector.push_back(state->config.start_worker_command[i]);
+  }
+
+  /* Pass in the worker's actor ID. */
+  const char *actor_id_string = "--actor-id";
   char id_string[ID_STRING_SIZE];
   ObjectID_to_string(actor_id, id_string, ID_STRING_SIZE);
-  /* Figure out how many arguments there are in the start_worker_command. */
-  int num_args = 0;
-  for (; state->config.start_worker_command[num_args] != NULL; ++num_args) {
+  command_vector.push_back(actor_id_string);
+  command_vector.push_back((const char *) id_string);
+
+  /* Add a flag for reconstructing the actor if necessary. */
+  const char *reconstruct_string = "--reconstruct";
+  if (reconstruct) {
+    command_vector.push_back(reconstruct_string);
   }
-  const char **start_actor_worker_command =
-      (const char **) malloc((num_args + 3) * sizeof(const char *));
-  for (int i = 0; i < num_args; ++i) {
-    start_actor_worker_command[i] = state->config.start_worker_command[i];
-  }
-  start_actor_worker_command[num_args] = "--actor-id";
-  start_actor_worker_command[num_args + 1] = (const char *) id_string;
-  start_actor_worker_command[num_args + 2] = NULL;
+
+  /* Add a NULL pointer to the end. */
+  command_vector.push_back(NULL);
+
   /* Try to execute the worker command. Exit if we're not successful. */
-  execvp(start_actor_worker_command[0],
-         (char *const *) start_actor_worker_command);
-  free(start_actor_worker_command);
+  execvp(command_vector[0], (char *const *) command_vector.data());
+
   LocalSchedulerState_free(state);
   LOG_FATAL("Failed to start worker");
 }
@@ -391,7 +403,7 @@ LocalSchedulerState *LocalSchedulerState_init(
 
   /* Start the initial set of workers. */
   for (int i = 0; i < num_workers; ++i) {
-    start_worker(state, NIL_ACTOR_ID);
+    start_worker(state, NIL_ACTOR_ID, false);
   }
 
   /* Initialize the time at which the previous heartbeat was sent. */
@@ -593,15 +605,18 @@ void reconstruct_task_update_callback(Task *task,
   TaskSpec *spec = Task_task_spec(task);
   /* If the task is an actor task, then we currently do not reconstruct it.
    * TODO(rkn): Handle this better. */
-  CHECK(ActorID_equal(TaskSpec_actor_id(spec), NIL_ACTOR_ID));
-  /* Resubmit the task. */
-  handle_task_submitted(state, state->algorithm_state, spec,
-                        Task_task_spec_size(task));
-  /* Recursively reconstruct the task's inputs, if necessary. */
-  for (int64_t i = 0; i < TaskSpec_num_args(spec); ++i) {
-    if (TaskSpec_arg_by_ref(spec, i)) {
-      ObjectID arg_id = TaskSpec_arg_id(spec, i);
-      reconstruct_object(state, arg_id);
+  if (!ActorID_equal(TaskSpec_actor_id(spec), NIL_ACTOR_ID)) {
+    LOG_WARN("We are not resubmitting this task because it is an actor task.");
+  } else {
+    /* Resubmit the task. */
+    handle_task_submitted(state, state->algorithm_state, spec,
+                          Task_task_spec_size(task));
+    /* Recursively reconstruct the task's inputs, if necessary. */
+    for (int64_t i = 0; i < TaskSpec_num_args(spec); ++i) {
+      if (TaskSpec_arg_by_ref(spec, i)) {
+        ObjectID arg_id = TaskSpec_arg_id(spec, i);
+        reconstruct_object(state, arg_id);
+      }
     }
   }
 }
@@ -906,7 +921,7 @@ void process_message(event_loop *loop,
     /* If the disconnected worker was not an actor, start a new worker to make
      * sure there are enough workers in the pool. */
     if (ActorID_equal(worker->actor_id, NIL_ACTOR_ID)) {
-      start_worker(state, NIL_ACTOR_ID);
+      start_worker(state, NIL_ACTOR_ID, false);
     }
   } break;
   case MessageType_EventLogMessage: {
@@ -1090,11 +1105,14 @@ void handle_task_scheduled_callback(Task *original_task,
  * @param actor_id The ID of the actor being created.
  * @param local_scheduler_id The ID of the local scheduler that is responsible
  *        for creating the actor.
+ * @param reconstruct True if the actor should be started in "reconstruct" mode.
+ * @param context The context for this callback.
  * @return Void.
  */
 void handle_actor_creation_callback(ActorID actor_id,
                                     WorkerID driver_id,
                                     DBClientID local_scheduler_id,
+                                    bool reconstruct,
                                     void *context) {
   LocalSchedulerState *state = (LocalSchedulerState *) context;
 
@@ -1103,11 +1121,29 @@ void handle_actor_creation_callback(ActorID actor_id,
     return;
   }
 
-  /* Make sure the actor entry is not already present in the actor map table.
-   * TODO(rkn): We will need to remove this check to handle the case where the
-   * corresponding publish is retried and the case in which a task that creates
-   * an actor is resubmitted due to fault tolerance. */
-  CHECK(state->actor_mapping.count(actor_id) == 0);
+  if (!reconstruct) {
+    /* Make sure the actor entry is not already present in the actor map table.
+     * TODO(rkn): We will need to remove this check to handle the case where the
+     * corresponding publish is retried and the case in which a task that
+     * creates an actor is resubmitted due to fault tolerance. */
+    CHECK(state->actor_mapping.count(actor_id) == 0);
+  } else {
+    /* In this case, the actor already exists. Check that the driver hasn't
+     * changed but that the local scheduler has. */
+    auto it = state->actor_mapping.find(actor_id);
+    CHECK(it != state->actor_mapping.end());
+    CHECK(WorkerID_equal(it->second.driver_id, driver_id));
+    CHECK(!DBClientID_equal(it->second.local_scheduler_id, local_scheduler_id));
+    /* If the actor was previously assigned to this local scheduler, kill the
+     * actor. */
+    if (DBClientID_equal(it->second.local_scheduler_id,
+                         get_db_client_id(state->db))) {
+      /* TODO(rkn): We should kill the actor here if it is still around. Also,
+       * if it hasn't registered yet, we should keep track of its PID so we can
+       * kill it anyway. */
+    }
+  }
+
   /* Create a new entry and add it to the actor mapping table. TODO(rkn):
    * Currently this is never removed (except when the local scheduler state is
    * deleted). */
@@ -1119,11 +1155,12 @@ void handle_actor_creation_callback(ActorID actor_id,
   /* If this local scheduler is responsible for the actor, then start a new
    * worker for the actor. */
   if (DBClientID_equal(local_scheduler_id, get_db_client_id(state->db))) {
-    start_worker(state, actor_id);
+    start_worker(state, actor_id, reconstruct);
   }
   /* Let the scheduling algorithm process the fact that a new actor has been
    * created. */
-  handle_actor_creation_notification(state, state->algorithm_state, actor_id);
+  handle_actor_creation_notification(state, state->algorithm_state, actor_id,
+                                     reconstruct);
 }
 
 int heartbeat_handler(event_loop *loop, timer_id id, void *context) {

src/local_scheduler/local_scheduler.h

@@ -114,9 +114,13 @@ void kill_worker(LocalSchedulerState *state,
  * @param state The local scheduler state.
  * @param actor_id The ID of the actor for this worker. If this worker is not an
  *        actor, then NIL_ACTOR_ID should be used.
+ * @param reconstruct True if the worker is an actor and is being started in
+ *        reconstruct mode.
  * @param Void.
  */
-void start_worker(LocalSchedulerState *state, ActorID actor_id);
+void start_worker(LocalSchedulerState *state,
+                  ActorID actor_id,
+                  bool reconstruct);
 
 /**
  * Check if a certain quantity of dynamic resources are available. If num_cpus

src/local_scheduler/local_scheduler_algorithm.cc

@@ -407,7 +407,13 @@ void add_task_to_actor_queue(LocalSchedulerState *state,
    * guaranteeing in-order execution of the tasks on the actor). TODO(rkn): This
    * check will fail if the fault-tolerance mechanism resubmits a task on an
    * actor. */
-  CHECK(task_counter >= entry.task_counter);
+  bool task_is_redundant = false;
+  if (task_counter < entry.task_counter) {
+    LOG_INFO(
+        "A task that has already been executed has been resubmitted, so we "
+        "are ignoring it. This should only happen during reconstruction.");
+    task_is_redundant = true;
+  }
 
   /* Create a new task queue entry. */
   TaskQueueEntry elt = TaskQueueEntry_init(spec, task_spec_size);
@@ -421,25 +427,36 @@ void add_task_to_actor_queue(LocalSchedulerState *state,
          (task_counter > TaskSpec_actor_counter(it->spec))) {
     ++it;
   }
-  entry.task_queue->insert(it, elt);
-
-  /* Update the task table. */
-  if (state->db != NULL) {
-    Task *task = Task_alloc(spec, task_spec_size, TASK_STATUS_QUEUED,
-                            get_db_client_id(state->db));
-    if (from_global_scheduler) {
-      /* If the task is from the global scheduler, it's already been added to
-       * the task table, so just update the entry. */
-      task_table_update(state->db, task, NULL, NULL, NULL);
-    } else {
-      /* Otherwise, this is the first time the task has been seen in the system
-       * (unless it's a resubmission of a previous task), so add the entry. */
-      task_table_add_task(state->db, task, NULL, NULL, NULL);
-    }
+  if (it != entry.task_queue->end() &&
+      task_counter == TaskSpec_actor_counter(it->spec)) {
+    LOG_INFO(
+        "A task that has already been executed has been resubmitted, so we "
+        "are ignoring it. This should only happen during reconstruction.");
+    task_is_redundant = true;
   }
 
-  /* Record the fact that this actor has a task waiting to execute. */
-  algorithm_state->actors_with_pending_tasks.insert(actor_id);
+  if (!task_is_redundant) {
+    entry.task_queue->insert(it, elt);
+
+    /* Update the task table. */
+    if (state->db != NULL) {
+      Task *task = Task_alloc(spec, task_spec_size, TASK_STATUS_QUEUED,
+                              get_db_client_id(state->db));
+      if (from_global_scheduler) {
+        /* If the task is from the global scheduler, it's already been added to
+         * the task table, so just update the entry. */
+        task_table_update(state->db, task, NULL, NULL, NULL);
+      } else {
+        /* Otherwise, this is the first time the task has been seen in the
+         * system (unless it's a resubmission of a previous task), so add the
+         * entry. */
+        task_table_add_task(state->db, task, NULL, NULL, NULL);
+      }
+    }
+
+    /* Record the fact that this actor has a task waiting to execute. */
+    algorithm_state->actors_with_pending_tasks.insert(actor_id);
+  }
 }
 
 /**
@@ -666,7 +683,7 @@ void dispatch_tasks(LocalSchedulerState *state,
       if (state->child_pids.size() == 0) {
         /* If there are no workers, including those pending PID registration,
          * then we must start a new one to replenish the worker pool. */
-        start_worker(state, NIL_ACTOR_ID);
+        start_worker(state, NIL_ACTOR_ID, false);
       }
       return;
     }
@@ -979,7 +996,8 @@ void handle_actor_task_submitted(LocalSchedulerState *state,
 void handle_actor_creation_notification(
     LocalSchedulerState *state,
     SchedulingAlgorithmState *algorithm_state,
-    ActorID actor_id) {
+    ActorID actor_id,
+    bool reconstruct) {
   int num_cached_actor_tasks =
       algorithm_state->cached_submitted_actor_tasks.size();
 

src/local_scheduler/local_scheduler_algorithm.h

@@ -88,12 +88,14 @@ void handle_actor_task_submitted(LocalSchedulerState *state,
  * @param state The state of the local scheduler.
  * @param algorithm_state State maintained by the scheduling algorithm.
  * @param actor_id The ID of the actor being created.
+ * @param reconstruct True if the actor is being created in "reconstruct" mode.
  * @return Void.
  */
 void handle_actor_creation_notification(
     LocalSchedulerState *state,
     SchedulingAlgorithmState *algorithm_state,
-    ActorID actor_id);
+    ActorID actor_id,
+    bool reconstruct);
 
 /**
  * This function will be called when a task is assigned by the global scheduler

src/local_scheduler/test/local_scheduler_tests.cc

@@ -646,7 +646,7 @@ TEST start_kill_workers_test(void) {
             num_workers - 1);
 
   /* Start a worker after the local scheduler has been initialized. */
-  start_worker(local_scheduler->local_scheduler_state, NIL_ACTOR_ID);
+  start_worker(local_scheduler->local_scheduler_state, NIL_ACTOR_ID, false);
   /* Accept the workers as clients to the plasma manager. */
   int new_worker_fd = accept_client(local_scheduler->plasma_manager_fd);
   /* The new worker should register its process ID. */