From 44826878ff0a091b9c22a621607bf55dcee068d5 Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Thu, 6 Aug 2020 16:37:50 -0700 Subject: [PATCH] [Core] Remove Legacy Raylet Code (#9936) * Remove a flag and some methods in node manager including HandleDisconnectedActor, ResubmitTask, and HandleTaskReconstruction * Make actor creator always required + remove raylet transport * Remove actor reporter + remove FinishAssignedActorCreationTask * Remove actor tasks. * Remove finishactortask and switched it to finishactorcreation task * Remove reconstruction policy. * Remove lineage cache. * Formatting. * Remove actor frontier code. * Removed build error. * Revert "Remove reconstruction policy." This reverts commit 9d25c9bced4da5fbcac5d484d51013345f16513b. * Recover HandleReconstruction to mark expired objects as failed. --- .bazelrc | 1 - BUILD.bazel | 11 - python/ray/_raylet.pyx | 5 - python/ray/actor.py | 5 +- python/ray/includes/ray_config.pxd | 2 - python/ray/tests/test_actor_advanced.py | 5 - python/ray/tests/test_gcs_fault_tolerance.py | 10 - python/ray/util/__init__.py | 3 +- python/ray/util/named_actors.py | 71 +- src/ray/common/ray_config_def.h | 4 - src/ray/core_worker/actor_manager.cc | 29 - src/ray/core_worker/actor_reporter.cc | 37 - src/ray/core_worker/actor_reporter.h | 44 - src/ray/core_worker/core_worker.cc | 33 +- src/ray/core_worker/core_worker.h | 13 - src/ray/core_worker/task_manager.cc | 6 - src/ray/core_worker/task_manager.h | 6 - .../core_worker/test/actor_manager_test.cc | 1 - .../test/direct_task_transport_test.cc | 79 +- src/ray/core_worker/test/task_manager_test.cc | 13 +- .../transport/direct_task_transport.cc | 4 +- .../transport/direct_task_transport.h | 3 +- .../core_worker/transport/raylet_transport.cc | 87 -- .../core_worker/transport/raylet_transport.h | 61 -- src/ray/gcs/gcs_server/gcs_actor_manager.cc | 4 +- src/ray/gcs/redis_gcs_client.cc | 6 +- src/ray/protobuf/core_worker.proto | 20 - src/ray/protobuf/node_manager.proto | 13 - src/ray/raylet/format/node_manager.fbs | 24 - src/ray/raylet/lineage_cache.cc | 412 --------- src/ray/raylet/lineage_cache.h | 327 ------- src/ray/raylet/lineage_cache_test.cc | 670 --------------- src/ray/raylet/node_manager.cc | 813 +++--------------- src/ray/raylet/node_manager.h | 59 +- src/ray/raylet/worker.cc | 21 - src/ray/raylet/worker.h | 2 - .../rpc/node_manager/node_manager_client.h | 6 - .../rpc/node_manager/node_manager_server.h | 5 - src/ray/rpc/worker/core_worker_client.h | 10 - src/ray/rpc/worker/core_worker_server.h | 2 - src/ray/stats/metric_defs.h | 4 - 41 files changed, 180 insertions(+), 2751 deletions(-) delete mode 100644 src/ray/core_worker/actor_reporter.cc delete mode 100644 src/ray/core_worker/actor_reporter.h delete mode 100644 src/ray/core_worker/transport/raylet_transport.cc delete mode 100644 src/ray/core_worker/transport/raylet_transport.h delete mode 100644 src/ray/raylet/lineage_cache.cc delete mode 100644 src/ray/raylet/lineage_cache.h delete mode 100644 src/ray/raylet/lineage_cache_test.cc diff --git a/.bazelrc b/.bazelrc index ed7de086e..38dbaa83f 100644 --- a/.bazelrc +++ b/.bazelrc @@ -101,7 +101,6 @@ test:ci --nocache_test_results test:ci --spawn_strategy=local test:ci --test_output=errors test:ci --test_verbose_timeout_warnings -test:ci --test_env=RAY_GCS_ACTOR_SERVICE_ENABLED aquery:get-toolchain --include_commandline=false aquery:get-toolchain --noimplicit_deps diff --git a/BUILD.bazel b/BUILD.bazel index 219dd4c9c..7e79e57a7 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -738,17 +738,6 @@ cc_test( ], ) -cc_test( - name = "lineage_cache_test", - srcs = ["src/ray/raylet/lineage_cache_test.cc"], - copts = COPTS, - deps = [ - ":node_manager_fbs", - ":raylet_lib", - "@com_google_googletest//:gtest_main", - ], -) - cc_test( name = "reconstruction_policy_test", srcs = ["src/ray/raylet/reconstruction_policy_test.cc"], diff --git a/python/ray/_raylet.pyx b/python/ray/_raylet.pyx index 9ff67222b..94fd062e5 100644 --- a/python/ray/_raylet.pyx +++ b/python/ray/_raylet.pyx @@ -127,11 +127,6 @@ OPTIMIZED = __OPTIMIZE__ logger = logging.getLogger(__name__) -def gcs_actor_service_enabled(): - return ( - RayConfig.instance().gcs_actor_service_enabled()) - - cdef int check_status(const CRayStatus& status) nogil except -1: if status.ok(): return 0 diff --git a/python/ray/actor.py b/python/ray/actor.py index bea304fe8..d66b330ad 100644 --- a/python/ray/actor.py +++ b/python/ray/actor.py @@ -9,7 +9,7 @@ import ray._raylet import ray.signature as signature import ray.worker from ray import ActorClassID, Language -from ray._raylet import PythonFunctionDescriptor, gcs_actor_service_enabled +from ray._raylet import PythonFunctionDescriptor from ray import cross_language logger = logging.getLogger(__name__) @@ -591,9 +591,6 @@ class ActorClass: worker.current_session_and_job, original_handle=True) - if name is not None and not gcs_actor_service_enabled(): - ray.util.named_actors._register_actor(name, actor_handle) - return actor_handle diff --git a/python/ray/includes/ray_config.pxd b/python/ray/includes/ray_config.pxd index a84f0edb0..791ff9059 100644 --- a/python/ray/includes/ray_config.pxd +++ b/python/ray/includes/ray_config.pxd @@ -87,8 +87,6 @@ cdef extern from "ray/common/ray_config.h" nogil: int64_t max_direct_call_object_size() const - c_bool gcs_actor_service_enabled() const - c_bool put_small_object_in_memory_store() const uint32_t max_tasks_in_flight_per_worker() const diff --git a/python/ray/tests/test_actor_advanced.py b/python/ray/tests/test_actor_advanced.py index eaeaa2e96..651a92c1b 100644 --- a/python/ray/tests/test_actor_advanced.py +++ b/python/ray/tests/test_actor_advanced.py @@ -853,11 +853,6 @@ def test_actor_creation_task_crash(ray_start_regular): ray.get(ra.f.remote()) -@pytest.mark.skipif( - os.environ.get("RAY_GCS_ACTOR_SERVICE_ENABLED") != "true", - reason=("This edge case is not handled when GCS actor management is off. " - "We won't fix this because GCS actor management " - "will be on by default anyway.")) @pytest.mark.parametrize( "ray_start_regular", [{ "num_cpus": 2, diff --git a/python/ray/tests/test_gcs_fault_tolerance.py b/python/ray/tests/test_gcs_fault_tolerance.py index 4dae6bec4..133e42cce 100644 --- a/python/ray/tests/test_gcs_fault_tolerance.py +++ b/python/ray/tests/test_gcs_fault_tolerance.py @@ -1,4 +1,3 @@ -import os import sys import ray @@ -21,9 +20,6 @@ def increase(x): return x + 1 -@pytest.mark.skipif( - os.environ.get("RAY_GCS_ACTOR_SERVICE_ENABLED", "true") != "true", - reason=("This testcase can only be run when GCS actor management is on.")) @pytest.mark.parametrize( "ray_start_regular", [generate_internal_config_map(num_heartbeats_timeout=20)], @@ -47,9 +43,6 @@ def test_gcs_server_restart(ray_start_regular): assert result == 2 -@pytest.mark.skipif( - os.environ.get("RAY_GCS_ACTOR_SERVICE_ENABLED", "true") != "true", - reason=("This testcase can only be run when GCS actor management is on.")) @pytest.mark.parametrize( "ray_start_regular", [generate_internal_config_map(num_heartbeats_timeout=20)], @@ -69,9 +62,6 @@ def test_gcs_server_restart_during_actor_creation(ray_start_regular): assert len(unready) == 0 -@pytest.mark.skipif( - os.environ.get("RAY_GCS_ACTOR_SERVICE_ENABLED", "true") != "true", - reason=("This testcase can only be run when GCS actor management is on.")) @pytest.mark.parametrize( "ray_start_cluster_head", [generate_internal_config_map(num_heartbeats_timeout=20)], diff --git a/python/ray/util/__init__.py b/python/ray/util/__init__.py index 7fb14563d..17b048c16 100644 --- a/python/ray/util/__init__.py +++ b/python/ray/util/__init__.py @@ -2,7 +2,7 @@ from ray.util import iter from ray.util.actor_pool import ActorPool from ray.util.debug import log_once, disable_log_once_globally, \ enable_periodic_logging -from ray.util.named_actors import get_actor, register_actor +from ray.util.named_actors import get_actor __all__ = [ "ActorPool", @@ -11,5 +11,4 @@ __all__ = [ "get_actor", "iter", "log_once", - "register_actor", ] diff --git a/python/ray/util/named_actors.py b/python/ray/util/named_actors.py index 64f377d09..b1d39fe2d 100644 --- a/python/ray/util/named_actors.py +++ b/python/ray/util/named_actors.py @@ -1,44 +1,13 @@ import logging import ray -import ray.cloudpickle as pickle -from ray.experimental.internal_kv import _internal_kv_get, _internal_kv_put -from ray.gcs_utils import ActorTableData logger = logging.getLogger(__name__) -def _calculate_key(name): - """Generate a Redis key with the given name. - - Args: - name: The name of the named actor. - - Returns: - The key to use for storing a named actor in Redis. - """ - return b"Actor:" + name.encode("ascii") - - def _get_actor(name): - if ray._raylet.gcs_actor_service_enabled(): - worker = ray.worker.global_worker - handle = worker.core_worker.get_named_actor_handle(name) - else: - actor_name = _calculate_key(name) - pickled_state = _internal_kv_get(actor_name) - if pickled_state is None: - raise ValueError( - "The actor with name={} doesn't exist".format(name)) - handle = pickle.loads(pickled_state) - # If the actor state is dead, that means that this name is reusable. - # We don't delete the name entry from key value store when - # the actor is killed because ray.kill is asynchronous, - # and it can cause worker leaks. - actor_info = ray.actors(actor_id=handle._actor_id.hex()) - actor_state = actor_info.get("State", None) - if actor_state and actor_state == ActorTableData.DEAD: - raise ValueError("The actor with name={} is dead.".format(name)) + worker = ray.worker.global_worker + handle = worker.core_worker.get_named_actor_handle(name) return handle @@ -56,39 +25,3 @@ def get_actor(name: str) -> ray.actor.ActorHandle: logger.warning("ray.util.get_actor has been moved to ray.get_actor and " "will be removed in the future.") return _get_actor(name) - - -def _register_actor(name, actor_handle): - if not isinstance(name, str): - raise TypeError("The name argument must be a string.") - if not isinstance(actor_handle, ray.actor.ActorHandle): - raise TypeError("The actor_handle argument must be an ActorHandle " - "object.") - actor_name = _calculate_key(name) - - # First check if the actor already exists. - try: - _get_actor(name) - exists = True - except ValueError: - exists = False - - if exists: - raise ValueError("An actor with name={} already exists or there " - "was timeout in getting this actor handle." - .format(name)) - - # Add the actor to Redis if it does not already exist. - _internal_kv_put(actor_name, pickle.dumps(actor_handle), overwrite=True) - - -def register_actor(name, actor_handle): - """Register a named actor under a string key. - - Args: - name: The name of the named actor. - actor_handle: The actor object to be associated with this name - """ - logger.warning("ray.util.register_actor is deprecated. To create a " - "named, detached actor, use Actor.options(name=\"name\").") - return _register_actor(name, actor_handle) diff --git a/src/ray/common/ray_config_def.h b/src/ray/common/ray_config_def.h index 13bdf4cc8..e4293817d 100644 --- a/src/ray/common/ray_config_def.h +++ b/src/ray/common/ray_config_def.h @@ -312,10 +312,6 @@ RAY_CONFIG(bool, plasma_store_as_thread, false) /// changed. When the address changed, we will resubscribe again. RAY_CONFIG(int64_t, gcs_service_address_check_interval_milliseconds, 1000) -RAY_CONFIG(bool, gcs_actor_service_enabled, - getenv("RAY_GCS_ACTOR_SERVICE_ENABLED") == nullptr || - getenv("RAY_GCS_ACTOR_SERVICE_ENABLED") == std::string("true")) - /// The batch size for metrics export. RAY_CONFIG(int64_t, metrics_report_batch_size, 100) diff --git a/src/ray/core_worker/actor_manager.cc b/src/ray/core_worker/actor_manager.cc index 3ca57895a..c92bdc605 100644 --- a/src/ray/core_worker/actor_manager.cc +++ b/src/ray/core_worker/actor_manager.cc @@ -91,35 +91,6 @@ bool ActorManager::AddActorHandle(std::unique_ptr actor_handle, std::placeholders::_1, std::placeholders::_2); RAY_CHECK_OK(gcs_client_->Actors().AsyncSubscribe( actor_id, actor_notification_callback, nullptr)); - - if (!RayConfig::instance().gcs_actor_service_enabled()) { - RAY_CHECK(reference_counter_->SetDeleteCallback( - actor_creation_return_id, - [this, actor_id, is_owner_handle](const ObjectID &object_id) { - if (is_owner_handle) { - // If we own the actor and the actor handle is no longer in scope, - // terminate the actor. We do not do this if the GCS service is - // enabled since then the GCS will terminate the actor for us. - // TODO(sang): Remove this block once gcs_actor_service is enabled by - // default. - RAY_LOG(INFO) << "Owner's handle and creation ID " << object_id - << " has gone out of scope, sending message to actor " - << actor_id << " to do a clean exit."; - RAY_CHECK(CheckActorHandleExists(actor_id)); - direct_actor_submitter_->KillActor(actor_id, - /*force_kill=*/false, - /*no_restart=*/false); - - // TODO(swang): Erase the actor handle once all refs to the actor - // have gone out of scope. We cannot erase it here in case the - // language frontend receives another ref to the same actor. In this - // case, we must remember the last task counter that we sent to the - // actor. - // TODO(ekl) we can't unsubscribe to actor notifications here due to - // https://github.com/ray-project/ray/pull/6885 - } - })); - } } return inserted; diff --git a/src/ray/core_worker/actor_reporter.cc b/src/ray/core_worker/actor_reporter.cc deleted file mode 100644 index 049256a11..000000000 --- a/src/ray/core_worker/actor_reporter.cc +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2017 The Ray Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "ray/core_worker/actor_reporter.h" - -#include "ray/gcs/pb_util.h" -#include "ray/gcs/redis_accessor.h" - -namespace ray { - -void ActorReporter::PublishTerminatedActor(const TaskSpecification &actor_creation_task) { - auto actor_id = actor_creation_task.ActorCreationId(); - auto data = gcs::CreateActorTableData(actor_creation_task, rpc::Address(), - rpc::ActorTableData::DEAD, 0); - - auto update_callback = [actor_id](Status status) { - if (!status.ok()) { - // Only one node at a time should succeed at creating or updating the actor. - RAY_LOG(ERROR) << "Failed to update state to DEAD for actor " << actor_id - << ", error: " << status.ToString(); - } - }; - RAY_CHECK_OK(gcs_client_->Actors().AsyncRegister(data, update_callback)); -} - -} // namespace ray diff --git a/src/ray/core_worker/actor_reporter.h b/src/ray/core_worker/actor_reporter.h deleted file mode 100644 index 1a1323de9..000000000 --- a/src/ray/core_worker/actor_reporter.h +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2017 The Ray Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#pragma once - -#include "ray/gcs/redis_gcs_client.h" - -namespace ray { - -// TODO(SANG): This class won't be needed once GCS actor mangement becomes the default. -// Interface for testing. -class ActorReporterInterface { - public: - virtual void PublishTerminatedActor(const TaskSpecification &actor_creation_task) = 0; - - virtual ~ActorReporterInterface() {} -}; - -class ActorReporter : public ActorReporterInterface { - public: - ActorReporter(std::shared_ptr gcs_client) : gcs_client_(gcs_client) {} - - ~ActorReporter() {} - - /// Called when an actor that we own can no longer be restarted. - void PublishTerminatedActor(const TaskSpecification &actor_creation_task) override; - - private: - /// GCS client - std::shared_ptr gcs_client_; -}; - -} // namespace ray diff --git a/src/ray/core_worker/core_worker.cc b/src/ray/core_worker/core_worker.cc index 6066582ae..ef3dab8eb 100644 --- a/src/ray/core_worker/core_worker.cc +++ b/src/ray/core_worker/core_worker.cc @@ -20,7 +20,6 @@ #include "ray/common/task/task_util.h" #include "ray/core_worker/context.h" #include "ray/core_worker/transport/direct_actor_transport.h" -#include "ray/core_worker/transport/raylet_transport.h" #include "ray/gcs/gcs_client/service_based_gcs_client.h" #include "ray/stats/stats.h" #include "ray/util/process.h" @@ -286,9 +285,6 @@ CoreWorker::CoreWorker(const CoreWorkerOptions &options, const WorkerID &worker_ auto execute_task = std::bind(&CoreWorker::ExecuteTask, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4); - raylet_task_receiver_ = - std::unique_ptr(new CoreWorkerRayletTaskReceiver( - worker_context_.GetWorkerID(), local_raylet_client_, execute_task)); direct_task_receiver_ = std::unique_ptr(new CoreWorkerDirectTaskReceiver( worker_context_, task_execution_service_, execute_task, @@ -378,8 +374,6 @@ CoreWorker::CoreWorker(const CoreWorkerOptions &options, const WorkerID &worker_ boost::asio::chrono::milliseconds(kInternalHeartbeatMillis)); internal_timer_.async_wait(boost::bind(&CoreWorker::InternalHeartbeat, this, _1)); - actor_reporter_ = std::unique_ptr(new ActorReporter(gcs_client_)); - plasma_store_provider_.reset(new CoreWorkerPlasmaStoreProvider( options_.store_socket, local_raylet_client_, reference_counter_, options_.check_signals, @@ -407,7 +401,7 @@ CoreWorker::CoreWorker(const CoreWorkerOptions &options, const WorkerID &worker_ }); }; task_manager_.reset(new TaskManager( - memory_store_, reference_counter_, actor_reporter_, + memory_store_, reference_counter_, [this](TaskSpecification &spec, bool delay) { if (delay) { // Retry after a delay to emulate the existing Raylet reconstruction @@ -463,10 +457,8 @@ CoreWorker::CoreWorker(const CoreWorkerOptions &options, const WorkerID &worker_ new raylet::RayletClient(std::move(grpc_client))); }; - std::shared_ptr actor_creator = nullptr; - if (RayConfig::instance().gcs_actor_service_enabled()) { - actor_creator = std::make_shared(gcs_client_); - } + std::shared_ptr actor_creator = + std::make_shared(gcs_client_); direct_actor_submitter_ = std::shared_ptr( new CoreWorkerDirectActorTaskSubmitter(client_factory, memory_store_, @@ -477,8 +469,9 @@ CoreWorker::CoreWorker(const CoreWorkerOptions &options, const WorkerID &worker_ rpc_address_, local_raylet_client_, client_factory, raylet_client_factory, memory_store_, task_manager_, local_raylet_id, RayConfig::instance().worker_lease_timeout_milliseconds(), + std::move(actor_creator), RayConfig::instance().max_tasks_in_flight_per_worker(), - std::move(actor_creator), boost::asio::steady_timer(io_service_))); + boost::asio::steady_timer(io_service_))); future_resolver_.reset(new FutureResolver(memory_store_, client_factory, rpc_address_)); // Unfortunately the raylet client has to be constructed after the receivers. if (direct_task_receiver_ != nullptr) { @@ -1426,7 +1419,6 @@ const ActorHandle *CoreWorker::GetActorHandle(const ActorID &actor_id) const { std::pair CoreWorker::GetNamedActorHandle( const std::string &name) { - RAY_CHECK(RayConfig::instance().gcs_actor_service_enabled()); RAY_CHECK(!name.empty()); if (options_.is_local_mode) { return GetNamedActorHandleLocalMode(name); @@ -1783,20 +1775,6 @@ Status CoreWorker::GetAndPinArgsForExecutor(const TaskSpecification &task, return Status::OK(); } -void CoreWorker::HandleAssignTask(const rpc::AssignTaskRequest &request, - rpc::AssignTaskReply *reply, - rpc::SendReplyCallback send_reply_callback) { - if (HandleWrongRecipient(WorkerID::FromBinary(request.intended_worker_id()), - send_reply_callback)) { - return; - } - - task_queue_length_ += 1; - task_execution_service_.post([=] { - raylet_task_receiver_->HandleAssignTask(request, reply, send_reply_callback); - }); -} - void CoreWorker::HandlePushTask(const rpc::PushTaskRequest &request, rpc::PushTaskReply *reply, rpc::SendReplyCallback send_reply_callback) { @@ -1882,7 +1860,6 @@ void CoreWorker::HandleWaitForActorOutOfScope( const rpc::WaitForActorOutOfScopeRequest &request, rpc::WaitForActorOutOfScopeReply *reply, rpc::SendReplyCallback send_reply_callback) { // Currently WaitForActorOutOfScope is only used when GCS actor service is enabled. - RAY_CHECK(RayConfig::instance().gcs_actor_service_enabled()); if (HandleWrongRecipient(WorkerID::FromBinary(request.intended_worker_id()), send_reply_callback)) { return; diff --git a/src/ray/core_worker/core_worker.h b/src/ray/core_worker/core_worker.h index f4d13ddd8..0549063ba 100644 --- a/src/ray/core_worker/core_worker.h +++ b/src/ray/core_worker/core_worker.h @@ -20,7 +20,6 @@ #include "ray/common/placement_group.h" #include "ray/core_worker/actor_handle.h" #include "ray/core_worker/actor_manager.h" -#include "ray/core_worker/actor_reporter.h" #include "ray/core_worker/common.h" #include "ray/core_worker/context.h" #include "ray/core_worker/future_resolver.h" @@ -31,7 +30,6 @@ #include "ray/core_worker/store_provider/plasma_store_provider.h" #include "ray/core_worker/transport/direct_actor_transport.h" #include "ray/core_worker/transport/direct_task_transport.h" -#include "ray/core_worker/transport/raylet_transport.h" #include "ray/gcs/redis_gcs_client.h" #include "ray/gcs/subscription_executor.h" #include "ray/raylet_client/raylet_client.h" @@ -735,11 +733,6 @@ class CoreWorker : public rpc::CoreWorkerServiceHandler { /// post work to the appropriate event loop. /// - /// Implements gRPC server handler. - void HandleAssignTask(const rpc::AssignTaskRequest &request, - rpc::AssignTaskReply *reply, - rpc::SendReplyCallback send_reply_callback) override; - /// Implements gRPC server handler. void HandlePushTask(const rpc::PushTaskRequest &request, rpc::PushTaskReply *reply, rpc::SendReplyCallback send_reply_callback) override; @@ -1035,9 +1028,6 @@ class CoreWorker : public rpc::CoreWorkerServiceHandler { // Tracks the currently pending tasks. std::shared_ptr task_manager_; - // Interface for publishing actor death event for actor creation failure. - std::shared_ptr actor_reporter_; - // Interface to submit tasks directly to other actors. std::shared_ptr direct_actor_submitter_; @@ -1095,9 +1085,6 @@ class CoreWorker : public rpc::CoreWorkerServiceHandler { /// of that resource allocated for this worker. This is set on task assignment. std::shared_ptr resource_ids_ GUARDED_BY(mutex_); - // Interface that receives tasks from the raylet. - std::unique_ptr raylet_task_receiver_; - /// Common rpc service for all worker modules. rpc::CoreWorkerGrpcService grpc_service_; diff --git a/src/ray/core_worker/task_manager.cc b/src/ray/core_worker/task_manager.cc index 96a378932..121d71d3d 100644 --- a/src/ray/core_worker/task_manager.cc +++ b/src/ray/core_worker/task_manager.cc @@ -445,12 +445,6 @@ void TaskManager::MarkPendingTaskFailed(const TaskID &task_id, const auto object_id = ObjectID::ForTaskReturn(task_id, /*index=*/i + 1); RAY_UNUSED(in_memory_store_->Put(RayObject(error_type), object_id)); } - - if (spec.IsActorCreationTask()) { - // Publish actor death if actor creation task failed after - // a number of retries. - actor_reporter_->PublishTerminatedActor(spec); - } } absl::optional TaskManager::GetTaskSpec(const TaskID &task_id) const { diff --git a/src/ray/core_worker/task_manager.h b/src/ray/core_worker/task_manager.h index 977d8e57c..83038c009 100644 --- a/src/ray/core_worker/task_manager.h +++ b/src/ray/core_worker/task_manager.h @@ -19,7 +19,6 @@ #include "absl/synchronization/mutex.h" #include "ray/common/id.h" #include "ray/common/task/task.h" -#include "ray/core_worker/actor_reporter.h" #include "ray/core_worker/store_provider/memory_store/memory_store.h" #include "src/ray/protobuf/core_worker.pb.h" #include "src/ray/protobuf/gcs.pb.h" @@ -58,13 +57,11 @@ class TaskManager : public TaskFinisherInterface, public TaskResubmissionInterfa public: TaskManager(std::shared_ptr in_memory_store, std::shared_ptr reference_counter, - std::shared_ptr actor_reporter, RetryTaskCallback retry_task_callback, const std::function &check_node_alive, ReconstructObjectCallback reconstruct_object_callback) : in_memory_store_(in_memory_store), reference_counter_(reference_counter), - actor_reporter_(actor_reporter), retry_task_callback_(retry_task_callback), check_node_alive_(check_node_alive), reconstruct_object_callback_(reconstruct_object_callback) { @@ -234,9 +231,6 @@ class TaskManager : public TaskFinisherInterface, public TaskResubmissionInterfa /// submitted tasks (dependencies and return objects). std::shared_ptr reference_counter_; - // Interface for publishing actor creation. - std::shared_ptr actor_reporter_; - /// Called when a task should be retried. const RetryTaskCallback retry_task_callback_; diff --git a/src/ray/core_worker/test/actor_manager_test.cc b/src/ray/core_worker/test/actor_manager_test.cc index f8a871ae7..6d8ea0ee8 100644 --- a/src/ray/core_worker/test/actor_manager_test.cc +++ b/src/ray/core_worker/test/actor_manager_test.cc @@ -18,7 +18,6 @@ #include "gtest/gtest.h" #include "ray/common/task/task_spec.h" #include "ray/common/test_util.h" -#include "ray/core_worker/actor_reporter.h" #include "ray/core_worker/reference_count.h" #include "ray/core_worker/transport/direct_actor_transport.h" #include "ray/gcs/redis_accessor.h" diff --git a/src/ray/core_worker/test/direct_task_transport_test.cc b/src/ray/core_worker/test/direct_task_transport_test.cc index 80126d872..3f36170ad 100644 --- a/src/ray/core_worker/test/direct_task_transport_test.cc +++ b/src/ray/core_worker/test/direct_task_transport_test.cc @@ -166,6 +166,22 @@ class MockRayletClient : public WorkerLeaseInterface { std::list> cancel_callbacks = {}; }; +class MockActorCreator : public ActorCreatorInterface { + public: + MockActorCreator() {} + + Status RegisterActor(const TaskSpecification &task_spec) override { + return Status::OK(); + }; + + Status AsyncCreateActor(const TaskSpecification &task_spec, + const gcs::StatusCallback &callback) override { + return Status::OK(); + } + + ~MockActorCreator() {} +}; + TEST(TestMemoryStore, TestPromoteToPlasma) { size_t num_plasma_puts = 0; auto mem = std::make_shared( @@ -321,8 +337,10 @@ TEST(DirectTaskTransportTest, TestSubmitOneTask) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, - task_finisher, ClientID::Nil(), kLongTimeout); + task_finisher, ClientID::Nil(), kLongTimeout, + actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = @@ -355,8 +373,10 @@ TEST(DirectTaskTransportTest, TestHandleTaskFailure) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, - task_finisher, ClientID::Nil(), kLongTimeout); + task_finisher, ClientID::Nil(), kLongTimeout, + actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -382,8 +402,10 @@ TEST(DirectTaskTransportTest, TestConcurrentWorkerLeases) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, - task_finisher, ClientID::Nil(), kLongTimeout); + task_finisher, ClientID::Nil(), kLongTimeout, + actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -430,8 +452,10 @@ TEST(DirectTaskTransportTest, TestReuseWorkerLease) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, - task_finisher, ClientID::Nil(), kLongTimeout); + task_finisher, ClientID::Nil(), kLongTimeout, + actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -485,8 +509,10 @@ TEST(DirectTaskTransportTest, TestRetryLeaseCancellation) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, - task_finisher, ClientID::Nil(), kLongTimeout); + task_finisher, ClientID::Nil(), kLongTimeout, + actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -538,8 +564,10 @@ TEST(DirectTaskTransportTest, TestConcurrentCancellationAndSubmission) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, - task_finisher, ClientID::Nil(), kLongTimeout); + task_finisher, ClientID::Nil(), kLongTimeout, + actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -588,8 +616,10 @@ TEST(DirectTaskTransportTest, TestWorkerNotReusedOnError) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, - task_finisher, ClientID::Nil(), kLongTimeout); + task_finisher, ClientID::Nil(), kLongTimeout, + actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -629,8 +659,10 @@ TEST(DirectTaskTransportTest, TestWorkerNotReturnedOnExit) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, - task_finisher, ClientID::Nil(), kLongTimeout); + task_finisher, ClientID::Nil(), kLongTimeout, + actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -669,9 +701,10 @@ TEST(DirectTaskTransportTest, TestSpillback) { return client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, lease_client_factory, store, task_finisher, - ClientID::Nil(), kLongTimeout); + ClientID::Nil(), kLongTimeout, actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -726,9 +759,10 @@ TEST(DirectTaskTransportTest, TestSpillbackRoundTrip) { }; auto task_finisher = std::make_shared(); auto local_raylet_id = ClientID::FromRandom(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, lease_client_factory, store, task_finisher, - local_raylet_id, kLongTimeout); + local_raylet_id, kLongTimeout, actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -781,8 +815,10 @@ void TestSchedulingKey(const std::shared_ptr store, auto worker_client = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, - task_finisher, ClientID::Nil(), kLongTimeout); + task_finisher, ClientID::Nil(), kLongTimeout, + actor_creator); ASSERT_TRUE(submitter.SubmitTask(same1).ok()); ASSERT_TRUE(submitter.SubmitTask(same2).ok()); @@ -891,9 +927,10 @@ TEST(DirectTaskTransportTest, TestWorkerLeaseTimeout) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, task_finisher, ClientID::Nil(), - /*lease_timeout_ms=*/5); + /*lease_timeout_ms=*/5, actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -943,8 +980,10 @@ TEST(DirectTaskTransportTest, TestKillExecutingTask) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, - task_finisher, ClientID::Nil(), kLongTimeout); + task_finisher, ClientID::Nil(), kLongTimeout, + actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -988,8 +1027,10 @@ TEST(DirectTaskTransportTest, TestKillPendingTask) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, - task_finisher, ClientID::Nil(), kLongTimeout); + task_finisher, ClientID::Nil(), kLongTimeout, + actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -1014,8 +1055,10 @@ TEST(DirectTaskTransportTest, TestKillResolvingTask) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, - task_finisher, ClientID::Nil(), kLongTimeout); + task_finisher, ClientID::Nil(), kLongTimeout, + actor_creator); std::unordered_map empty_resources; ray::FunctionDescriptor empty_descriptor = ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""); @@ -1042,6 +1085,7 @@ TEST(DirectTaskTransportTest, TestPipeliningConcurrentWorkerLeases) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); // Set max_tasks_in_flight_per_worker to a value larger than 1 to enable the pipelining // of task submissions. This is done by passing a max_tasks_in_flight_per_worker @@ -1049,7 +1093,7 @@ TEST(DirectTaskTransportTest, TestPipeliningConcurrentWorkerLeases) { uint32_t max_tasks_in_flight_per_worker = 10; CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, task_finisher, ClientID::Nil(), kLongTimeout, - max_tasks_in_flight_per_worker); + actor_creator, max_tasks_in_flight_per_worker); // Prepare 20 tasks and save them in a vector. std::unordered_map empty_resources; @@ -1110,6 +1154,7 @@ TEST(DirectTaskTransportTest, TestPipeliningReuseWorkerLease) { auto store = std::make_shared(); auto factory = [&](const rpc::Address &addr) { return worker_client; }; auto task_finisher = std::make_shared(); + auto actor_creator = std::make_shared(); // Set max_tasks_in_flight_per_worker to a value larger than 1 to enable the pipelining // of task submissions. This is done by passing a max_tasks_in_flight_per_worker @@ -1117,7 +1162,7 @@ TEST(DirectTaskTransportTest, TestPipeliningReuseWorkerLease) { uint32_t max_tasks_in_flight_per_worker = 10; CoreWorkerDirectTaskSubmitter submitter(address, raylet_client, factory, nullptr, store, task_finisher, ClientID::Nil(), kLongTimeout, - max_tasks_in_flight_per_worker); + actor_creator, max_tasks_in_flight_per_worker); // prepare 30 tasks and save them in a vector std::unordered_map empty_resources; diff --git a/src/ray/core_worker/test/task_manager_test.cc b/src/ray/core_worker/test/task_manager_test.cc index fc800716a..82cbfd287 100644 --- a/src/ray/core_worker/test/task_manager_test.cc +++ b/src/ray/core_worker/test/task_manager_test.cc @@ -17,7 +17,6 @@ #include "gtest/gtest.h" #include "ray/common/task/task_spec.h" #include "ray/common/test_util.h" -#include "ray/core_worker/actor_reporter.h" #include "ray/core_worker/reference_count.h" #include "ray/core_worker/store_provider/memory_store/memory_store.h" @@ -35,14 +34,6 @@ TaskSpecification CreateTaskHelper(uint64_t num_returns, return task; } -class MockActorManager : public ActorReporterInterface { - void PublishTerminatedActor(const TaskSpecification &actor_creation_task) override { - num_terminations += 1; - } - - int num_terminations = 0; -}; - class TaskManagerTest : public ::testing::Test { public: TaskManagerTest(bool lineage_pinning_enabled = false) @@ -50,8 +41,7 @@ class TaskManagerTest : public ::testing::Test { reference_counter_(std::shared_ptr(new ReferenceCounter( rpc::Address(), /*distributed_ref_counting_enabled=*/true, lineage_pinning_enabled))), - actor_reporter_(std::shared_ptr(new MockActorManager())), - manager_(store_, reference_counter_, actor_reporter_, + manager_(store_, reference_counter_, [this](TaskSpecification &spec, bool delay) { num_retries_++; return Status::OK(); @@ -63,7 +53,6 @@ class TaskManagerTest : public ::testing::Test { std::shared_ptr store_; std::shared_ptr reference_counter_; - std::shared_ptr actor_reporter_; bool all_nodes_alive_ = true; std::vector objects_to_recover_; TaskManager manager_; diff --git a/src/ray/core_worker/transport/direct_task_transport.cc b/src/ray/core_worker/transport/direct_task_transport.cc index 469402e24..c9aa2cc05 100644 --- a/src/ray/core_worker/transport/direct_task_transport.cc +++ b/src/ray/core_worker/transport/direct_task_transport.cc @@ -21,7 +21,7 @@ namespace ray { Status CoreWorkerDirectTaskSubmitter::SubmitTask(TaskSpecification task_spec) { RAY_LOG(DEBUG) << "Submit task " << task_spec.TaskId(); - if (actor_creator_ && task_spec.IsActorCreationTask()) { + if (task_spec.IsActorCreationTask()) { // Synchronously register the actor to GCS server. // Previously, we asynchronously registered the actor after all its dependencies were // resolved. This caused a problem: if the owner of the actor dies before dependencies @@ -37,7 +37,7 @@ Status CoreWorkerDirectTaskSubmitter::SubmitTask(TaskSpecification task_spec) { resolver_.ResolveDependencies(task_spec, [this, task_spec]() { RAY_LOG(DEBUG) << "Task dependencies resolved " << task_spec.TaskId(); - if (actor_creator_ && task_spec.IsActorCreationTask()) { + if (task_spec.IsActorCreationTask()) { // If gcs actor management is enabled, the actor creation task will be sent to // gcs server directly after the in-memory dependent objects are resolved. For // more details please see the protocol of actor management based on gcs. diff --git a/src/ray/core_worker/transport/direct_task_transport.h b/src/ray/core_worker/transport/direct_task_transport.h index a53a298f0..0eb985b13 100644 --- a/src/ray/core_worker/transport/direct_task_transport.h +++ b/src/ray/core_worker/transport/direct_task_transport.h @@ -54,10 +54,9 @@ class CoreWorkerDirectTaskSubmitter { rpc::ClientFactoryFn client_factory, LeaseClientFactoryFn lease_client_factory, std::shared_ptr store, std::shared_ptr task_finisher, ClientID local_raylet_id, - int64_t lease_timeout_ms, + int64_t lease_timeout_ms, std::shared_ptr actor_creator, uint32_t max_tasks_in_flight_per_worker = RayConfig::instance().max_tasks_in_flight_per_worker(), - std::shared_ptr actor_creator = nullptr, absl::optional cancel_timer = absl::nullopt) : rpc_address_(rpc_address), local_lease_client_(lease_client), diff --git a/src/ray/core_worker/transport/raylet_transport.cc b/src/ray/core_worker/transport/raylet_transport.cc deleted file mode 100644 index 92bb046a1..000000000 --- a/src/ray/core_worker/transport/raylet_transport.cc +++ /dev/null @@ -1,87 +0,0 @@ -// Copyright 2017 The Ray Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "ray/core_worker/transport/raylet_transport.h" -#include "ray/common/common_protocol.h" -#include "ray/common/task/task.h" - -namespace ray { - -CoreWorkerRayletTaskReceiver::CoreWorkerRayletTaskReceiver( - const WorkerID &worker_id, std::shared_ptr &raylet_client, - const TaskHandler &task_handler) - : worker_id_(worker_id), raylet_client_(raylet_client), task_handler_(task_handler) {} - -void CoreWorkerRayletTaskReceiver::HandleAssignTask( - const rpc::AssignTaskRequest &request, rpc::AssignTaskReply *reply, - rpc::SendReplyCallback send_reply_callback) { - const Task task(request.task()); - const auto &task_spec = task.GetTaskSpecification(); - RAY_LOG(DEBUG) << "Received task " << task_spec.TaskId() << " is create " - << task_spec.IsActorCreationTask(); - - // Set the resource IDs for this task. - // TODO: convert the resource map to protobuf and change this. - auto resource_ids = std::make_shared(); - auto resource_infos = - flatbuffers::GetRoot(request.resource_ids().data()) - ->resource_infos(); - for (size_t i = 0; i < resource_infos->size(); ++i) { - auto const &fractional_resource_ids = resource_infos->Get(i); - auto &acquired_resources = - (*resource_ids)[string_from_flatbuf(*fractional_resource_ids->resource_name())]; - - size_t num_resource_ids = fractional_resource_ids->resource_ids()->size(); - size_t num_resource_fractions = fractional_resource_ids->resource_fractions()->size(); - RAY_CHECK(num_resource_ids == num_resource_fractions); - RAY_CHECK(num_resource_ids > 0); - for (size_t j = 0; j < num_resource_ids; ++j) { - int64_t resource_id = fractional_resource_ids->resource_ids()->Get(j); - double resource_fraction = fractional_resource_ids->resource_fractions()->Get(j); - if (num_resource_ids > 1) { - int64_t whole_fraction = resource_fraction; - RAY_CHECK(whole_fraction == resource_fraction); - } - acquired_resources.push_back(std::make_pair(resource_id, resource_fraction)); - } - } - - std::vector> results; - ReferenceCounter::ReferenceTableProto borrower_refs; - // NOTE(swang): Distributed ref counting does not work for the raylet - // transport. - auto status = task_handler_(task_spec, resource_ids, &results, &borrower_refs); - if (status.IsSystemExit()) { - return; - } - - RAY_LOG(DEBUG) << "Assigned task " << task_spec.TaskId() << " finished execution."; - - // Notify raylet that current task is done via a `TaskDone` message. This is to - // ensure that the task is marked as finished by raylet only after previous - // raylet client calls are completed. For example, if the worker sends a - // NotifyUnblocked message that it is no longer blocked in a `ray.get` - // on the normal raylet socket, then completes an assigned task, we - // need to guarantee that raylet gets the former message first before - // marking the task as completed. This is why a `TaskDone` message - // is required - without it, it's possible that raylet receives - // rpc reply first before the NotifyUnblocked message arrives, - // as they use different connections, the `TaskDone` message is sent - // to raylet via the same connection so the order is guaranteed. - RAY_UNUSED(raylet_client_->TaskDone()); - // Send rpc reply. - send_reply_callback(status, nullptr, nullptr); -} - -} // namespace ray diff --git a/src/ray/core_worker/transport/raylet_transport.h b/src/ray/core_worker/transport/raylet_transport.h deleted file mode 100644 index e05b1827b..000000000 --- a/src/ray/core_worker/transport/raylet_transport.h +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright 2017 The Ray Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#pragma once - -#include - -#include "ray/common/ray_object.h" -#include "ray/core_worker/reference_count.h" -#include "ray/raylet_client/raylet_client.h" -#include "ray/rpc/worker/core_worker_server.h" - -namespace ray { - -class CoreWorkerRayletTaskReceiver { - public: - using TaskHandler = - std::function &resource_ids, - std::vector> *return_objects, - ReferenceCounter::ReferenceTableProto *borrower_refs)>; - - CoreWorkerRayletTaskReceiver(const WorkerID &worker_id, - std::shared_ptr &raylet_client, - const TaskHandler &task_handler); - - /// Handle a `AssignTask` request. - /// The implementation can handle this request asynchronously. When handling is done, - /// the `send_reply_callback` should be called. - /// - /// \param[in] request The request message. - /// \param[out] reply The reply message. - /// \param[in] send_reply_callback The callback to be called when the request is done. - void HandleAssignTask(const rpc::AssignTaskRequest &request, - rpc::AssignTaskReply *reply, - rpc::SendReplyCallback send_reply_callback); - - private: - // WorkerID of this worker. - WorkerID worker_id_; - /// Reference to the core worker's raylet client. This is a pointer ref so that it - /// can be initialized by core worker after this class is constructed. - std::shared_ptr &raylet_client_; - /// The callback function to process a task. - TaskHandler task_handler_; - /// The callback to process arg wait complete. - std::function on_wait_complete_; -}; - -} // namespace ray diff --git a/src/ray/gcs/gcs_server/gcs_actor_manager.cc b/src/ray/gcs/gcs_server/gcs_actor_manager.cc index 8ae2fb4f6..278d8b7ed 100644 --- a/src/ray/gcs/gcs_server/gcs_actor_manager.cc +++ b/src/ray/gcs/gcs_server/gcs_actor_manager.cc @@ -944,9 +944,7 @@ void GcsActorManager::LoadInitialData(const EmptyCallback &done) { } // Notify raylets to release unused workers. - if (RayConfig::instance().gcs_actor_service_enabled()) { - gcs_actor_scheduler_->ReleaseUnusedWorkers(node_to_workers); - } + gcs_actor_scheduler_->ReleaseUnusedWorkers(node_to_workers); RAY_LOG(DEBUG) << "The number of registered actors is " << registered_actors_.size() << ", and the number of created actors is " << created_actors_.size(); diff --git a/src/ray/gcs/redis_gcs_client.cc b/src/ray/gcs/redis_gcs_client.cc index 8af7ca40e..a2f07e7bb 100644 --- a/src/ray/gcs/redis_gcs_client.cc +++ b/src/ray/gcs/redis_gcs_client.cc @@ -68,11 +68,7 @@ Status RedisGcsClient::Connect(boost::asio::io_service &io_service) { resource_table_.reset(new DynamicResourceTable({primary_context}, this)); worker_table_.reset(new WorkerTable(shard_contexts, this)); - if (RayConfig::instance().gcs_actor_service_enabled()) { - actor_accessor_.reset(new RedisActorInfoAccessor(this)); - } else { - actor_accessor_.reset(new RedisLogBasedActorInfoAccessor(this)); - } + actor_accessor_.reset(new RedisActorInfoAccessor(this)); job_accessor_.reset(new RedisJobInfoAccessor(this)); object_accessor_.reset(new RedisObjectInfoAccessor(this)); diff --git a/src/ray/protobuf/core_worker.proto b/src/ray/protobuf/core_worker.proto index a6e93aad5..51f0a9ecf 100644 --- a/src/ray/protobuf/core_worker.proto +++ b/src/ray/protobuf/core_worker.proto @@ -55,24 +55,6 @@ message ActorHandle { int64 max_task_retries = 9; } -message AssignTaskRequest { - // The ID of the worker this message is intended for. This is used to - // ensure that workers don't try to execute tasks assigned to workers - // that used to be bound to the same port. - bytes intended_worker_id = 1; - - // The task to be pushed. - Task task = 2; - - // A list of the resources reserved for this worker. - // TODO(zhijunfu): `resource_ids` is represented as - // flatbutters-serialized bytes, will be moved to protobuf later. - bytes resource_ids = 3; -} - -message AssignTaskReply { -} - message ReturnObject { // Object ID. bytes object_id = 1; @@ -286,8 +268,6 @@ message PlasmaObjectReadyReply { } service CoreWorkerService { - // Push a task to a worker from the raylet. - rpc AssignTask(AssignTaskRequest) returns (AssignTaskReply); // Push a task directly to this worker from another. rpc PushTask(PushTaskRequest) returns (PushTaskReply); // Reply from raylet that wait for direct actor call args has completed. diff --git a/src/ray/protobuf/node_manager.proto b/src/ray/protobuf/node_manager.proto index 584a7e4a1..e720c534c 100644 --- a/src/ray/protobuf/node_manager.proto +++ b/src/ray/protobuf/node_manager.proto @@ -89,17 +89,6 @@ message CancelWorkerLeaseReply { bool success = 1; } -message ForwardTaskRequest { - // The ID of the task to be forwarded. - bytes task_id = 1; - // The tasks in the uncommitted lineage of the forwarded task. This - // should include task_id. - repeated Task uncommitted_tasks = 2; -} - -message ForwardTaskReply { -} - message PinObjectIDsRequest { // Address of the owner to ask when to unpin the objects. Address owner_address = 1; @@ -171,8 +160,6 @@ service NodeManagerService { // Cancel a pending lease request. This only returns success if the // lease request was not yet granted. rpc CancelWorkerLease(CancelWorkerLeaseRequest) returns (CancelWorkerLeaseReply); - // Forward a task and its uncommitted lineage to the remote node manager. - rpc ForwardTask(ForwardTaskRequest) returns (ForwardTaskReply); // Pin the provided object IDs. rpc PinObjectIDs(PinObjectIDsRequest) returns (PinObjectIDsReply); // Get the current node stats. diff --git a/src/ray/raylet/format/node_manager.fbs b/src/ray/raylet/format/node_manager.fbs index 9cbdf62e7..c5457804f 100644 --- a/src/ray/raylet/format/node_manager.fbs +++ b/src/ray/raylet/format/node_manager.fbs @@ -55,22 +55,6 @@ enum MessageType:int { NotifyDirectCallTaskBlocked, // Notify the current worker is unblocked. This is only used by direct task calls. NotifyDirectCallTaskUnblocked, - // A request to get the task frontier for an actor, called by the actor when - // saving a checkpoint. - GetActorFrontierRequest, - // The ActorFrontier response to a GetActorFrontierRequest. The raylet - // returns the actor's per-handle task counts and execution dependencies, - // which can later be used as the argument to SetActorFrontier - // when resuming from the checkpoint. - GetActorFrontierReply, - // A request to set the task frontier for an actor, called when resuming from - // a checkpoint. The raylet will update the actor's per-handle task - // counts and execution dependencies, discard any tasks that already executed - // before the checkpoint, and make any tasks on the frontier runnable by - // making their execution dependencies available. - SetActorFrontier, - // A node manager request to process a task forwarded from another node manager. - ForwardTaskRequest, // Wait for objects to be ready either from local or remote Plasma stores. WaitRequest, // The response message to WaitRequest; replies with the objects found and objects @@ -182,14 +166,6 @@ table RegisterNodeManagerRequest { client_id: string; } -table ForwardTaskRequest { - // The ID of the task to be forwarded. - task_id: string; - // The tasks in the uncommitted lineage of the forwarded task. This - // should include task_id. - uncommitted_tasks: [Task]; -} - // Mimics the Address protobuf. table Address { raylet_id: string; diff --git a/src/ray/raylet/lineage_cache.cc b/src/ray/raylet/lineage_cache.cc deleted file mode 100644 index c48ea4790..000000000 --- a/src/ray/raylet/lineage_cache.cc +++ /dev/null @@ -1,412 +0,0 @@ -// Copyright 2017 The Ray Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "ray/raylet/lineage_cache.h" - -#include - -#include "ray/gcs/redis_gcs_client.h" -#include "ray/stats/stats.h" - -namespace ray { - -namespace raylet { - -LineageEntry::LineageEntry(const Task &task, GcsStatus status) - : status_(status), task_(task) { - ComputeParentTaskIds(); -} - -GcsStatus LineageEntry::GetStatus() const { return status_; } - -bool LineageEntry::SetStatus(GcsStatus new_status) { - if (status_ < new_status) { - status_ = new_status; - return true; - } else { - return false; - } -} - -void LineageEntry::ResetStatus(GcsStatus new_status) { - RAY_CHECK(new_status < status_); - status_ = new_status; -} - -void LineageEntry::MarkExplicitlyForwarded(const ClientID &node_id) { - forwarded_to_.insert(node_id); -} - -bool LineageEntry::WasExplicitlyForwarded(const ClientID &node_id) const { - return forwarded_to_.find(node_id) != forwarded_to_.end(); -} - -const TaskID LineageEntry::GetEntryId() const { - return task_.GetTaskSpecification().TaskId(); -} - -const std::unordered_set &LineageEntry::GetParentTaskIds() const { - return parent_task_ids_; -} - -void LineageEntry::ComputeParentTaskIds() { - parent_task_ids_.clear(); - // A task's parents are the tasks that created its arguments. - for (const auto &dependency : task_.GetDependencies()) { - parent_task_ids_.insert(ObjectID::FromBinary(dependency.object_id()).TaskId()); - } -} - -const Task &LineageEntry::TaskData() const { return task_; } - -Task &LineageEntry::TaskDataMutable() { return task_; } - -void LineageEntry::UpdateTaskData(const Task &task) { - task_.CopyTaskExecutionSpec(task); - ComputeParentTaskIds(); -} - -Lineage::Lineage() {} - -boost::optional Lineage::GetEntry(const TaskID &task_id) const { - auto entry = entries_.find(task_id); - if (entry != entries_.end()) { - return entry->second; - } else { - return boost::optional(); - } -} - -boost::optional Lineage::GetEntryMutable(const TaskID &task_id) { - auto entry = entries_.find(task_id); - if (entry != entries_.end()) { - return entry->second; - } else { - return boost::optional(); - } -} - -void Lineage::RemoveChild(const TaskID &parent_id, const TaskID &child_id) { - auto parent_it = children_.find(parent_id); - RAY_CHECK(parent_it->second.erase(child_id) == 1); - if (parent_it->second.empty()) { - children_.erase(parent_it); - } -} - -void Lineage::AddChild(const TaskID &parent_id, const TaskID &child_id) { - auto inserted = children_[parent_id].insert(child_id); - RAY_CHECK(inserted.second); -} - -bool Lineage::SetEntry(const Task &task, GcsStatus status) { - // Get the status of the current entry at the key. - auto task_id = task.GetTaskSpecification().TaskId(); - auto it = entries_.find(task_id); - bool updated = false; - if (it != entries_.end()) { - if (it->second.SetStatus(status)) { - // We assume here that the new `task` has the same fields as the task - // already in the lineage cache. If this is not true, then it is - // necessary to update the task data of the existing lineage cache entry - // with LineageEntry::UpdateTaskData. - updated = true; - } - } else { - LineageEntry new_entry(task, status); - it = entries_.emplace(std::make_pair(task_id, std::move(new_entry))).first; - updated = true; - - // New task data was added to the local cache, so record which tasks it - // depends on. Add all new tasks that it depends on. - for (const auto &parent_id : it->second.GetParentTaskIds()) { - AddChild(parent_id, task_id); - } - } - return updated; -} - -boost::optional Lineage::PopEntry(const TaskID &task_id) { - auto entry = entries_.find(task_id); - if (entry != entries_.end()) { - LineageEntry entry = std::move(entries_.at(task_id)); - - // Remove the task's dependencies. - for (const auto &parent_id : entry.GetParentTaskIds()) { - RemoveChild(parent_id, task_id); - } - entries_.erase(task_id); - - return entry; - } else { - return boost::optional(); - } -} - -const std::unordered_map &Lineage::GetEntries() const { - return entries_; -} - -const std::unordered_set &Lineage::GetChildren(const TaskID &task_id) const { - static const std::unordered_set empty_children; - const auto it = children_.find(task_id); - if (it != children_.end()) { - return it->second; - } else { - return empty_children; - } -} - -LineageCache::LineageCache(const ClientID &self_node_id, - std::shared_ptr gcs_client, - uint64_t max_lineage_size) - : self_node_id_(self_node_id), gcs_client_(gcs_client) {} - -/// A helper function to add some uncommitted lineage to the local cache. -void LineageCache::AddUncommittedLineage(const TaskID &task_id, - const Lineage &uncommitted_lineage) { - // TODO(edoakes): remove this method, it's currently only called in unit tests. - RAY_LOG(DEBUG) << "Adding uncommitted task " << task_id << " on " << self_node_id_; - // If the entry is not found in the lineage to merge, then we stop since - // there is nothing to copy into the merged lineage. - auto entry = uncommitted_lineage.GetEntry(task_id); - if (!entry) { - return; - } - RAY_CHECK(entry->GetStatus() == GcsStatus::UNCOMMITTED); - - // Insert a copy of the entry into our cache. - const auto &parent_ids = entry->GetParentTaskIds(); - // If the insert is successful, then continue the DFS. The insert will fail - // if the new entry has an equal or lower GCS status than the current entry - // in our cache. This also prevents us from traversing the same node twice. - if (lineage_.SetEntry(entry->TaskData(), entry->GetStatus())) { - RAY_CHECK(SubscribeTask(task_id)); - for (const auto &parent_id : parent_ids) { - AddUncommittedLineage(parent_id, uncommitted_lineage); - } - } -} - -bool LineageCache::CommitTask(const Task &task) { - // TODO(edoakes): remove this method, it's currently only called in unit tests. - const TaskID task_id = task.GetTaskSpecification().TaskId(); - RAY_LOG(DEBUG) << "Committing task " << task_id << " on " << self_node_id_; - - if (lineage_.SetEntry(task, GcsStatus::UNCOMMITTED) || - lineage_.GetEntry(task_id)->GetStatus() == GcsStatus::UNCOMMITTED) { - // Attempt to flush the task if the task is uncommitted. - FlushTask(task_id); - return true; - } else { - // The task was already committing (COMMITTING). - return false; - } -} - -void LineageCache::FlushAllUncommittedTasks() { - size_t num_flushed = 0; - for (const auto &entry : lineage_.GetEntries()) { - // Flush all tasks that have not yet committed. - if (entry.second.GetStatus() == GcsStatus::UNCOMMITTED) { - RAY_CHECK(UnsubscribeTask(entry.first)); - FlushTask(entry.first); - num_flushed++; - } - } - - RAY_LOG(DEBUG) << "Flushed " << num_flushed << " uncommitted tasks"; -} - -void LineageCache::MarkTaskAsForwarded(const TaskID &task_id, const ClientID &node_id) { - RAY_CHECK(!node_id.IsNil()); - auto entry = lineage_.GetEntryMutable(task_id); - if (entry) { - entry->MarkExplicitlyForwarded(node_id); - } -} - -/// A helper function to get the uncommitted lineage of a task. -void GetUncommittedLineageHelper(const TaskID &task_id, const Lineage &lineage_from, - Lineage &lineage_to, const ClientID &node_id) { - // If the entry is not found in the lineage to merge, then we stop since - // there is nothing to copy into the merged lineage. - auto entry = lineage_from.GetEntry(task_id); - if (!entry) { - return; - } - // If this task has already been forwarded to this node, then we can stop. - if (entry->WasExplicitlyForwarded(node_id)) { - return; - } - - // Insert a copy of the entry into lineage_to. If the insert is successful, - // then continue the DFS. The insert will fail if the new entry has an equal - // or lower GCS status than the current entry in lineage_to. This also - // prevents us from traversing the same node twice. - if (lineage_to.SetEntry(entry->TaskData(), entry->GetStatus())) { - for (const auto &parent_id : entry->GetParentTaskIds()) { - GetUncommittedLineageHelper(parent_id, lineage_from, lineage_to, node_id); - } - } -} - -Lineage LineageCache::GetUncommittedLineage(const TaskID &task_id, - const ClientID &node_id) const { - Lineage uncommitted_lineage; - // Add all uncommitted ancestors from the lineage cache to the uncommitted - // lineage of the requested task. - GetUncommittedLineageHelper(task_id, lineage_, uncommitted_lineage, node_id); - // The lineage always includes the requested task id, so add the task if it - // wasn't already added. The requested task may not have been added if it was - // already explicitly forwarded to this node before. - if (uncommitted_lineage.GetEntries().empty()) { - auto entry = lineage_.GetEntry(task_id); - if (entry) { - RAY_CHECK(uncommitted_lineage.SetEntry(entry->TaskData(), entry->GetStatus())); - } - } - return uncommitted_lineage; -} - -void LineageCache::FlushTask(const TaskID &task_id) { - auto entry = lineage_.GetEntryMutable(task_id); - RAY_CHECK(entry); - RAY_CHECK(entry->GetStatus() < GcsStatus::COMMITTING); - - auto task_callback = [this, task_id](Status status) { - RAY_CHECK(status.ok()); - HandleEntryCommitted(task_id); - }; - auto task = lineage_.GetEntry(task_id); - auto task_data = std::make_shared(); - task_data->mutable_task()->mutable_task_spec()->CopyFrom( - task->TaskData().GetTaskSpecification().GetMessage()); - task_data->mutable_task()->mutable_task_execution_spec()->CopyFrom( - task->TaskData().GetTaskExecutionSpec().GetMessage()); - RAY_CHECK_OK(gcs_client_->Tasks().AsyncAdd(task_data, task_callback)); - - // We successfully wrote the task, so mark it as committing. - // TODO(swang): Use a batched interface and write with all object entries. - RAY_CHECK(entry->SetStatus(GcsStatus::COMMITTING)); -} - -bool LineageCache::SubscribeTask(const TaskID &task_id) { - auto inserted = subscribed_tasks_.insert(task_id); - bool unsubscribed = inserted.second; - if (unsubscribed) { - auto subscribe = [this](const TaskID &task_id, const TaskTableData) { - HandleEntryCommitted(task_id); - }; - // Subscribe to the task. - RAY_CHECK_OK(gcs_client_->Tasks().AsyncSubscribe(task_id, subscribe, - /*done*/ nullptr)); - } - // Return whether we were previously unsubscribed to this task and are now - // subscribed. - return unsubscribed; -} - -bool LineageCache::UnsubscribeTask(const TaskID &task_id) { - auto it = subscribed_tasks_.find(task_id); - bool subscribed = (it != subscribed_tasks_.end()); - if (subscribed) { - // Cancel subscribe to the task. - RAY_CHECK_OK(gcs_client_->Tasks().AsyncUnsubscribe(task_id)); - subscribed_tasks_.erase(it); - } - // Return whether we were previously subscribed to this task and are now - // unsubscribed. - return subscribed; -} - -void LineageCache::EvictTask(const TaskID &task_id) { - // If the entry has already been evicted, exit. - auto entry = lineage_.GetEntry(task_id); - if (!entry) { - return; - } - // If the entry has not yet been committed, exit. - if (entry->GetStatus() != GcsStatus::COMMITTED) { - return; - } - // Entries cannot be safely evicted until their parents are all evicted. - for (const auto &parent_id : entry->GetParentTaskIds()) { - if (ContainsTask(parent_id)) { - return; - } - } - - // Evict the task. - RAY_LOG(DEBUG) << "Evicting task " << task_id << " on " << self_node_id_; - lineage_.PopEntry(task_id); - // Try to evict the children of the evict task. These are the tasks that have - // a dependency on the evicted task. - const auto children = lineage_.GetChildren(task_id); - for (const auto &child_id : children) { - EvictTask(child_id); - } -} - -void LineageCache::HandleEntryCommitted(const TaskID &task_id) { - RAY_LOG(DEBUG) << "Task committed: " << task_id; - auto entry = lineage_.GetEntryMutable(task_id); - if (!entry) { - // The task has already been evicted due to a previous commit notification. - return; - } - // Record the commit acknowledgement and attempt to evict the task. - entry->SetStatus(GcsStatus::COMMITTED); - EvictTask(task_id); - // We got the notification about the task's commit, so no longer need any - // more notifications. - UnsubscribeTask(task_id); -} - -const Task &LineageCache::GetTaskOrDie(const TaskID &task_id) const { - const auto &entries = lineage_.GetEntries(); - auto it = entries.find(task_id); - RAY_CHECK(it != entries.end()); - return it->second.TaskData(); -} - -bool LineageCache::ContainsTask(const TaskID &task_id) const { - const auto &entries = lineage_.GetEntries(); - auto it = entries.find(task_id); - return it != entries.end(); -} - -const Lineage &LineageCache::GetLineage() const { return lineage_; } - -std::string LineageCache::DebugString() const { - std::stringstream result; - result << "LineageCache:"; - result << "\n- child map size: " << lineage_.GetChildrenSize(); - result << "\n- num subscribed tasks: " << subscribed_tasks_.size(); - result << "\n- lineage size: " << lineage_.GetEntries().size(); - return result.str(); -} - -void LineageCache::RecordMetrics() const { - stats::LineageCacheStats().Record(lineage_.GetChildrenSize(), - {{stats::ValueTypeKey, "num_children"}}); - stats::LineageCacheStats().Record(subscribed_tasks_.size(), - {{stats::ValueTypeKey, "num_subscribed_tasks"}}); - stats::LineageCacheStats().Record(lineage_.GetEntries().size(), - {{stats::ValueTypeKey, "num_lineages"}}); -} - -} // namespace raylet - -} // namespace ray diff --git a/src/ray/raylet/lineage_cache.h b/src/ray/raylet/lineage_cache.h deleted file mode 100644 index 36b219f0a..000000000 --- a/src/ray/raylet/lineage_cache.h +++ /dev/null @@ -1,327 +0,0 @@ -// Copyright 2017 The Ray Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#pragma once - -#include -#include - -#include "ray/common/id.h" -#include "ray/common/status.h" -#include "ray/common/task/task.h" -#include "ray/gcs/redis_gcs_client.h" - -namespace ray { - -namespace raylet { - -using rpc::TaskTableData; - -/// The status of a lineage cache entry according to its status in the GCS. -/// Tasks can only transition to a higher GcsStatus (e.g., an UNCOMMITTED state -/// can become COMMITTING but not vice versa). If a task is evicted from the -/// local cache, it implicitly goes back to state `NONE`, after which it may be -/// added to the local cache again (e.g., if it is forwarded to us again). -enum class GcsStatus { - /// The task is not in the lineage cache. - NONE = 0, - /// The task is uncommitted. Unless there is a failure, we will expect a - /// different node to commit this task. - UNCOMMITTED, - /// We flushed this task and are waiting for the commit acknowledgement. - COMMITTING, - // Tasks for which we received a commit acknowledgement, but which we cannot - // evict yet (due to an ancestor that has not been evicted). This is to allow - // a performance optimization that avoids unnecessary subscribes when we - // receive tasks that were already COMMITTED at the sender. - COMMITTED, -}; - -/// \class LineageEntry -/// -/// A task entry in the data lineage. Each entry's parents are the tasks that -/// created the entry's arguments. -class LineageEntry { - public: - /// Create an entry for a task. - /// - /// \param task The task data to eventually be written back to the GCS. - /// \param status The status of this entry, according to its write status in - /// the GCS. - LineageEntry(const Task &task, GcsStatus status); - - /// Get this entry's GCS status. - /// - /// \return The entry's status in the GCS. - GcsStatus GetStatus() const; - - /// Set this entry's GCS status. The status is only set if the new status - /// is strictly greater than the entry's previous status, according to the - /// GcsStatus enum. - /// - /// \param new_status Set the entry's status to this value if it is greater - /// than the current status. - /// \return Whether the entry was set to the new status. - bool SetStatus(GcsStatus new_status); - - /// Reset this entry's GCS status to a lower status. The new status must - /// be lower than the current status. - /// - /// \param new_status This must be lower than the current status. - void ResetStatus(GcsStatus new_status); - - /// Mark this entry as having been explicitly forwarded to a remote node manager. - /// - /// \param node_id The ID of the remote node manager. - void MarkExplicitlyForwarded(const ClientID &node_id); - - /// Gets whether this entry was explicitly forwarded to a remote node. - /// - /// \param node_id The ID of the remote node manager. - /// \return Whether this entry was explicitly forwarded to the remote node. - bool WasExplicitlyForwarded(const ClientID &node_id) const; - - /// Get this entry's ID. - /// - /// \return The entry's ID. - const TaskID GetEntryId() const; - - /// Get the IDs of this entry's parent tasks. These are the IDs of the tasks - /// that created its arguments. - /// - /// \return The IDs of the parent entries. - const std::unordered_set &GetParentTaskIds() const; - - /// Get the task data. - /// - /// \return The task data. - const Task &TaskData() const; - - /// Get a mutable version of the task data. - /// - /// \return The task data. - /// TODO(swang): This is pretty ugly. - Task &TaskDataMutable(); - - /// Update the task data with a new task. - /// - /// \return Void. - void UpdateTaskData(const Task &task); - - private: - /// Compute cached parent task IDs. This task is dependent on values returned - /// by these tasks. - void ComputeParentTaskIds(); - - /// The current state of this entry according to its status in the GCS. - GcsStatus status_; - /// The task data to be written to the GCS. This is nullptr if the entry is - /// an object. - // const Task task_; - Task task_; - /// A cached copy of the parent task IDs. This task is dependent on values - /// returned by these tasks. - std::unordered_set parent_task_ids_; - /// IDs of node managers that this task has been explicitly forwarded to. - std::unordered_set forwarded_to_; -}; - -/// \class Lineage -/// -/// A lineage DAG, according to the data dependency graph. Each node is a task, -/// with an outgoing edge to each of its parent tasks. For a given task, the -/// parents are the tasks that created its arguments. Each entry also records -/// the current status in the GCS for that task or object. -class Lineage { - public: - /// Construct an empty Lineage. - Lineage(); - - /// Get an entry from the lineage. - /// - /// \param entry_id The ID of the entry to get. - /// \return An optional reference to the entry. If this is empty, then the - /// entry ID is not in the lineage. - boost::optional GetEntry(const TaskID &entry_id) const; - boost::optional GetEntryMutable(const TaskID &task_id); - - /// Set an entry in the lineage. If an entry with this ID already exists, - /// then the entry is overwritten if and only if the new entry has a higher - /// GCS status than the current. The current entry's object or task data will - /// also be overwritten. - /// - /// \param task The task data to set, if status is greater than the current entry. - /// \param status The GCS status. - /// \return Whether the entry was set. - bool SetEntry(const Task &task, GcsStatus status); - - /// Delete and return an entry from the lineage. - /// - /// \param entry_id The ID of the entry to pop. - /// \return An optional reference to the popped entry. If this is empty, then - /// the entry ID is not in the lineage. - boost::optional PopEntry(const TaskID &entry_id); - - /// Get all entries in the lineage. - /// - /// \return A const reference to the lineage entries. - const std::unordered_map &GetEntries() const; - - /// Return the IDs of tasks in the lineage that are dependent on the given - /// task. - /// - /// \param The ID of the task whose children to get. - /// \return The list of IDs for tasks that are in the lineage and dependent - /// on the given task. - const std::unordered_set &GetChildren(const TaskID &task_id) const; - - /// Return the size of the children_ map. This is used for debugging purposes - /// only. - size_t GetChildrenSize() const { return children_.size(); } - - private: - /// The lineage entries. - std::unordered_map entries_; - /// A mapping from each task in the lineage to its children. - std::unordered_map> children_; - - /// Record the fact that the child task depends on the parent task. - void AddChild(const TaskID &parent_id, const TaskID &child_id); - /// Erase the fact that the child task depends on the parent task. - void RemoveChild(const TaskID &parent_id, const TaskID &child_id); -}; - -/// \class LineageCache -/// -/// A cache of the task table. This consists of all tasks that this node owns, -/// as well as their lineage, that have not yet been added durably -/// ("committed") to the GCS. -/// -/// The current policy is to flush each task as soon as it enters the -/// UNCOMMITTED_READY state. For safety, we only evict tasks if they have been -/// committed and if their parents have been all evicted. Thus, the invariant -/// is that if g depends on f, and g has been evicted, then f must have been -/// committed. -class LineageCache { - public: - /// Create a lineage cache for the given task storage system. - /// TODO(swang): Pass in the policy (interface?). - LineageCache(const ClientID &self_node_id, std::shared_ptr gcs_client, - uint64_t max_lineage_size); - - /// Asynchronously commit a task to the GCS. - /// - /// \param task The task to commit. It will be moved to the COMMITTING state. - /// \return Whether the task was successfully committed. This can fail if the - /// task was already in the COMMITTING state. - bool CommitTask(const Task &task); - - /// Flush all tasks in the local cache that are not already being - /// committed. This is equivalent to all tasks in the UNCOMMITTED - /// state. - /// - /// \return Void. - void FlushAllUncommittedTasks(); - - /// Add a task and its (estimated) uncommitted lineage to the local cache. We - /// will subscribe to commit notifications for all uncommitted tasks to - /// determine when it is safe to evict the lineage from the local cache. - /// - /// \param task_id The ID of the uncommitted task to add. - /// \param uncommitted_lineage The task's uncommitted lineage. These are the - /// tasks that the given task is data-dependent on, but that have not - /// been committed to the GCS. This must contain the given task ID. - /// \return Void. - void AddUncommittedLineage(const TaskID &task_id, const Lineage &uncommitted_lineage); - - /// Mark a task as having been explicitly forwarded to a node. - /// The lineage of the task is implicitly assumed to have also been forwarded. - /// - /// \param task_id The ID of the task to get the uncommitted lineage for. - /// \param node_id The ID of the node to get the uncommitted lineage for. - void MarkTaskAsForwarded(const TaskID &task_id, const ClientID &node_id); - - /// Get the uncommitted lineage of a task that hasn't been forwarded to a node yet. - /// The uncommitted lineage consists of all tasks in the given task's lineage - /// that have not been committed in the GCS, as far as we know. - /// - /// \param task_id The ID of the task to get the uncommitted lineage for. If - /// the task is not found, then the returned lineage will be empty. - /// \param node_id The ID of the receiving node. - /// \return The uncommitted, unforwarded lineage of the task. The returned lineage - /// includes the entry for the requested entry_id. - Lineage GetUncommittedLineage(const TaskID &task_id, const ClientID &node_id) const; - - /// Handle the commit of a task entry in the GCS. This attempts to evict the - /// task if possible. - /// - /// \param task_id The ID of the task entry that was committed. - void HandleEntryCommitted(const TaskID &task_id); - - /// Get a task. The task must be in the lineage cache. - /// - /// \param task_id The ID of the task to get. - /// \return A const reference to the task data. - const Task &GetTaskOrDie(const TaskID &task_id) const; - - /// Get whether the lineage cache contains the task. - /// - /// \param task_id The ID of the task to get. - /// \return Whether the task is in the lineage cache. - bool ContainsTask(const TaskID &task_id) const; - - /// Get all lineage in the lineage cache. - /// - /// \return A const reference to the lineage. - const Lineage &GetLineage() const; - - /// Returns debug string for class. - /// - /// \return string. - std::string DebugString() const; - - /// Record metrics. - void RecordMetrics() const; - - private: - FRIEND_TEST(LineageCacheTest, BarReturnsZeroOnNull); - /// Flush a task that is in UNCOMMITTED_READY state. - void FlushTask(const TaskID &task_id); - /// Evict a single task. This should only be called if we are sure that the - /// task has been committed. The task will only be evicted if all of its - /// parents have also been evicted. If successful, then we will also attempt - /// to evict the task's children. - void EvictTask(const TaskID &task_id); - /// Subscribe to notifications for a task. Returns whether the operation - /// was successful (whether we were not already subscribed). - bool SubscribeTask(const TaskID &task_id); - /// Unsubscribe from notifications for a task. Returns whether the operation - /// was successful (whether we were subscribed). - bool UnsubscribeTask(const TaskID &task_id); - - /// ID of this node. - ClientID self_node_id_; - /// A client connection to the GCS. - std::shared_ptr gcs_client_; - /// All tasks and objects that we are responsible for writing back to the - /// GCS, and the tasks and objects in their lineage. - Lineage lineage_; - /// The tasks that we've subscribed to. - /// We will receive a notification for these tasks on commit. - std::unordered_set subscribed_tasks_; -}; - -} // namespace raylet - -} // namespace ray diff --git a/src/ray/raylet/lineage_cache_test.cc b/src/ray/raylet/lineage_cache_test.cc deleted file mode 100644 index 44cab43c7..000000000 --- a/src/ray/raylet/lineage_cache_test.cc +++ /dev/null @@ -1,670 +0,0 @@ -// Copyright 2017 The Ray Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "ray/raylet/lineage_cache.h" - -#include -#include - -#include "gmock/gmock.h" -#include "gtest/gtest.h" -#include "ray/common/task/task.h" -#include "ray/common/task/task_execution_spec.h" -#include "ray/common/task/task_spec.h" -#include "ray/common/task/task_util.h" -#include "ray/common/test_util.h" -#include "ray/gcs/callback.h" -#include "ray/gcs/redis_accessor.h" -#include "ray/gcs/redis_gcs_client.h" -#include "ray/raylet/format/node_manager_generated.h" - -namespace ray { - -namespace raylet { - -const static JobID kDefaultJobId = JobID::FromInt(1); - -const static TaskID kDefaultDriverTaskId = TaskID::ForDriverTask(kDefaultJobId); - -class MockGcsClient; - -class MockTaskInfoAccessor : public gcs::RedisTaskInfoAccessor { - public: - MockTaskInfoAccessor(gcs::RedisGcsClient *gcs_client) - : RedisTaskInfoAccessor(gcs_client) {} - - virtual ~MockTaskInfoAccessor() {} - - void RegisterSubscribeCallback( - const gcs::SubscribeCallback ¬ification_callback) { - notification_callback_ = notification_callback; - } - - Status AsyncAdd(const std::shared_ptr &task_data, - const gcs::StatusCallback &done) { - TaskID task_id = TaskID::FromBinary(task_data->task().task_spec().task_id()); - task_table_[task_id] = task_data; - auto callback = done; - // If we requested notifications for this task ID, send the notification as - // part of the callback. - if (subscribed_tasks_.count(task_id) == 1) { - callback = [this, done, task_id, task_data](Status status) { - done(status); - // If we're subscribed to the task to be added, also send a - // subscription notification. - notification_callback_(task_id, *task_data); - }; - } - - callbacks_.push_back({callback, task_id}); - num_task_adds_++; - return ray::Status::OK(); - } - - Status RemoteAdd(std::shared_ptr task_data) { - TaskID task_id = TaskID::FromBinary(task_data->task().task_spec().task_id()); - task_table_[task_id] = task_data; - // Send a notification after the add if the lineage cache requested - // notifications for this key. - bool send_notification = (subscribed_tasks_.count(task_id) == 1); - auto callback = [this, send_notification, task_id, task_data](Status status) { - if (send_notification) { - notification_callback_(task_id, *task_data); - } - }; - return AsyncAdd(task_data, callback); - } - - Status AsyncSubscribe( - const TaskID &task_id, - const gcs::SubscribeCallback ¬ification_callback, - const gcs::StatusCallback &done) { - subscribed_tasks_.insert(task_id); - if (task_table_.count(task_id) == 1) { - notification_callbacks_.push_back({notification_callback_, task_id}); - } - num_requested_notifications_ += 1; - return ray::Status::OK(); - } - - Status AsyncUnsubscribe(const TaskID &task_id) { - subscribed_tasks_.erase(task_id); - return ray::Status::OK(); - } - - void Flush() { - auto callbacks = std::move(callbacks_); - callbacks_.clear(); - for (const auto &callback : callbacks) { - callback.first(Status::OK()); - } - for (const auto &callback : notification_callbacks_) { - callback.first(callback.second, *task_table_[callback.second]); - } - } - - const std::unordered_map> &TaskTable() const { - return task_table_; - } - - const std::unordered_set &SubscribedTasks() const { return subscribed_tasks_; } - - const int NumRequestedNotifications() const { return num_requested_notifications_; } - - const int NumTaskAdds() const { return num_task_adds_; } - - private: - std::unordered_map> task_table_; - std::vector> callbacks_; - - typedef gcs::SubscribeCallback TaskSubscribeCallback; - TaskSubscribeCallback notification_callback_; - std::vector> notification_callbacks_; - - std::unordered_set subscribed_tasks_; - int num_requested_notifications_ = 0; - int num_task_adds_ = 0; -}; - -class MockNodeInfoAccessor : public gcs::RedisNodeInfoAccessor { - public: - MockNodeInfoAccessor(gcs::RedisGcsClient *gcs_client, const ClientID &node_id) - : RedisNodeInfoAccessor(gcs_client), node_id_(node_id) {} - - const ClientID &GetSelfId() const override { return node_id_; } - - private: - ClientID node_id_; -}; - -class MockGcsClient : public gcs::RedisGcsClient { - public: - MockGcsClient(const gcs::GcsClientOptions &options, const ClientID &node_id) - : RedisGcsClient(options) { - task_table_fake_.reset(new gcs::raylet::TaskTable({nullptr}, this)); - task_accessor_.reset(new MockTaskInfoAccessor(this)); - node_accessor_.reset(new MockNodeInfoAccessor(this, node_id)); - } - - gcs::raylet::TaskTable &raylet_task_table() override { return *task_table_fake_; } - - MockTaskInfoAccessor &MockTasks() { - return *dynamic_cast(task_accessor_.get()); - } - - private: - std::unique_ptr task_table_fake_; -}; - -class LineageCacheTest : public ::testing::Test { - public: - LineageCacheTest() : max_lineage_size_(10), num_notifications_(0) { - gcs::GcsClientOptions options("10.10.10.10", 12100, ""); - mock_gcs_ = std::make_shared(options, node_id_); - - lineage_cache_.reset(new LineageCache(node_id_, mock_gcs_, max_lineage_size_)); - - mock_gcs_->MockTasks().RegisterSubscribeCallback( - [this](const TaskID &task_id, const TaskTableData &data) { - lineage_cache_->HandleEntryCommitted(task_id); - num_notifications_++; - }); - } - - protected: - uint64_t max_lineage_size_; - uint64_t num_notifications_; - ClientID node_id_{ClientID::FromRandom()}; - std::shared_ptr mock_gcs_; - std::unique_ptr lineage_cache_; -}; - -static inline Task ExampleTask(const std::vector &arguments, - uint64_t num_returns) { - TaskSpecBuilder builder; - rpc::Address address; - builder.SetCommonTaskSpec(RandomTaskId(), Language::PYTHON, - ray::FunctionDescriptorBuilder::BuildPython("", "", "", ""), - JobID::Nil(), RandomTaskId(), 0, RandomTaskId(), address, - num_returns, {}, {}); - for (const auto &arg : arguments) { - builder.AddArg(TaskArgByReference(arg, rpc::Address())); - } - rpc::TaskExecutionSpec execution_spec_message; - execution_spec_message.set_num_forwards(1); - return Task(builder.Build(), TaskExecutionSpecification(execution_spec_message)); -} - -/// Helper method to create a Lineage object with a single task. -Lineage CreateSingletonLineage(const Task &task) { - Lineage singleton_lineage; - singleton_lineage.SetEntry(task, GcsStatus::UNCOMMITTED); - return singleton_lineage; -} - -std::vector InsertTaskChain(LineageCache &lineage_cache, - std::vector &inserted_tasks, int chain_size, - const std::vector &initial_arguments, - int64_t num_returns) { - std::vector arguments = initial_arguments; - for (int i = 0; i < chain_size; i++) { - auto task = ExampleTask(arguments, num_returns); - Lineage lineage = CreateSingletonLineage(task); - lineage_cache.AddUncommittedLineage(task.GetTaskSpecification().TaskId(), lineage); - inserted_tasks.push_back(task); - arguments.clear(); - for (size_t j = 0; j < task.GetTaskSpecification().NumReturns(); j++) { - arguments.push_back(task.GetTaskSpecification().ReturnId(j)); - } - } - return arguments; -} - -TEST_F(LineageCacheTest, TestGetUncommittedLineage) { - // Insert two independent chains of tasks. - std::vector tasks1; - auto return_values1 = - InsertTaskChain(*lineage_cache_, tasks1, 3, std::vector(), 1); - std::vector task_ids1; - for (const auto &task : tasks1) { - task_ids1.push_back(task.GetTaskSpecification().TaskId()); - } - - std::vector tasks2; - auto return_values2 = - InsertTaskChain(*lineage_cache_, tasks2, 2, std::vector(), 2); - std::vector task_ids2; - for (const auto &task : tasks2) { - task_ids2.push_back(task.GetTaskSpecification().TaskId()); - } - - // Get the uncommitted lineage for the last task (the leaf) of one of the chains. - auto uncommitted_lineage = - lineage_cache_->GetUncommittedLineage(task_ids1.back(), ClientID::Nil()); - // Check that the uncommitted lineage is exactly equal to the first chain of tasks. - ASSERT_EQ(task_ids1.size(), uncommitted_lineage.GetEntries().size()); - for (auto &task_id : task_ids1) { - ASSERT_TRUE(uncommitted_lineage.GetEntry(task_id)); - } - - // Insert one task that is dependent on the previous chains of tasks. - std::vector combined_tasks = tasks1; - combined_tasks.insert(combined_tasks.end(), tasks2.begin(), tasks2.end()); - std::vector combined_arguments = return_values1; - combined_arguments.insert(combined_arguments.end(), return_values2.begin(), - return_values2.end()); - InsertTaskChain(*lineage_cache_, combined_tasks, 1, combined_arguments, 1); - std::vector combined_task_ids; - for (const auto &task : combined_tasks) { - combined_task_ids.push_back(task.GetTaskSpecification().TaskId()); - } - - // Get the uncommitted lineage for the inserted task. - uncommitted_lineage = - lineage_cache_->GetUncommittedLineage(combined_task_ids.back(), ClientID::Nil()); - // Check that the uncommitted lineage is exactly equal to the entire set of - // tasks inserted so far. - ASSERT_EQ(combined_task_ids.size(), uncommitted_lineage.GetEntries().size()); - for (auto &task_id : combined_task_ids) { - ASSERT_TRUE(uncommitted_lineage.GetEntry(task_id)); - } -} - -TEST_F(LineageCacheTest, TestDuplicateUncommittedLineage) { - // Insert a chain of tasks. - std::vector tasks; - auto return_values = - InsertTaskChain(*lineage_cache_, tasks, 3, std::vector(), 1); - std::vector task_ids; - for (const auto &task : tasks) { - task_ids.push_back(task.GetTaskSpecification().TaskId()); - } - // Check that we subscribed to each of the uncommitted tasks. - ASSERT_EQ(mock_gcs_->MockTasks().NumRequestedNotifications(), task_ids.size()); - - // Check that if we add the same tasks as UNCOMMITTED again, we do not issue - // duplicate subscribe requests. - Lineage duplicate_lineage; - for (const auto &task : tasks) { - duplicate_lineage.SetEntry(task, GcsStatus::UNCOMMITTED); - } - lineage_cache_->AddUncommittedLineage(task_ids.back(), duplicate_lineage); - ASSERT_EQ(mock_gcs_->MockTasks().NumRequestedNotifications(), task_ids.size()); - - // Check that if we commit one of the tasks, we still do not issue any - // duplicate subscribe requests. - lineage_cache_->CommitTask(tasks.front()); - lineage_cache_->AddUncommittedLineage(task_ids.back(), duplicate_lineage); - ASSERT_EQ(mock_gcs_->MockTasks().NumRequestedNotifications(), task_ids.size()); -} - -TEST_F(LineageCacheTest, TestMarkTaskAsForwarded) { - // Insert chain of tasks. - std::vector tasks; - auto return_values = - InsertTaskChain(*lineage_cache_, tasks, 4, std::vector(), 1); - std::vector task_ids; - for (const auto &task : tasks) { - task_ids.push_back(task.GetTaskSpecification().TaskId()); - } - - auto node_id = ClientID::FromRandom(); - auto node_id2 = ClientID::FromRandom(); - auto forwarded_task_id = task_ids[task_ids.size() - 2]; - auto remaining_task_id = task_ids[task_ids.size() - 1]; - lineage_cache_->MarkTaskAsForwarded(forwarded_task_id, node_id); - - auto uncommitted_lineage = - lineage_cache_->GetUncommittedLineage(remaining_task_id, node_id); - auto uncommitted_lineage_all = - lineage_cache_->GetUncommittedLineage(remaining_task_id, node_id2); - - ASSERT_EQ(1, uncommitted_lineage.GetEntries().size()); - ASSERT_EQ(4, uncommitted_lineage_all.GetEntries().size()); - ASSERT_TRUE(uncommitted_lineage.GetEntry(remaining_task_id)); - - // Check that lineage of requested task includes itself, regardless of whether - // it has been forwarded before. - auto uncommitted_lineage_forwarded = - lineage_cache_->GetUncommittedLineage(forwarded_task_id, node_id); - ASSERT_EQ(1, uncommitted_lineage_forwarded.GetEntries().size()); -} - -TEST_F(LineageCacheTest, TestWritebackReady) { - // Insert a chain of dependent tasks. - size_t num_tasks_flushed = 0; - std::vector tasks; - InsertTaskChain(*lineage_cache_, tasks, 3, std::vector(), 1); - - // Check that when no tasks have been marked as ready, we do not flush any - // entries. - ASSERT_EQ(mock_gcs_->MockTasks().TaskTable().size(), num_tasks_flushed); - - // Check that after marking the first task as ready, we flush only that task. - ASSERT_TRUE(lineage_cache_->CommitTask(tasks.front())); - num_tasks_flushed++; - ASSERT_EQ(mock_gcs_->MockTasks().TaskTable().size(), num_tasks_flushed); -} - -TEST_F(LineageCacheTest, TestWritebackOrder) { - // Insert a chain of dependent tasks. - std::vector tasks; - InsertTaskChain(*lineage_cache_, tasks, 3, std::vector(), 1); - size_t num_tasks_flushed = tasks.size(); - - // Mark all tasks as ready. All tasks should be flushed. - for (const auto &task : tasks) { - ASSERT_TRUE(lineage_cache_->CommitTask(task)); - } - - ASSERT_EQ(mock_gcs_->MockTasks().TaskTable().size(), num_tasks_flushed); -} - -TEST_F(LineageCacheTest, TestEvictChain) { - // Create a chain of 3 tasks. - size_t num_tasks_flushed = 0; - std::vector tasks; - std::vector arguments; - for (int i = 0; i < 3; i++) { - auto task = ExampleTask(arguments, 1); - tasks.push_back(task); - arguments = {task.GetTaskSpecification().ReturnId(0)}; - } - - Lineage uncommitted_lineage; - for (const auto &task : tasks) { - uncommitted_lineage.SetEntry(task, GcsStatus::UNCOMMITTED); - } - // Mark the last task as ready to flush. - lineage_cache_->AddUncommittedLineage(tasks.back().GetTaskSpecification().TaskId(), - uncommitted_lineage); - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), tasks.size()); - ASSERT_TRUE(lineage_cache_->CommitTask(tasks.back())); - num_tasks_flushed++; - ASSERT_EQ(mock_gcs_->MockTasks().TaskTable().size(), num_tasks_flushed); - // Flush acknowledgements. The lineage cache should receive the commit for - // the flushed task, but its lineage should not be evicted yet. - mock_gcs_->MockTasks().Flush(); - ASSERT_EQ(lineage_cache_ - ->GetUncommittedLineage(tasks.back().GetTaskSpecification().TaskId(), - ClientID::Nil()) - .GetEntries() - .size(), - tasks.size()); - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), tasks.size()); - - // Simulate executing the task on a remote node and adding it to the GCS. - auto task_id = tasks.at(1).GetTaskSpecification().TaskId(); - auto task_data = std::make_shared(); - task_data->mutable_task()->mutable_task_spec()->set_task_id(task_id.Binary()); - RAY_CHECK_OK(mock_gcs_->MockTasks().RemoteAdd(task_data)); - mock_gcs_->MockTasks().Flush(); - ASSERT_EQ(lineage_cache_ - ->GetUncommittedLineage(tasks.back().GetTaskSpecification().TaskId(), - ClientID::Nil()) - .GetEntries() - .size(), - tasks.size()); - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), tasks.size()); - - // Simulate executing the task on a remote node and adding it to the GCS. - task_id = tasks.at(0).GetTaskSpecification().TaskId(); - auto task_data_2 = std::make_shared(); - task_data_2->mutable_task()->mutable_task_spec()->set_task_id(task_id.Binary()); - RAY_CHECK_OK(mock_gcs_->MockTasks().RemoteAdd(task_data_2)); - mock_gcs_->MockTasks().Flush(); - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), 0); - ASSERT_EQ(lineage_cache_->GetLineage().GetChildrenSize(), 0); -} - -TEST_F(LineageCacheTest, TestEvictManyParents) { - // Create some independent tasks. - std::vector parent_tasks; - std::vector arguments; - for (int i = 0; i < 10; i++) { - auto task = ExampleTask({}, 1); - parent_tasks.push_back(task); - arguments.push_back(task.GetTaskSpecification().ReturnId(0)); - auto lineage = CreateSingletonLineage(task); - lineage_cache_->AddUncommittedLineage(task.GetTaskSpecification().TaskId(), lineage); - } - // Create a child task that is dependent on all of the previous tasks. - auto child_task = ExampleTask(arguments, 1); - auto lineage = CreateSingletonLineage(child_task); - lineage_cache_->AddUncommittedLineage(child_task.GetTaskSpecification().TaskId(), - lineage); - - // Flush the child task. Make sure that it remains in the cache, since none - // of its parents have been committed yet, and that the uncommitted lineage - // still includes all of the parent tasks. - size_t total_tasks = parent_tasks.size() + 1; - lineage_cache_->CommitTask(child_task); - mock_gcs_->MockTasks().Flush(); - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), total_tasks); - ASSERT_EQ(lineage_cache_ - ->GetUncommittedLineage(child_task.GetTaskSpecification().TaskId(), - ClientID::Nil()) - .GetEntries() - .size(), - total_tasks); - - // Flush each parent task and check for eviction safety. - for (const auto &parent_task : parent_tasks) { - lineage_cache_->CommitTask(parent_task); - mock_gcs_->MockTasks().Flush(); - total_tasks--; - if (total_tasks > 1) { - // Each task should be evicted as soon as its commit is acknowledged, - // since the parent tasks have no dependencies. - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), total_tasks); - ASSERT_EQ(lineage_cache_ - ->GetUncommittedLineage(child_task.GetTaskSpecification().TaskId(), - ClientID::Nil()) - .GetEntries() - .size(), - total_tasks); - } else { - // After the last task has been committed, then the child task should - // also be evicted. The lineage cache should now be empty. - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), 0); - } - } - ASSERT_EQ(lineage_cache_->GetLineage().GetChildrenSize(), 0); -} - -TEST_F(LineageCacheTest, TestEviction) { - // Insert a chain of dependent tasks. - uint64_t lineage_size = max_lineage_size_ + 1; - size_t num_tasks_flushed = 0; - std::vector tasks; - InsertTaskChain(*lineage_cache_, tasks, lineage_size, std::vector(), 1); - - // Check that the last task in the chain still has all tasks in its - // uncommitted lineage. - const auto last_task_id = tasks.back().GetTaskSpecification().TaskId(); - auto uncommitted_lineage = - lineage_cache_->GetUncommittedLineage(last_task_id, ClientID::Nil()); - ASSERT_EQ(uncommitted_lineage.GetEntries().size(), lineage_size); - - // Simulate executing the first task on a remote node and adding it to the - // GCS. - auto it = tasks.begin(); - auto task_id = it->GetTaskSpecification().TaskId(); - auto task_data = std::make_shared(); - task_data->mutable_task()->mutable_task_spec()->set_task_id(task_id.Binary()); - RAY_CHECK_OK(mock_gcs_->MockTasks().RemoteAdd(task_data)); - it++; - // Check that the remote task is flushed. - num_tasks_flushed++; - mock_gcs_->MockTasks().Flush(); - ASSERT_EQ(mock_gcs_->MockTasks().TaskTable().size(), num_tasks_flushed); - // Check that the last task in the chain still has all tasks in its - // uncommitted lineage. - ASSERT_EQ(uncommitted_lineage.GetEntries().size(), lineage_size); - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), - lineage_size - num_tasks_flushed); - - // Simulate executing all the rest of the tasks except the last one on a - // remote node and adding them to the GCS. - tasks.pop_back(); - for (; it != tasks.end(); it++) { - auto task_id = it->GetTaskSpecification().TaskId(); - auto task_data = std::make_shared(); - task_data->mutable_task()->mutable_task_spec()->set_task_id(task_id.Binary()); - RAY_CHECK_OK(mock_gcs_->MockTasks().RemoteAdd(task_data)); - // Check that the remote task is flushed. - num_tasks_flushed++; - mock_gcs_->MockTasks().Flush(); - ASSERT_EQ(mock_gcs_->MockTasks().TaskTable().size(), num_tasks_flushed); - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), - lineage_size - num_tasks_flushed); - } - // All tasks have now been flushed. Check that enough lineage has been - // evicted that the uncommitted lineage is now less than the maximum size. - uncommitted_lineage = - lineage_cache_->GetUncommittedLineage(last_task_id, ClientID::Nil()); - ASSERT_TRUE(uncommitted_lineage.GetEntries().size() < max_lineage_size_); - // The remaining task should have no uncommitted lineage. - ASSERT_EQ(uncommitted_lineage.GetEntries().size(), 1); - ASSERT_EQ(lineage_cache_->GetLineage().GetChildrenSize(), 1); -} - -TEST_F(LineageCacheTest, TestOutOfOrderEviction) { - // Insert a chain of dependent tasks that is more than twice as long as the - // maximum lineage size. This will ensure that we request notifications for - // at most 2 remote tasks. - uint64_t lineage_size = (2 * max_lineage_size_) + 2; - size_t num_tasks_flushed = 0; - std::vector tasks; - InsertTaskChain(*lineage_cache_, tasks, lineage_size, std::vector(), 1); - - // Check that the last task in the chain still has all tasks in its - // uncommitted lineage. - const auto last_task_id = tasks.back().GetTaskSpecification().TaskId(); - auto uncommitted_lineage = - lineage_cache_->GetUncommittedLineage(last_task_id, ClientID::Nil()); - ASSERT_EQ(uncommitted_lineage.GetEntries().size(), lineage_size); - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), lineage_size); - - // Simulate executing the rest of the tasks on a remote node and receiving - // the notifications from the GCS in reverse order of execution. - auto last_task = tasks.front(); - tasks.erase(tasks.begin()); - for (auto it = tasks.rbegin(); it != tasks.rend(); it++) { - auto task_id = it->GetTaskSpecification().TaskId(); - auto task_data = std::make_shared(); - task_data->mutable_task()->mutable_task_spec()->set_task_id(task_id.Binary()); - RAY_CHECK_OK(mock_gcs_->MockTasks().RemoteAdd(task_data)); - // Check that the remote task is flushed. - num_tasks_flushed++; - mock_gcs_->MockTasks().Flush(); - ASSERT_EQ(mock_gcs_->MockTasks().TaskTable().size(), num_tasks_flushed); - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), lineage_size); - } - // Flush the last task. The lineage should not get evicted until this task's - // commit is received. - auto task_id = last_task.GetTaskSpecification().TaskId(); - auto task_data = std::make_shared(); - task_data->mutable_task()->mutable_task_spec()->set_task_id(task_id.Binary()); - RAY_CHECK_OK(mock_gcs_->MockTasks().RemoteAdd(task_data)); - num_tasks_flushed++; - mock_gcs_->MockTasks().Flush(); - ASSERT_EQ(mock_gcs_->MockTasks().TaskTable().size(), num_tasks_flushed); - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), 0); - ASSERT_EQ(lineage_cache_->GetLineage().GetChildrenSize(), 0); -} - -TEST_F(LineageCacheTest, TestEvictionUncommittedChildren) { - // Insert a chain of dependent tasks. - size_t num_tasks_flushed = 0; - uint64_t lineage_size = max_lineage_size_ + 1; - std::vector tasks; - InsertTaskChain(*lineage_cache_, tasks, lineage_size, std::vector(), 1); - - // Add more tasks to the lineage cache that will remain local. Each of these - // tasks is dependent one of the tasks that was forwarded above. - for (const auto &task : tasks) { - auto return_id = task.GetTaskSpecification().ReturnId(0); - auto dependent_task = ExampleTask({return_id}, 1); - auto lineage = CreateSingletonLineage(dependent_task); - lineage_cache_->AddUncommittedLineage(dependent_task.GetTaskSpecification().TaskId(), - lineage); - ASSERT_TRUE(lineage_cache_->CommitTask(dependent_task)); - // Once the forwarded tasks are evicted from the lineage cache, we expect - // each of these dependent tasks to be flushed, since all of their - // dependencies have been committed. - num_tasks_flushed++; - } - - // Simulate executing the tasks on the remote node in reverse order and - // adding them to the GCS. Lineage at the local node should not get evicted - // until after the final remote task is executed, since a task can only be - // evicted once all of its ancestors have been committed. - for (auto it = tasks.rbegin(); it != tasks.rend(); it++) { - auto task_id = it->GetTaskSpecification().TaskId(); - auto task_data = std::make_shared(); - task_data->mutable_task()->mutable_task_spec()->set_task_id(task_id.Binary()); - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), lineage_size * 2); - RAY_CHECK_OK(mock_gcs_->MockTasks().RemoteAdd(task_data)); - num_tasks_flushed++; - mock_gcs_->MockTasks().Flush(); - ASSERT_EQ(mock_gcs_->MockTasks().TaskTable().size(), num_tasks_flushed); - } - // Check that after the final remote task is executed, all local lineage is - // now evicted. - ASSERT_EQ(lineage_cache_->GetLineage().GetEntries().size(), 0); - ASSERT_EQ(lineage_cache_->GetLineage().GetChildrenSize(), 0); -} - -TEST_F(LineageCacheTest, TestFlushAllUncommittedTasks) { - // Insert a chain of tasks. - std::vector tasks; - auto return_values = - InsertTaskChain(*lineage_cache_, tasks, 3, std::vector(), 1); - std::vector task_ids; - for (const auto &task : tasks) { - task_ids.push_back(task.GetTaskSpecification().TaskId()); - } - // Check that we subscribed to each of the uncommitted tasks. - ASSERT_EQ(mock_gcs_->MockTasks().NumRequestedNotifications(), task_ids.size()); - - // Flush all uncommitted tasks and make sure we add all tasks to - // the task table. - lineage_cache_->FlushAllUncommittedTasks(); - ASSERT_EQ(mock_gcs_->MockTasks().NumTaskAdds(), tasks.size()); - // Flush again and make sure there are no new tasks added to the - // task table. - lineage_cache_->FlushAllUncommittedTasks(); - ASSERT_EQ(mock_gcs_->MockTasks().NumTaskAdds(), tasks.size()); - - // Flush all GCS notifications. - mock_gcs_->MockTasks().Flush(); - // Make sure that we unsubscribed to the uncommitted tasks before - // we flushed them. - ASSERT_EQ(num_notifications_, 0); - - // Flush again and make sure there are no new tasks added to the - // task table. - lineage_cache_->FlushAllUncommittedTasks(); - ASSERT_EQ(mock_gcs_->MockTasks().NumTaskAdds(), tasks.size()); -} - -} // namespace raylet - -} // namespace ray - -int main(int argc, char **argv) { - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); -} diff --git a/src/ray/raylet/node_manager.cc b/src/ray/raylet/node_manager.cc index 179a90570..42e95b8f4 100644 --- a/src/ray/raylet/node_manager.cc +++ b/src/ray/raylet/node_manager.cc @@ -32,26 +32,6 @@ namespace { #define RAY_CHECK_ENUM(x, y) \ static_assert(static_cast(x) == static_cast(y), "protocol mismatch") -/// A helper function to return the expected actor counter for a given actor -/// and actor handle, according to the given actor registry. If a task's -/// counter is less than the returned value, then the task is a duplicate. If -/// the task's counter is equal to the returned value, then the task should be -/// the next to run. -int64_t GetExpectedTaskCounter( - const std::unordered_map - &actor_registry, - const ray::ActorID &actor_id, const ray::TaskID &actor_caller_id) { - auto actor_entry = actor_registry.find(actor_id); - RAY_CHECK(actor_entry != actor_registry.end()); - const auto &frontier = actor_entry->second.GetFrontier(); - int64_t expected_task_counter = 0; - auto frontier_entry = frontier.find(actor_caller_id); - if (frontier_entry != frontier.end()) { - expected_task_counter = frontier_entry->second.task_counter; - } - return expected_task_counter; -}; - struct ActorStats { int live_actors = 0; int dead_actors = 0; @@ -181,7 +161,6 @@ NodeManager::NodeManager(boost::asio::io_service &io_service, object_manager, reconstruction_policy_, io_service, self_node_id_, RayConfig::instance().initial_reconstruction_timeout_milliseconds(), gcs_client_), - lineage_cache_(self_node_id_, gcs_client_, config.max_lineage_size), actor_registry_(), node_manager_server_("NodeManager", config.node_manager_port), node_manager_service_(io_service, *this), @@ -700,28 +679,10 @@ void NodeManager::NodeRemoved(const GcsNodeInfo &node_info) { remote_node_manager_clients_.erase(client_entry); } - // For any live actors that were on the dead node, broadcast a notification - // about the actor's death - // TODO(swang): This could be very slow if there are many actors. - for (const auto &actor_entry : actor_registry_) { - if (actor_entry.second.GetNodeManagerId() == node_id && - actor_entry.second.GetState() == ActorTableData::ALIVE) { - RAY_LOG(INFO) << "Actor " << actor_entry.first - << " is disconnected, because its node " << node_id - << " is removed from cluster. It may be restarted."; - HandleDisconnectedActor(actor_entry.first, /*was_local=*/false, - /*intentional_disconnect=*/false); - } - } // Notify the object directory that the client has been removed so that it // can remove it from any cached locations. object_directory_->HandleClientRemoved(node_id); - // Flush all uncommitted tasks from the local lineage cache. This is to - // guarantee that all tasks get flushed eventually, in case one of the tasks - // in our local cache was supposed to be flushed by the node that died. - lineage_cache_.FlushAllUncommittedTasks(); - // Clean up workers that were owned by processes that were on the failed // node. rpc::Address address; @@ -944,24 +905,7 @@ void NodeManager::HandleActorStateTransition(const ActorID &actor_id, if (it == actor_registry_.end()) { it = actor_registry_.emplace(actor_id, actor_registration).first; } else { - if (RayConfig::instance().gcs_actor_service_enabled()) { - it->second = actor_registration; - } else { - // Only process the state transition if it is to a later state than ours. - if (actor_registration.GetState() > it->second.GetState() && - actor_registration.GetNumRestarts() == it->second.GetNumRestarts()) { - // The new state is later than ours if it is about the same lifetime, but - // a greater state. - it->second = actor_registration; - } else if (actor_registration.GetNumRestarts() > it->second.GetNumRestarts()) { - // The new state is also later than ours it is about a later lifetime of - // the actor. - it->second = actor_registration; - } else { - // Our state is already at or past the update, so skip the update. - return; - } - } + it->second = actor_registration; } RAY_LOG(DEBUG) << "Actor notification received: actor_id = " << actor_id << ", node_manager_id = " << actor_registration.GetNodeManagerId() @@ -999,7 +943,7 @@ void NodeManager::HandleActorStateTransition(const ActorID &actor_id, // The task's uncommitted lineage was already added to the local lineage // cache upon the initial submission, so it's okay to resubmit it with an // empty lineage this time. - SubmitTask(method, Lineage()); + SubmitTask(method); } } else if (actor_registration.GetState() == ActorTableData::DEAD) { // When an actor dies, loop over all of the queued tasks for that actor @@ -1011,20 +955,13 @@ void NodeManager::HandleActorStateTransition(const ActorID &actor_id, } } else if (actor_registration.GetState() == ActorTableData::RESTARTING) { RAY_LOG(DEBUG) << "Actor is being restarted: " << actor_id; - if (!RayConfig::instance().gcs_actor_service_enabled()) { - // The actor is dead and needs reconstruction. Attempting to reconstruct its - // creation task. - reconstruction_policy_.ListenAndMaybeReconstruct( - actor_registration.GetActorCreationDependency()); - } - // When an actor fails but can be restarted, resubmit all of the queued // tasks for that actor. This will mark the tasks as waiting for actor // creation. auto tasks_to_remove = local_queues_.GetTaskIdsForActor(actor_id); auto removed_tasks = local_queues_.RemoveTasks(tasks_to_remove); for (auto const &task : removed_tasks) { - SubmitTask(task, Lineage()); + SubmitTask(task); } } } @@ -1294,65 +1231,6 @@ void NodeManager::ProcessAnnounceWorkerPortMessage( } } -void NodeManager::HandleDisconnectedActor(const ActorID &actor_id, bool was_local, - bool intentional_disconnect) { - if (RayConfig::instance().gcs_actor_service_enabled()) { - // If gcs actor management is enabled, the gcs will take over the status change of all - // actors. - return; - } - auto actor_entry = actor_registry_.find(actor_id); - RAY_CHECK(actor_entry != actor_registry_.end()); - auto &actor_registration = actor_entry->second; - auto remainingRestarts = actor_registration.GetRemainingRestarts(); - RAY_LOG(DEBUG) << "The actor with ID " << actor_id << " died " - << (intentional_disconnect ? "intentionally" : "unintentionally") - << ", remaining restarts = " << remainingRestarts; - - // Check if this actor needs to be restarted. - ActorState new_state = - (remainingRestarts == -1 || remainingRestarts > 0) && !intentional_disconnect - ? ActorTableData::RESTARTING - : ActorTableData::DEAD; - if (was_local) { - // Clean up the dummy objects from this actor. - RAY_LOG(DEBUG) << "Removing dummy objects for actor: " << actor_id; - for (auto &dummy_object_pair : actor_entry->second.GetDummyObjects()) { - HandleObjectMissing(dummy_object_pair.first); - } - } - // Update the actor's state. - ActorTableData new_actor_info = actor_entry->second.GetTableData(); - new_actor_info.set_state(new_state); - if (was_local) { - // If the actor was local, immediately update the state in actor registry. - // So if we receive any actor tasks before we receive GCS notification, - // these tasks can be correctly routed to the `MethodsWaitingForActorCreation` - // queue, instead of being assigned to the dead actor. - HandleActorStateTransition(actor_id, ActorRegistration(new_actor_info)); - } - - auto done = [was_local, actor_id](Status status) { - if (was_local && !status.ok()) { - // If the disconnected actor was local, only this node will try to update actor - // state. So the update shouldn't fail. - RAY_LOG(FATAL) << "Failed to update state for actor " << actor_id - << ", status: " << status.ToString(); - } - }; - auto actor_notification = std::make_shared(new_actor_info); - RAY_CHECK_OK(gcs_client_->Actors().AsyncUpdate(actor_id, actor_notification, done)); - - if (was_local && new_state == ActorTableData::RESTARTING) { - RAY_LOG(INFO) << "A local actor (id = " << actor_id - << " ) is dead, reconstructing it."; - const ObjectID &actor_creation_dummy_object_id = - actor_registration.GetActorCreationDependency(); - HandleTaskReconstruction(actor_creation_dummy_object_id.TaskId(), - actor_creation_dummy_object_id); - } -} - void NodeManager::HandleWorkerAvailable(const std::shared_ptr &client) { std::shared_ptr worker = worker_pool_.GetRegisteredWorker(client); HandleWorkerAvailable(worker); @@ -1437,18 +1315,11 @@ void NodeManager::ProcessDisconnectClientMessage( if (is_worker) { const ActorID &actor_id = worker->GetActorId(); - if (!actor_id.IsNil()) { - // If the worker was an actor, update actor state, reconstruct the actor if needed, - // and clean up actor's tasks if the actor is permanently dead. - HandleDisconnectedActor(actor_id, true, intentional_disconnect); - } - const TaskID &task_id = worker->GetAssignedTaskId(); // If the worker was running a task or actor, clean up the task and push an // error to the driver, unless the worker is already dead. if ((!task_id.IsNil() || !actor_id.IsNil()) && !worker->IsDead()) { - // If the worker was an actor, the task was already cleaned up in - // `HandleDisconnectedActor`. + // If the worker was an actor, it'll be cleaned by GCS. if (actor_id.IsNil()) { Task task; if (local_queues_.RemoveTask(task_id, &task)) { @@ -1709,7 +1580,7 @@ void NodeManager::ProcessSubmitTaskMessage(const uint8_t *message_data) { // Submit the task to the raylet. Since the task was submitted // locally, there is no uncommitted lineage. - SubmitTask(Task(task_message), Lineage()); + SubmitTask(Task(task_message)); } void NodeManager::ScheduleAndDispatch() { @@ -1775,13 +1646,11 @@ void NodeManager::HandleRequestWorkerLease(const rpc::RequestWorkerLeaseRequest } auto reply_failure_handler = [this, worker_id]() { - if (RayConfig::instance().gcs_actor_service_enabled()) { - RAY_LOG(WARNING) - << "Failed to reply to GCS server, because it might have restarted. GCS " - "cannot obtain the information of the leased worker, so we need to " - "release the leased worker to avoid leakage."; - leased_workers_.erase(worker_id); - } + RAY_LOG(WARNING) + << "Failed to reply to GCS server, because it might have restarted. GCS " + "cannot obtain the information of the leased worker, so we need to " + "release the leased worker to avoid leakage."; + leased_workers_.erase(worker_id); }; send_reply_callback(Status::OK(), nullptr, reply_failure_handler); RAY_CHECK(leased_workers_.find(worker_id) == leased_workers_.end()) @@ -1804,7 +1673,7 @@ void NodeManager::HandleRequestWorkerLease(const rpc::RequestWorkerLeaseRequest reply->set_canceled(true); send_reply_callback(Status::OK(), nullptr, nullptr); }); - SubmitTask(task, Lineage()); + SubmitTask(task); } void NodeManager::HandleRequestResourceReserve( @@ -1939,24 +1808,6 @@ void NodeManager::HandleCancelWorkerLease(const rpc::CancelWorkerLeaseRequest &r send_reply_callback(Status::OK(), nullptr, nullptr); } -void NodeManager::HandleForwardTask(const rpc::ForwardTaskRequest &request, - rpc::ForwardTaskReply *reply, - rpc::SendReplyCallback send_reply_callback) { - // Get the forwarded task and its uncommitted lineage from the request. - TaskID task_id = TaskID::FromBinary(request.task_id()); - Lineage uncommitted_lineage; - for (int i = 0; i < request.uncommitted_tasks_size(); i++) { - Task task(request.uncommitted_tasks(i)); - RAY_CHECK(uncommitted_lineage.SetEntry(task, GcsStatus::UNCOMMITTED)); - } - const Task &task = uncommitted_lineage.GetEntry(task_id)->TaskData(); - RAY_LOG(DEBUG) << "Received forwarded task " << task.GetTaskSpecification().TaskId() - << " on node " << self_node_id_ - << " spillback=" << task.GetTaskExecutionSpec().NumForwards(); - SubmitTask(task, uncommitted_lineage, /* forwarded = */ true); - send_reply_callback(Status::OK(), nullptr, nullptr); -} - void NodeManager::ProcessSetResourceRequest( const std::shared_ptr &client, const uint8_t *message_data) { // Read the SetResource message @@ -2135,7 +1986,7 @@ void NodeManager::TreatTaskAsFailed(const Task &task, const ErrorType &error_typ // Loop over the return IDs (except the dummy ID) and store a fake object in // the object store. int64_t num_returns = spec.NumReturns(); - if (spec.IsActorCreationTask() || spec.IsActorTask()) { + if (spec.IsActorCreationTask()) { // TODO(rkn): We subtract 1 to avoid the dummy ID. However, this leaks // information about the TaskSpecification implementation. num_returns -= 1; @@ -2193,7 +2044,7 @@ void NodeManager::TreatTaskAsFailedIfLost(const Task &task) { // Loop over the return IDs (except the dummy ID) and check whether a // location for the return ID exists. int64_t num_returns = spec.NumReturns(); - if (spec.IsActorCreationTask() || spec.IsActorTask()) { + if (spec.IsActorCreationTask()) { // TODO(rkn): We subtract 1 to avoid the dummy ID. However, this leaks // information about the TaskSpecification implementation. num_returns -= 1; @@ -2224,15 +2075,16 @@ void NodeManager::TreatTaskAsFailedIfLost(const Task &task) { } } -void NodeManager::SubmitTask(const Task &task, const Lineage &uncommitted_lineage, - bool forwarded) { +void NodeManager::SubmitTask(const Task &task) { stats::TaskCountReceived().Record(1); const TaskSpecification &spec = task.GetTaskSpecification(); + // Actor tasks should be no longer submitted to raylet. + RAY_CHECK(!spec.IsActorTask()); const TaskID &task_id = spec.TaskId(); RAY_LOG(DEBUG) << "Submitting task: " << task.DebugString(); if (local_queues_.HasTask(task_id)) { - if (RayConfig::instance().gcs_actor_service_enabled() && spec.IsActorCreationTask()) { + if (spec.IsActorCreationTask()) { // NOTE(hchen): Normally when raylet receives a duplicated actor creation task // from GCS, raylet should just ignore the task. However, due to the hack that // we save the RPC reply in task's OnDispatch callback, we have to remove the @@ -2250,105 +2102,11 @@ void NodeManager::SubmitTask(const Task &task, const Lineage &uncommitted_lineag return; } } - - if (spec.IsActorTask()) { - // Check whether we know the location of the actor. - const auto actor_entry = actor_registry_.find(spec.ActorId()); - bool seen = actor_entry != actor_registry_.end(); - // If we have already seen this actor and this actor is not being restarted, - // its location is known. - bool location_known = - seen && actor_entry->second.GetState() != ActorTableData::RESTARTING; - if (location_known) { - if (actor_entry->second.GetState() == ActorTableData::DEAD) { - // If this actor is dead, either because the actor process is dead - // or because its residing node is dead, treat this task as failed. - TreatTaskAsFailed(task, ErrorType::ACTOR_DIED); - } else { - // If this actor is alive, check whether this actor is local. - auto node_manager_id = actor_entry->second.GetNodeManagerId(); - if (node_manager_id == self_node_id_) { - // The actor is local. - int64_t expected_task_counter = - GetExpectedTaskCounter(actor_registry_, spec.ActorId(), spec.CallerId()); - if (static_cast(spec.ActorCounter()) < expected_task_counter) { - // A task that has already been executed before has been found. The - // task will be treated as failed if at least one of the task's - // return values have been evicted, to prevent the application from - // hanging. - // TODO(swang): Clean up the task from the lineage cache? If the - // task is not marked as failed, then it may never get marked as - // ready to flush to the GCS. - RAY_LOG(WARNING) << "A task was resubmitted, so we are ignoring it. This " - << "should only happen during reconstruction."; - TreatTaskAsFailedIfLost(task); - } else { - // The task has not yet been executed. Queue the task for local - // execution, bypassing placement. - EnqueuePlaceableTask(task); - } - } else { - // The actor is remote. Forward the task to the node manager that owns - // the actor. - // Attempt to forward the task. If this fails to forward the task, - // the task will be resubmit locally. - ForwardTaskOrResubmit(task, node_manager_id); - } - } - } else { - ObjectID actor_creation_dummy_object; - if (!seen) { - // We do not have a registered location for the object, so either the - // actor has not yet been created or we missed the notification for the - // actor creation because this node joined the cluster after the actor - // was already created. Look up the actor's registered location in case - // we missed the creation notification. - const ActorID &actor_id = spec.ActorId(); - auto lookup_callback = - [this, actor_id](Status status, const boost::optional &data) { - if (data) { - // The actor has been created. We only need the last entry, because - // it represents the latest state of this actor. - HandleActorStateTransition(actor_id, ActorRegistration(*data)); - } - }; - RAY_CHECK_OK(gcs_client_->Actors().AsyncGet(actor_id, lookup_callback)); - actor_creation_dummy_object = spec.ActorCreationDummyObjectId(); - } else { - actor_creation_dummy_object = actor_entry->second.GetActorCreationDependency(); - } - - // Keep the task queued until we discover the actor's location. - // (See design_docs/task_states.rst for the state transition diagram.) - local_queues_.QueueTasks({task}, TaskState::WAITING_FOR_ACTOR_CREATION); - // The actor has not yet been created and may have failed. To make sure - // that the actor is eventually recreated, we maintain the invariant that - // if a task is in the MethodsWaitingForActorCreation queue, then it is - // subscribed to its respective actor creation task and that task only. - // Once the actor has been created and this method removed from the - // waiting queue, the caller must make the corresponding call to - // UnsubscribeGetDependencies. - task_dependency_manager_.SubscribeGetDependencies( - spec.TaskId(), {GetReferenceForActorDummyObject(actor_creation_dummy_object)}); - // Mark the task as pending. It will be canceled once we discover the - // actor's location and either execute the task ourselves or forward it - // to another node. - task_dependency_manager_.TaskPending(task); - } - } else { - // This is a non-actor task. Queue the task for a placement decision or for dispatch - // if the task was forwarded. - if (forwarded) { - // Check for local dependencies and enqueue as waiting or ready for dispatch. - EnqueuePlaceableTask(task); - } else { - // (See design_docs/task_states.rst for the state transition diagram.) - local_queues_.QueueTasks({task}, TaskState::PLACEABLE); - ScheduleTasks(cluster_resource_map_); - // TODO(atumanov): assert that !placeable.isempty() => insufficient available - // resources locally. - } - } + // (See design_docs/task_states.rst for the state transition diagram.) + local_queues_.QueueTasks({task}, TaskState::PLACEABLE); + ScheduleTasks(cluster_resource_map_); + // TODO(atumanov): assert that !placeable.isempty() => insufficient available + // resources locally. } void NodeManager::HandleDirectCallTaskBlocked( @@ -2576,18 +2334,9 @@ void NodeManager::EnqueuePlaceableTask(const Task &task) { void NodeManager::AssignTask(const std::shared_ptr &worker, const Task &task, std::vector> *post_assign_callbacks) { + // TODO(sang): Modify method names. const TaskSpecification &spec = task.GetTaskSpecification(); RAY_CHECK(post_assign_callbacks); - // If this is an actor task, check that the new task has the correct counter. - if (spec.IsActorTask()) { - // An actor task should only be ready to be assigned if it matches the - // expected task counter. - int64_t expected_task_counter = - GetExpectedTaskCounter(actor_registry_, spec.ActorId(), spec.CallerId()); - RAY_CHECK(static_cast(spec.ActorCounter()) == expected_task_counter) - << "Expected actor counter: " << expected_task_counter << ", task " - << spec.TaskId() << " has: " << spec.ActorCounter(); - } RAY_LOG(DEBUG) << "Assigning task " << spec.TaskId() << " to worker with pid " << worker->GetProcess().GetId() << ", worker id: " << worker->WorkerId(); @@ -2607,56 +2356,39 @@ void NodeManager::AssignTask(const std::shared_ptr &worker, } auto task_id = spec.TaskId(); - if (task.OnDispatch() != nullptr) { - if (task.GetTaskSpecification().IsDetachedActor()) { - worker->MarkDetachedActor(); - } - - const auto owner_worker_id = WorkerID::FromBinary(spec.CallerAddress().worker_id()); - const auto owner_node_id = ClientID::FromBinary(spec.CallerAddress().raylet_id()); - RAY_CHECK(!owner_worker_id.IsNil()); - RAY_LOG(DEBUG) << "Worker lease request DISPATCH " << task_id << " to worker " - << worker->WorkerId() << ", owner ID " << owner_worker_id; - - task.OnDispatch()(worker, initial_config_.node_manager_address, worker->Port(), - worker->WorkerId(), - spec.IsActorCreationTask() ? worker->GetLifetimeResourceIds() - : worker->GetTaskResourceIds()); - - // If the owner has died since this task was queued, cancel the task by - // killing the worker (unless this task is for a detached actor). - if (!worker->IsDetachedActor() && (failed_workers_cache_.count(owner_worker_id) > 0 || - failed_nodes_cache_.count(owner_node_id) > 0)) { - // TODO(swang): Skip assigning this task to this worker instead of - // killing the worker? - RAY_LOG(INFO) << "Owner of assigned task " << task.GetTaskSpecification().TaskId() - << " died, killing leased worker " << worker->WorkerId(); - KillWorker(worker); - } - - post_assign_callbacks->push_back([this, worker, task_id]() { - RAY_LOG(DEBUG) << "Finished assigning task " << task_id << " to worker " - << worker->WorkerId(); - - FinishAssignTask(worker, task_id, /*success=*/true); - }); - } else { - ResourceIdSet resource_id_set = - worker->GetTaskResourceIds().Plus(worker->GetLifetimeResourceIds()); - if (worker->AssignTask(task, resource_id_set).ok()) { - RAY_LOG(DEBUG) << "Assigned task " << task_id << " to worker " - << worker->WorkerId(); - post_assign_callbacks->push_back([this, worker, task_id]() { - FinishAssignTask(worker, task_id, /*success=*/true); - }); - } else { - RAY_LOG(ERROR) << "Failed to assign task " << task_id << " to worker " - << worker->WorkerId() << ", disconnecting client"; - post_assign_callbacks->push_back([this, worker, task_id]() { - FinishAssignTask(worker, task_id, /*success=*/false); - }); - } + RAY_CHECK(task.OnDispatch() != nullptr); + if (task.GetTaskSpecification().IsDetachedActor()) { + worker->MarkDetachedActor(); } + + const auto owner_worker_id = WorkerID::FromBinary(spec.CallerAddress().worker_id()); + const auto owner_node_id = ClientID::FromBinary(spec.CallerAddress().raylet_id()); + RAY_CHECK(!owner_worker_id.IsNil()); + RAY_LOG(DEBUG) << "Worker lease request DISPATCH " << task_id << " to worker " + << worker->WorkerId() << ", owner ID " << owner_worker_id; + + task.OnDispatch()(worker, initial_config_.node_manager_address, worker->Port(), + worker->WorkerId(), + spec.IsActorCreationTask() ? worker->GetLifetimeResourceIds() + : worker->GetTaskResourceIds()); + + // If the owner has died since this task was queued, cancel the task by + // killing the worker (unless this task is for a detached actor). + if (!worker->IsDetachedActor() && (failed_workers_cache_.count(owner_worker_id) > 0 || + failed_nodes_cache_.count(owner_node_id) > 0)) { + // TODO(swang): Skip assigning this task to this worker instead of + // killing the worker? + RAY_LOG(INFO) << "Owner of assigned task " << task.GetTaskSpecification().TaskId() + << " died, killing leased worker " << worker->WorkerId(); + KillWorker(worker); + } + + post_assign_callbacks->push_back([this, worker, task_id]() { + RAY_LOG(DEBUG) << "Finished assigning task " << task_id << " to worker " + << worker->WorkerId(); + + FinishAssignTask(worker, task_id, /*success=*/true); + }); } bool NodeManager::FinishAssignedTask(WorkerInterface &worker) { @@ -2686,10 +2418,10 @@ bool NodeManager::FinishAssignedTask(WorkerInterface &worker) { } const auto &spec = task.GetTaskSpecification(); // - if ((spec.IsActorCreationTask() || spec.IsActorTask())) { + if ((spec.IsActorCreationTask())) { // If this was an actor or actor creation task, handle the actor's new // state. - FinishAssignedActorTask(worker, task); + FinishAssignedActorCreationTask(worker, task); } else { // If this was a non-actor task, then cancel any ray.wait calls that were // made during the task execution. @@ -2702,7 +2434,7 @@ bool NodeManager::FinishAssignedTask(WorkerInterface &worker) { if (!RayConfig::instance().enable_multi_tenancy()) { // Unset the worker's assigned job Id if this is not an actor. - if (!spec.IsActorCreationTask() && !spec.IsActorTask()) { + if (!spec.IsActorCreationTask()) { worker.AssignJobId(JobID::Nil()); } } @@ -2770,171 +2502,17 @@ std::shared_ptr NodeManager::CreateActorTableDataFromCreationTas return actor_info_ptr; } -void NodeManager::FinishAssignedActorTask(WorkerInterface &worker, const Task &task) { - RAY_LOG(DEBUG) << "Finishing assigned actor task"; - ActorID actor_id; - TaskID caller_id; +void NodeManager::FinishAssignedActorCreationTask(WorkerInterface &worker, + const Task &task) { + RAY_LOG(DEBUG) << "Finishing assigned actor creation task"; const TaskSpecification task_spec = task.GetTaskSpecification(); - bool resumed_from_checkpoint = false; - if (task_spec.IsActorCreationTask()) { - actor_id = task_spec.ActorCreationId(); - caller_id = TaskID::Nil(); - if (checkpoint_id_to_restore_.count(actor_id) > 0) { - resumed_from_checkpoint = true; - } - } else { - actor_id = task_spec.ActorId(); - caller_id = task_spec.CallerId(); - } + ActorID actor_id = task_spec.ActorCreationId(); - if (task_spec.IsActorCreationTask()) { - // This was an actor creation task. Convert the worker to an actor. - worker.AssignActorId(actor_id); + // This was an actor creation task. Convert the worker to an actor. + worker.AssignActorId(actor_id); - if (task_spec.IsDetachedActor()) { - worker.MarkDetachedActor(); - } - - if (RayConfig::instance().gcs_actor_service_enabled()) { - // Gcs server is responsible for notifying other nodes of the changes of actor - // status, and thus raylet doesn't need to handle this anymore. - // And if `new_scheduler_enabled_` is true, this function `FinishAssignedActorTask` - // will not be called because raylet is not aware of the actual task when receiving - // a worker lease request. - return; - } - - // Lookup the parent actor id. - auto parent_task_id = task_spec.ParentTaskId(); - int port = worker.Port(); - auto worker_id = worker.WorkerId(); - RAY_CHECK_OK( - gcs_client_->Tasks().AsyncGet( - parent_task_id, - /*callback=*/ - [this, task_spec, resumed_from_checkpoint, port, parent_task_id, worker_id]( - Status status, const boost::optional &parent_task_data) { - if (parent_task_data) { - // The task was in the GCS task table. Use the stored task spec to - // get the parent actor id. - Task parent_task(parent_task_data->task()); - ActorID parent_actor_id = ActorID::Nil(); - if (parent_task.GetTaskSpecification().IsActorCreationTask()) { - parent_actor_id = parent_task.GetTaskSpecification().ActorCreationId(); - } else if (parent_task.GetTaskSpecification().IsActorTask()) { - parent_actor_id = parent_task.GetTaskSpecification().ActorId(); - } - FinishAssignedActorCreationTask(parent_actor_id, task_spec, - resumed_from_checkpoint, port, worker_id); - return; - } - // The parent task was not in the GCS task table. It should most likely be - // in the lineage cache. - ActorID parent_actor_id = ActorID::Nil(); - if (lineage_cache_.ContainsTask(parent_task_id)) { - // Use a copy of the cached task spec to get the parent actor id. - Task parent_task = lineage_cache_.GetTaskOrDie(parent_task_id); - if (parent_task.GetTaskSpecification().IsActorCreationTask()) { - parent_actor_id = parent_task.GetTaskSpecification().ActorCreationId(); - } else if (parent_task.GetTaskSpecification().IsActorTask()) { - parent_actor_id = parent_task.GetTaskSpecification().ActorId(); - } - } else { - RAY_LOG(WARNING) - << "Task metadata not found in either GCS or lineage cache. It may " - "have " - "been " - "evicted " - << "by the redis LRU configuration. Consider increasing the memory " - "allocation via " - << "ray.init(redis_max_memory=)."; - } - FinishAssignedActorCreationTask(parent_actor_id, task_spec, - resumed_from_checkpoint, port, worker_id); - })); - } else { - auto actor_entry = actor_registry_.find(actor_id); - RAY_CHECK(actor_entry != actor_registry_.end()); - // Extend the actor's frontier to include the executed task. - const ObjectID object_to_release = - actor_entry->second.ExtendFrontier(caller_id, task_spec.ActorDummyObject()); - if (!object_to_release.IsNil()) { - // If there were no new actor handles created, then no other actor task - // will depend on this execution dependency, so it safe to release. - HandleObjectMissing(object_to_release); - } - // Mark the dummy object as locally available to indicate that the actor's - // state has changed and the next method can run. This is not added to the - // object table, so the update will be invisible to both the local object - // manager and the other nodes. - // NOTE(swang): The dummy objects must be marked as local whenever - // ExtendFrontier is called, and vice versa, so that we can clean up the - // dummy objects properly in case the actor fails and needs to be - // restarted. - HandleObjectLocal(task_spec.ActorDummyObject()); - } -} - -void NodeManager::FinishAssignedActorCreationTask(const ActorID &parent_actor_id, - const TaskSpecification &task_spec, - bool resumed_from_checkpoint, int port, - const WorkerID &worker_id) { - // Notify the other node managers that the actor has been created. - const ActorID actor_id = task_spec.ActorCreationId(); - auto new_actor_info = CreateActorTableDataFromCreationTask(task_spec, port, worker_id); - new_actor_info->set_parent_id(parent_actor_id.Binary()); - auto update_callback = [actor_id](Status status) { - if (!status.ok()) { - // Only one node at a time should succeed at creating or updating the actor. - RAY_LOG(FATAL) << "Failed to update state to ALIVE for actor " << actor_id; - } - }; - - if (resumed_from_checkpoint) { - // This actor was resumed from a checkpoint. In this case, we first look - // up the checkpoint in GCS and use it to restore the actor registration - // and frontier. - const auto checkpoint_id = checkpoint_id_to_restore_[actor_id]; - checkpoint_id_to_restore_.erase(actor_id); - RAY_LOG(DEBUG) << "Looking up checkpoint " << checkpoint_id << " for actor " - << actor_id; - RAY_CHECK_OK(gcs_client_->Actors().AsyncGetCheckpoint( - checkpoint_id, actor_id, - [this, checkpoint_id, actor_id, new_actor_info, update_callback]( - Status status, const boost::optional &checkpoint_data) { - RAY_CHECK(checkpoint_data) << "Couldn't find checkpoint " << checkpoint_id - << " for actor " << actor_id << " in GCS."; - RAY_LOG(INFO) << "Restoring registration for actor " << actor_id - << " from checkpoint " << checkpoint_id; - ActorRegistration actor_registration = - ActorRegistration(*new_actor_info, *checkpoint_data); - // Mark the unreleased dummy objects in the checkpoint frontier as local. - for (const auto &entry : actor_registration.GetDummyObjects()) { - HandleObjectLocal(entry.first); - } - HandleActorStateTransition(actor_id, std::move(actor_registration)); - // The actor was created before. - RAY_CHECK_OK(gcs_client_->Actors().AsyncUpdate(actor_id, new_actor_info, - update_callback)); - })); - } else { - // The actor did not resume from a checkpoint. Immediately notify the - // other node managers that the actor has been created. - HandleActorStateTransition(actor_id, ActorRegistration(*new_actor_info)); - if (actor_registry_.find(actor_id) != actor_registry_.end()) { - // The actor was created before. - RAY_CHECK_OK( - gcs_client_->Actors().AsyncUpdate(actor_id, new_actor_info, update_callback)); - } else { - // The actor was never created before. - RAY_CHECK_OK(gcs_client_->Actors().AsyncRegister(new_actor_info, update_callback)); - } - } - if (!resumed_from_checkpoint) { - // The actor was not resumed from a checkpoint. Store the - // initial dummy object. All future handles to the actor will - // depend on this object. - HandleObjectLocal(task_spec.ActorDummyObject()); + if (task_spec.IsDetachedActor()) { + worker.MarkDetachedActor(); } } @@ -2988,88 +2566,19 @@ void NodeManager::HandleTaskReconstruction(const TaskID &task_id, }); } } else { - // We do not have the owner's address. This is either an actor creation - // task or a randomly generated ObjectID. Try to look up the spec for the - // actor creation task. - // TODO(swang): The task lookup is only needed when the GCS actor service is - // disabled. Once the GCS actor service is enabled by default, we can - // immediately mark the object as failed if there is no ownership - // information. - RAY_LOG(DEBUG) << "Required object " << required_object_id - << " fetch timed out, checking task table"; - RAY_CHECK_OK( - gcs_client_->Tasks().AsyncGet( - task_id, - /*callback=*/ - [this, required_object_id, task_id]( - Status status, const boost::optional &task_data) { - if (task_data) { - // The task was in the GCS task table. Use the stored task spec to - // re-execute the task. - ResubmitTask(Task(task_data->task()), required_object_id); - return; - } - // The task was not in the GCS task table. It must therefore be in the - // lineage cache. - if (lineage_cache_.ContainsTask(task_id)) { - // Use a copy of the cached task spec to re-execute the task. - const Task task = lineage_cache_.GetTaskOrDie(task_id); - ResubmitTask(task, required_object_id); - } else { - // No actor creation task spec was found. This is most likely a - // randomly generated ObjectID whose value is unreachable. Mark the - // object as failed. - RAY_LOG(WARNING) - << "Ray cannot get the value of ObjectIDs that are generated " - "randomly (ObjectID.from_random()) or out-of-band " - "(ObjectID.from_binary(...)) because Ray " - "does not know which task will create them. " - "If this was not how your object ID was generated, please file an " - "issue " - "at https://github.com/ray-project/ray/issues/"; - MarkObjectsAsFailed(ErrorType::OBJECT_UNRECONSTRUCTABLE, - {required_object_id}, JobID::Nil()); - } - })); + RAY_LOG(WARNING) + << "Ray cannot get the value of ObjectIDs that are generated " + "randomly (ObjectID.from_random()) or out-of-band " + "(ObjectID.from_binary(...)) because Ray " + "does not know which task will create them. " + "If this was not how your object ID was generated, please file an " + "issue " + "at https://github.com/ray-project/ray/issues/"; + MarkObjectsAsFailed(ErrorType::OBJECT_UNRECONSTRUCTABLE, {required_object_id}, + JobID::Nil()); } } -void NodeManager::ResubmitTask(const Task &task, const ObjectID &required_object_id) { - RAY_LOG(DEBUG) << "Attempting to resubmit task " - << task.GetTaskSpecification().TaskId(); - - // All failure handling is handled by the owner, except for actor creation - // tasks. - if (!task.GetTaskSpecification().IsActorCreationTask()) { - return; - } - - // When the GCS is disabled, the raylet is responsible for restarting the actor. - if (RayConfig::instance().gcs_actor_service_enabled()) { - return; - } - - // Actors should only be recreated if the first initialization failed or if - // the most recent instance of the actor failed. - const auto &actor_id = task.GetTaskSpecification().ActorCreationId(); - const auto it = actor_registry_.find(actor_id); - if (it != actor_registry_.end() && it->second.GetState() == ActorTableData::ALIVE) { - // If the actor is still alive, then do not resubmit the task. If the - // actor actually is dead and a result is needed, then reconstruction - // for this task will be triggered again. - RAY_LOG(WARNING) << "Actor creation task resubmitted, but the actor is still alive."; - return; - } - - RAY_LOG(INFO) << "Resubmitting actor creation task " - << task.GetTaskSpecification().TaskId() << " on node " << self_node_id_; - // The task may be reconstructed. Submit it with an empty lineage, since any - // uncommitted lineage must already be in the lineage cache. At this point, - // the task should not yet exist in the local scheduling queue. If it does, - // then this is a spurious reconstruction. - SubmitTask(task, Lineage()); -} - void NodeManager::HandleObjectLocal(const ObjectID &object_id) { // Notify the task dependency manager that this object is local. const auto ready_task_ids = task_dependency_manager_.HandleObjectLocal(object_id); @@ -3182,163 +2691,41 @@ void NodeManager::HandleObjectMissing(const ObjectID &object_id) { void NodeManager::ForwardTaskOrResubmit(const Task &task, const ClientID &node_manager_id) { - /// TODO(rkn): Should we check that the node manager is remote and not local? - /// TODO(rkn): Should we check if the remote node manager is known to be dead? // Attempt to forward the task. - ForwardTask( - task, node_manager_id, - [this, node_manager_id](ray::Status error, const Task &task) { - const TaskID task_id = task.GetTaskSpecification().TaskId(); - RAY_LOG(INFO) << "Failed to forward task " << task_id << " to node manager " - << node_manager_id; + // TODO(sang): Modify method names. + ForwardTask(task, node_manager_id, + [this, node_manager_id](ray::Status error, const Task &task) { + const TaskID task_id = task.GetTaskSpecification().TaskId(); + RAY_LOG(INFO) << "Failed to forward task " << task_id + << " to node manager " << node_manager_id; - // Mark the failed task as pending to let other raylets know that we still - // have the task. TaskDependencyManager::TaskPending() is assumed to be - // idempotent. - task_dependency_manager_.TaskPending(task); - - // Actor tasks can only be executed at the actor's location, so they are - // retried after a timeout. All other tasks that fail to be forwarded are - // deemed to be placeable again. - if (task.GetTaskSpecification().IsActorTask()) { - // The task is for an actor on another node. Create a timer to resubmit - // the task in a little bit. TODO(rkn): Really this should be a - // unique_ptr instead of a shared_ptr. However, it's a little harder to - // move unique_ptrs into lambdas. - auto retry_timer = std::make_shared(io_service_); - auto retry_duration = boost::posix_time::milliseconds( - RayConfig::instance() - .node_manager_forward_task_retry_timeout_milliseconds()); - retry_timer->expires_from_now(retry_duration); - retry_timer->async_wait( - [this, task_id, retry_timer](const boost::system::error_code &error) { - // Timer killing will receive the boost::asio::error::operation_aborted, - // we only handle the timeout event. - RAY_CHECK(!error); - RAY_LOG(INFO) << "Resubmitting task " << task_id - << " because ForwardTask failed."; - // Remove the RESUBMITTED task from the SWAP queue. - Task task; - TaskState state; - if (local_queues_.RemoveTask(task_id, &task, &state)) { - RAY_CHECK(state == TaskState::SWAP); - // Submit the task again. - SubmitTask(task, Lineage()); - } + // Mark the failed task as pending to let other raylets know that we still + // have the task. TaskDependencyManager::TaskPending() is assumed to be + // idempotent. + task_dependency_manager_.TaskPending(task); + // The task is not for an actor and may therefore be placed on another + // node immediately. Send it to the scheduling policy to be placed again. + local_queues_.QueueTasks({task}, TaskState::PLACEABLE); + ScheduleTasks(cluster_resource_map_); }); - // Temporarily move the RESUBMITTED task to the SWAP queue while the - // timer is active. - local_queues_.QueueTasks({task}, TaskState::SWAP); - } else { - // The task is not for an actor and may therefore be placed on another - // node immediately. Send it to the scheduling policy to be placed again. - local_queues_.QueueTasks({task}, TaskState::PLACEABLE); - ScheduleTasks(cluster_resource_map_); - } - }); } void NodeManager::ForwardTask( const Task &task, const ClientID &node_id, const std::function &on_error) { - // Override spillback for direct tasks. - if (task.OnSpillback() != nullptr) { - auto node_info = gcs_client_->Nodes().Get(node_id); - RAY_CHECK(node_info) - << "Spilling back to a node manager, but no GCS info found for node " << node_id; - task.OnSpillback()(node_id, node_info->node_manager_address(), - node_info->node_manager_port()); - return; - } - - // Lookup node manager client for this node_id and use it to send the request. - auto client_entry = remote_node_manager_clients_.find(node_id); - if (client_entry == remote_node_manager_clients_.end()) { - // TODO(atumanov): caller must handle failure to ensure tasks are not lost. - RAY_LOG(INFO) << "No node manager client found for GCS client id " << node_id; - on_error(ray::Status::IOError("Node manager client not found"), task); - return; - } - auto &client = client_entry->second; - - const auto &spec = task.GetTaskSpecification(); - auto task_id = spec.TaskId(); - - if (worker_pool_.HasPendingWorkerForTask(spec.GetLanguage(), task_id)) { - // There is a worker being starting for this task, - // so we shouldn't forward this task to another node. - on_error(ray::Status::Invalid("Already has pending worker for this task"), task); - return; - } - - // Get the task's unforwarded, uncommitted lineage. - Lineage uncommitted_lineage = lineage_cache_.GetUncommittedLineage(task_id, node_id); - if (uncommitted_lineage.GetEntries().empty()) { - // There is no uncommitted lineage. This can happen if the lineage was - // already evicted before we forwarded the task. - uncommitted_lineage.SetEntry(task, GcsStatus::NONE); - } - auto entry = uncommitted_lineage.GetEntryMutable(task_id); - Task &lineage_cache_entry_task = entry->TaskDataMutable(); - // Increment forward count for the forwarded task. - lineage_cache_entry_task.IncrementNumForwards(); - RAY_LOG(DEBUG) << "Forwarding task " << task_id << " from " << self_node_id_ << " to " - << node_id << " spillback=" - << lineage_cache_entry_task.GetTaskExecutionSpec().NumForwards(); - - // Prepare the request message. - rpc::ForwardTaskRequest request; - request.set_task_id(task_id.Binary()); - for (auto &task_entry : uncommitted_lineage.GetEntries()) { - auto task = request.add_uncommitted_tasks(); - task->mutable_task_spec()->CopyFrom( - task_entry.second.TaskData().GetTaskSpecification().GetMessage()); - task->mutable_task_execution_spec()->CopyFrom( - task_entry.second.TaskData().GetTaskExecutionSpec().GetMessage()); - } - - client->ForwardTask(request, [this, on_error, task, task_id, node_id]( - Status status, const rpc::ForwardTaskReply &reply) { - if (local_queues_.HasTask(task_id)) { - // It must have been forwarded back to us if it's in the queue again - // so just return here. - return; - } - - if (status.ok()) { - const auto &spec = task.GetTaskSpecification(); - // Mark as forwarded so that the task and its lineage are not - // re-forwarded in the future to the receiving node. - lineage_cache_.MarkTaskAsForwarded(task_id, node_id); - - // Notify the task dependency manager that we are no longer responsible - // for executing this task. - task_dependency_manager_.TaskCanceled(task_id); - // Preemptively push any local arguments to the receiving node. For now, we - // only do this with actor tasks, since actor tasks must be executed by a - // specific process and therefore have affinity to the receiving node. - if (spec.IsActorTask()) { - // Iterate through the object's arguments. NOTE(swang): We do not include - // the execution dependencies here since those cannot be transferred - // between nodes. - for (size_t i = 0; i < spec.NumArgs(); ++i) { - if (spec.ArgByRef(i)) { - ObjectID argument_id = spec.ArgId(i); - // If the argument is local, then push it to the receiving node. - if (task_dependency_manager_.CheckObjectLocal(argument_id)) { - object_manager_.Push(argument_id, node_id); - } - } - } - } - } else { - on_error(status, task); - } - }); + // This method spillbacks lease requests to other nodes. + // TODO(sang): Modify method names. + RAY_CHECK(task.OnSpillback() != nullptr); + auto node_info = gcs_client_->Nodes().Get(node_id); + RAY_CHECK(node_info) + << "Spilling back to a node manager, but no GCS info found for node " << node_id; + task.OnSpillback()(node_id, node_info->node_manager_address(), + node_info->node_manager_port()); } void NodeManager::FinishAssignTask(const std::shared_ptr &worker, const TaskID &task_id, bool success) { + // TODO(sang): Modify method names. RAY_LOG(DEBUG) << "FinishAssignTask: " << task_id; // Remove the ASSIGNED task from the READY queue. Task assigned_task; @@ -3461,7 +2848,6 @@ std::string NodeManager::DebugString() const { result << "\n" << local_queues_.DebugString(); result << "\n" << reconstruction_policy_.DebugString(); result << "\n" << task_dependency_manager_.DebugString(); - result << "\n" << lineage_cache_.DebugString(); { absl::MutexLock guard(&plasma_object_notification_lock_); result << "\nnum async plasma notifications: " @@ -3859,7 +3245,6 @@ void NodeManager::RecordMetrics() { local_queues_.RecordMetrics(); reconstruction_policy_.RecordMetrics(); task_dependency_manager_.RecordMetrics(); - lineage_cache_.RecordMetrics(); auto statistical_data = GetActorStatisticalData(actor_registry_); stats::ActorStats().Record(statistical_data.live_actors, diff --git a/src/ray/raylet/node_manager.h b/src/ray/raylet/node_manager.h index 7f4db082d..fe746e0b9 100644 --- a/src/ray/raylet/node_manager.h +++ b/src/ray/raylet/node_manager.h @@ -27,7 +27,6 @@ #include "ray/common/task/scheduling_resources.h" #include "ray/object_manager/object_manager.h" #include "ray/raylet/actor_registration.h" -#include "ray/raylet/lineage_cache.h" #include "ray/raylet/scheduling/scheduling_ids.h" #include "ray/raylet/scheduling/cluster_resource_scheduler.h" #include "ray/raylet/scheduling/cluster_task_manager.h" @@ -246,12 +245,8 @@ class NodeManager : public rpc::NodeManagerServiceHandler { /// Handle specified task's submission to the local node manager. /// /// \param task The task being submitted. - /// \param uncommitted_lineage The uncommitted lineage of the task. - /// \param forwarded True if the task has been forwarded from a different - /// node manager and false if it was submitted by a local worker. /// \return Void. - void SubmitTask(const Task &task, const Lineage &uncommitted_lineage, - bool forwarded = false); + void SubmitTask(const Task &task); /// Assign a task to a worker. The task is assumed to not be queued in local_queues_. /// /// \param[in] worker The worker to assign the task to. @@ -274,25 +269,11 @@ class NodeManager : public rpc::NodeManagerServiceHandler { /// \param worker The port that the actor is listening on. std::shared_ptr CreateActorTableDataFromCreationTask( const TaskSpecification &task_spec, int port, const WorkerID &worker_id); - /// Handle a worker finishing an assigned actor task or actor creation task. + /// Handle a worker finishing an assigned actor creation task. /// \param worker The worker that finished the task. /// \param task The actor task or actor creation task. /// \return Void. - void FinishAssignedActorTask(WorkerInterface &worker, const Task &task); - /// Helper function for handling worker to finish its assigned actor task - /// or actor creation task. Gets invoked when tasks's parent actor is known. - /// - /// \param parent_actor_id The actor id corresponding to the actor which creates - /// the new actor. - /// \param task_spec Task specification of the actor creation task that created the - /// actor. - /// \param resumed_from_checkpoint If the actor was resumed from a checkpoint. - /// \param port Rpc server port that the actor is listening on. - /// \return Void. - void FinishAssignedActorCreationTask(const ActorID &parent_actor_id, - const TaskSpecification &task_spec, - bool resumed_from_checkpoint, int port, - const WorkerID &worker_id); + void FinishAssignedActorCreationTask(WorkerInterface &worker, const Task &task); /// Make a placement decision for placeable tasks given the resource_map /// provided. This will perform task state transitions and task forwarding. /// @@ -321,13 +302,7 @@ class NodeManager : public rpc::NodeManagerServiceHandler { /// \return Void. void HandleTaskReconstruction(const TaskID &task_id, const ObjectID &required_object_id); - /// Resubmit a task for execution. This is a task that was previously already - /// submitted to a raylet but which must now be re-executed. - /// - /// \param task The task being resubmitted. - /// \param required_object_id The object id that triggered the resubmission. - /// \return Void. - void ResubmitTask(const Task &task, const ObjectID &required_object_id); + /// Attempt to forward a task to a remote different node manager. If this /// fails, the task will be resubmit locally. /// @@ -553,13 +528,6 @@ class NodeManager : public rpc::NodeManagerServiceHandler { /// \param message_data A pointer to the message data. void ProcessNotifyActorResumedFromCheckpoint(const uint8_t *message_data); - /// Update actor frontier when a task finishes. - /// If the task is an actor creation task and the actor was resumed from a checkpoint, - /// restore the frontier from the checkpoint. Otherwise, just extend actor frontier. - /// - /// \param task The task that just finished. - void UpdateActorFrontier(const Task &task); - /// Process client message of SetResourceRequest /// \param client The client that sent the message. /// \param message_data A pointer to the message data. @@ -567,18 +535,6 @@ class NodeManager : public rpc::NodeManagerServiceHandler { void ProcessSetResourceRequest(const std::shared_ptr &client, const uint8_t *message_data); - /// Handle the case where an actor is disconnected, determine whether this - /// actor needs to be restarted and then update actor table. - /// This function needs to be called either when actor process dies or when - /// a node dies. - /// - /// \param actor_id Id of this actor. - /// \param was_local Whether the disconnected was on this local node. - /// \param intentional_disconnect Wether the client was intentionally disconnected. - /// \return Void. - void HandleDisconnectedActor(const ActorID &actor_id, bool was_local, - bool intentional_disconnect); - /// Finish assigning a task to a worker. /// /// \param worker Worker that the task is assigned to. @@ -631,11 +587,6 @@ class NodeManager : public rpc::NodeManagerServiceHandler { rpc::CancelWorkerLeaseReply *reply, rpc::SendReplyCallback send_reply_callback) override; - /// Handle a `ForwardTask` request. - void HandleForwardTask(const rpc::ForwardTaskRequest &request, - rpc::ForwardTaskReply *reply, - rpc::SendReplyCallback send_reply_callback) override; - /// Handle a `PinObjectIDs` request. void HandlePinObjectIDs(const rpc::PinObjectIDsRequest &request, rpc::PinObjectIDsReply *reply, @@ -744,8 +695,6 @@ class NodeManager : public rpc::NodeManagerServiceHandler { ReconstructionPolicy reconstruction_policy_; /// A manager to make waiting tasks's missing object dependencies available. TaskDependencyManager task_dependency_manager_; - /// The lineage cache for the GCS object and task tables. - LineageCache lineage_cache_; /// A mapping from actor ID to registration information about that actor /// (including which node manager owns it). std::unordered_map actor_registry_; diff --git a/src/ray/raylet/worker.cc b/src/ray/raylet/worker.cc index ccd74f036..c37685f1a 100644 --- a/src/ray/raylet/worker.cc +++ b/src/ray/raylet/worker.cc @@ -174,27 +174,6 @@ void Worker::AcquireTaskCpuResources(const ResourceIdSet &cpu_resources) { task_resource_ids_.Release(cpu_resources); } -Status Worker::AssignTask(const Task &task, const ResourceIdSet &resource_id_set) { - RAY_CHECK(port_ > 0); - rpc::AssignTaskRequest request; - request.set_intended_worker_id(worker_id_.Binary()); - request.mutable_task()->mutable_task_spec()->CopyFrom( - task.GetTaskSpecification().GetMessage()); - request.mutable_task()->mutable_task_execution_spec()->CopyFrom( - task.GetTaskExecutionSpec().GetMessage()); - request.set_resource_ids(resource_id_set.Serialize()); - - rpc_client_->AssignTask(request, [](Status status, const rpc::AssignTaskReply &reply) { - if (!status.ok()) { - RAY_LOG(DEBUG) << "Worker failed to finish executing task: " << status.ToString(); - } - // Worker has finished this task. There's nothing to do here - // and assigning new task will be done when raylet receives - // `TaskDone` message. - }); - return Status::OK(); -} - void Worker::DirectActorCallArgWaitComplete(int64_t tag) { RAY_CHECK(port_ > 0); rpc::DirectActorCallArgWaitCompleteRequest request; diff --git a/src/ray/raylet/worker.h b/src/ray/raylet/worker.h index c89ece745..cb845fbaf 100644 --- a/src/ray/raylet/worker.h +++ b/src/ray/raylet/worker.h @@ -79,7 +79,6 @@ class WorkerInterface { virtual ResourceIdSet ReleaseTaskCpuResources() = 0; virtual void AcquireTaskCpuResources(const ResourceIdSet &cpu_resources) = 0; - virtual Status AssignTask(const Task &task, const ResourceIdSet &resource_id_set) = 0; virtual void DirectActorCallArgWaitComplete(int64_t tag) = 0; // Setter, geter, and clear methods for allocated_instances_. @@ -165,7 +164,6 @@ class Worker : public WorkerInterface { ResourceIdSet ReleaseTaskCpuResources(); void AcquireTaskCpuResources(const ResourceIdSet &cpu_resources); - Status AssignTask(const Task &task, const ResourceIdSet &resource_id_set); void DirectActorCallArgWaitComplete(int64_t tag); // Setter, geter, and clear methods for allocated_instances_. diff --git a/src/ray/rpc/node_manager/node_manager_client.h b/src/ray/rpc/node_manager/node_manager_client.h index c908a4d79..e6fe99d50 100644 --- a/src/ray/rpc/node_manager/node_manager_client.h +++ b/src/ray/rpc/node_manager/node_manager_client.h @@ -41,12 +41,6 @@ class NodeManagerClient { new GrpcClient(address, port, client_call_manager)); }; - /// Forward a task and its uncommitted lineage. - /// - /// \param[in] request The request message. - /// \param[in] callback The callback function that handles reply. - VOID_RPC_CLIENT_METHOD(NodeManagerService, ForwardTask, grpc_client_, ) - /// Get current node stats. VOID_RPC_CLIENT_METHOD(NodeManagerService, GetNodeStats, grpc_client_, ) diff --git a/src/ray/rpc/node_manager/node_manager_server.h b/src/ray/rpc/node_manager/node_manager_server.h index b3896de07..66783976e 100644 --- a/src/ray/rpc/node_manager/node_manager_server.h +++ b/src/ray/rpc/node_manager/node_manager_server.h @@ -28,7 +28,6 @@ namespace rpc { RPC_SERVICE_HANDLER(NodeManagerService, ReturnWorker) \ RPC_SERVICE_HANDLER(NodeManagerService, ReleaseUnusedWorkers) \ RPC_SERVICE_HANDLER(NodeManagerService, CancelWorkerLease) \ - RPC_SERVICE_HANDLER(NodeManagerService, ForwardTask) \ RPC_SERVICE_HANDLER(NodeManagerService, PinObjectIDs) \ RPC_SERVICE_HANDLER(NodeManagerService, GetNodeStats) \ RPC_SERVICE_HANDLER(NodeManagerService, GlobalGC) \ @@ -76,10 +75,6 @@ class NodeManagerServiceHandler { rpc::CancelResourceReserveReply *reply, rpc::SendReplyCallback send_reply_callback) = 0; - virtual void HandleForwardTask(const ForwardTaskRequest &request, - ForwardTaskReply *reply, - SendReplyCallback send_reply_callback) = 0; - virtual void HandlePinObjectIDs(const PinObjectIDsRequest &request, PinObjectIDsReply *reply, SendReplyCallback send_reply_callback) = 0; diff --git a/src/ray/rpc/worker/core_worker_client.h b/src/ray/rpc/worker/core_worker_client.h index 75cdcdf5a..8af9329e5 100644 --- a/src/ray/rpc/worker/core_worker_client.h +++ b/src/ray/rpc/worker/core_worker_client.h @@ -104,14 +104,6 @@ class CoreWorkerClientInterface { return empty_addr_; } - /// This is called by the Raylet to assign a task to the worker. - /// - /// \param[in] request The request message. - /// \param[in] callback The callback function that handles reply. - /// \return if the rpc call succeeds - virtual void AssignTask(const AssignTaskRequest &request, - const ClientCallback &callback) {} - /// Push an actor task directly from worker to worker. /// /// \param[in] request The request message. @@ -196,8 +188,6 @@ class CoreWorkerClient : public std::enable_shared_from_this, const rpc::Address &Addr() const override { return addr_; } - VOID_RPC_CLIENT_METHOD(CoreWorkerService, AssignTask, grpc_client_, override) - VOID_RPC_CLIENT_METHOD(CoreWorkerService, DirectActorCallArgWaitComplete, grpc_client_, override) diff --git a/src/ray/rpc/worker/core_worker_server.h b/src/ray/rpc/worker/core_worker_server.h index abc3dfa23..ff95a32c8 100644 --- a/src/ray/rpc/worker/core_worker_server.h +++ b/src/ray/rpc/worker/core_worker_server.h @@ -27,7 +27,6 @@ namespace rpc { /// NOTE: See src/ray/core_worker/core_worker.h on how to add a new grpc handler. #define RAY_CORE_WORKER_RPC_HANDLERS \ - RPC_SERVICE_HANDLER(CoreWorkerService, AssignTask) \ RPC_SERVICE_HANDLER(CoreWorkerService, PushTask) \ RPC_SERVICE_HANDLER(CoreWorkerService, DirectActorCallArgWaitComplete) \ RPC_SERVICE_HANDLER(CoreWorkerService, GetObjectStatus) \ @@ -42,7 +41,6 @@ namespace rpc { RPC_SERVICE_HANDLER(CoreWorkerService, PlasmaObjectReady) #define RAY_CORE_WORKER_DECLARE_RPC_HANDLERS \ - DECLARE_VOID_RPC_SERVICE_HANDLER_METHOD(AssignTask) \ DECLARE_VOID_RPC_SERVICE_HANDLER_METHOD(PushTask) \ DECLARE_VOID_RPC_SERVICE_HANDLER_METHOD(DirectActorCallArgWaitComplete) \ DECLARE_VOID_RPC_SERVICE_HANDLER_METHOD(GetObjectStatus) \ diff --git a/src/ray/stats/metric_defs.h b/src/ray/stats/metric_defs.h index 0d5021827..ccbaf3617 100644 --- a/src/ray/stats/metric_defs.h +++ b/src/ray/stats/metric_defs.h @@ -61,10 +61,6 @@ static Gauge ObjectManagerStats("object_manager_stats", "Stat the metric values of object in raylet", "pcs", {ValueTypeKey}); -static Gauge LineageCacheStats("lineage_cache_stats", - "Stats the metric values of lineage cache.", "pcs", - {ValueTypeKey}); - static Gauge TaskDependencyManagerStats("task_dependency_manager_stats", "Stat the metric values of task dependency.", "pcs", {ValueTypeKey});