[Core] remove unused method GcsResourceManager::UpdateResourceCapacity (

ray-project#22462) In the implementation of `GcsResourceManager::UpdateResourceCapacity`, 'cluster_scheduling_resources_' is modified, but this method is only used in c++ unit test, it is easy to cause confuse when reading the code. Since this method can be completely replaced by `GcsResourceManager::OnNodeAdd`, just remove it. Co-authored-by: 黑驰 <senlin.zsl@antgroup.com>
minaek · Feb 18, 2022 · 3341fae · 3341fae
1 parent df85d31
commit 3341fae
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 35 deletions.
diff --git a/src/ray/gcs/gcs_server/gcs_resource_manager.cc b/src/ray/gcs/gcs_server/gcs_resource_manager.cc
@@ -332,21 +332,6 @@ void GcsResourceManager::SetAvailableResources(const NodeID &node_id,
   }
 }
 
-void GcsResourceManager::UpdateResourceCapacity(
-    const NodeID &node_id,
-    const absl::flat_hash_map<std::string, double> &changed_resources) {
-  auto iter = cluster_scheduling_resources_.find(node_id);
-  if (iter != cluster_scheduling_resources_.end()) {
-    SchedulingResources &scheduling_resources = *iter->second;
-    for (const auto &entry : changed_resources) {
-      scheduling_resources.UpdateResourceCapacity(entry.first, entry.second);
-    }
-  } else {
-    cluster_scheduling_resources_.emplace(
-        node_id, std::make_shared<SchedulingResources>(ResourceSet(changed_resources)));
-  }
-}
-
 void GcsResourceManager::DeleteResources(
     const NodeID &node_id, const std::vector<std::string> &deleted_resources) {
   auto iter = cluster_scheduling_resources_.find(node_id);

diff --git a/src/ray/gcs/gcs_server/gcs_resource_manager.h b/src/ray/gcs/gcs_server/gcs_resource_manager.h
@@ -127,14 +127,6 @@ class GcsResourceManager : public rpc::NodeResourceInfoHandler {
 
   std::string DebugString() const;
 
-  /// Update the total resources and available resources of the specified node.
-  ///
-  /// \param node_id Id of a node.
-  /// \param changed_resources Changed resources of a node.
-  void UpdateResourceCapacity(
-      const NodeID &node_id,
-      const absl::flat_hash_map<std::string, double> &changed_resources);
-
   /// Add resources changed listener.
   void AddResourcesChangedListener(std::function<void()> listener);
 

diff --git a/src/ray/gcs/gcs_server/test/gcs_placement_group_scheduler_test.cc b/src/ray/gcs/gcs_server/test/gcs_placement_group_scheduler_test.cc
@@ -102,13 +102,9 @@ class GcsPlacementGroupSchedulerTest : public ::testing::Test {
   }
 
   void AddNode(const std::shared_ptr<rpc::GcsNodeInfo> &node, int cpu_num = 10) {
+    (*node->mutable_resources_total())["CPU"] = cpu_num;
     gcs_node_manager_->AddNode(node);
     gcs_resource_manager_->OnNodeAdd(*node);
-
-    const auto &node_id = NodeID::FromBinary(node->node_id());
-    absl::flat_hash_map<std::string, double> resource_map;
-    resource_map["CPU"] = cpu_num;
-    gcs_resource_manager_->UpdateResourceCapacity(node_id, resource_map);
   }
 
   void ScheduleFailedWithZeroNodeTest(rpc::PlacementStrategy strategy) {

diff --git a/src/ray/gcs/gcs_server/test/gcs_resource_manager_test.cc b/src/ray/gcs/gcs_server/test/gcs_resource_manager_test.cc
@@ -36,18 +36,22 @@ class GcsResourceManagerTest : public ::testing::Test {
 };
 
 TEST_F(GcsResourceManagerTest, TestBasic) {
-  // Add node resources.
-  auto node_id = NodeID::FromRandom();
   const std::string cpu_resource = "CPU";
   absl::flat_hash_map<std::string, double> resource_map;
   resource_map[cpu_resource] = 10;
-  ResourceSet resource_set(resource_map);
-  gcs_resource_manager_->UpdateResourceCapacity(node_id, resource_map);
+
+  auto node = Mocker::GenNodeInfo();
+  node->mutable_resources_total()->insert(resource_map.begin(), resource_map.end());
+  // Add node resources.
+  gcs_resource_manager_->OnNodeAdd(*node);
 
   // Get and check cluster resources.
   const auto &cluster_resource = gcs_resource_manager_->GetClusterResources();
   ASSERT_EQ(1, cluster_resource.size());
 
+  const auto &node_id = NodeID::FromBinary(node->node_id());
+  ResourceSet resource_set(resource_map);
+
   // Test `AcquireResources`.
   ASSERT_TRUE(gcs_resource_manager_->AcquireResources(node_id, resource_set));
   ASSERT_FALSE(gcs_resource_manager_->AcquireResources(node_id, resource_set));

diff --git a/src/ray/gcs/gcs_server/test/gcs_resource_scheduler_test.cc b/src/ray/gcs/gcs_server/test/gcs_resource_scheduler_test.cc
@@ -40,9 +40,10 @@ class GcsResourceSchedulerTest : public ::testing::Test {
 
   void AddClusterResources(const NodeID &node_id, const std::string &resource_name,
                            double resource_value) {
-    absl::flat_hash_map<std::string, double> resource_map;
-    resource_map[resource_name] = resource_value;
-    gcs_resource_manager_->UpdateResourceCapacity(node_id, resource_map);
+    auto node = Mocker::GenNodeInfo();
+    node->set_node_id(node_id.Binary());
+    (*node->mutable_resources_total())[resource_name] = resource_value;
+    gcs_resource_manager_->OnNodeAdd(*node);
   }
 
   void CheckClusterAvailableResources(const NodeID &node_id,