Skip to content

Commit 5dec6f5

Browse files
committed
Fix compile and lint error
Signed-off-by: 黑驰 <[email protected]>
1 parent be1ee6b commit 5dec6f5

File tree

5 files changed

+40
-13
lines changed

5 files changed

+40
-13
lines changed

python/ray/dashboard/modules/virtual_cluster/tests/test_virtual_cluster.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,3 +447,7 @@ def _get_virtual_clusters():
447447
return False
448448

449449
wait_for_condition(_get_virtual_clusters, timeout=10)
450+
451+
452+
if __name__ == "__main__":
453+
pass

src/ray/gcs/gcs_server/gcs_virtual_cluster.cc

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ Status DivisibleCluster::CreateJobCluster(const std::string &job_cluster_id,
357357
ReplicaInstances replica_instances_to_add;
358358
// Lookup undivided alive node instances based on `replica_sets_to_add`.
359359
auto success = LookupUndividedNodeInstances(
360-
replica_sets, replica_instances_to_add, [this](const auto &node_instance) {
360+
replica_sets, replica_instances_to_add, [](const auto &node_instance) {
361361
return !node_instance.is_dead();
362362
});
363363
if (!success) {
@@ -472,9 +472,12 @@ bool DivisibleCluster::ReplenishNodeInstances(
472472
const NodeInstanceReplenishCallback &callback) {
473473
RAY_CHECK(callback != nullptr);
474474
bool any_node_instance_replenished = false;
475-
for (const auto &[job_cluster_id, job_cluster] : job_clusters_) {
475+
for (const auto &[_, job_cluster] : job_clusters_) {
476+
// Explicitly capture the `job_cluster` by value to avoid the compile error.
477+
// e.g. error: 'job_cluster_id' in capture list does not name a variable
478+
const auto &job_cluster_id = job_cluster->GetID();
476479
bool replenished = job_cluster->ReplenishNodeInstances(
477-
[this, &job_cluster_id, &callback, &any_node_instance_replenished](
480+
[this, &job_cluster_id, &any_node_instance_replenished, &callback](
478481
std::shared_ptr<NodeInstance> node_instance) {
479482
const auto &template_id = node_instance->template_id();
480483
if (auto replenished_node_instance = callback(node_instance)) {
@@ -515,7 +518,11 @@ bool DivisibleCluster::ReplenishNodeInstances(
515518

516519
if (replenished) {
517520
// Flush and publish the job cluster data.
518-
async_data_flusher_(job_cluster->ToProto(), nullptr);
521+
auto status = async_data_flusher_(job_cluster->ToProto(), nullptr);
522+
if (!status.ok()) {
523+
RAY_LOG(ERROR) << "Failed to flush and publish the job cluster " << job_cluster_id
524+
<< " data: " << status.message();
525+
}
519526
}
520527
}
521528

@@ -845,7 +852,7 @@ Status PrimaryCluster::DetermineNodeInstanceAdditionsAndRemovals(
845852
// Lookup undivided alive node instances from main cluster based on
846853
// `replica_sets_to_add`.
847854
auto success = LookupUndividedNodeInstances(
848-
replica_sets_to_add, replica_instances_to_add, [this](const auto &node_instance) {
855+
replica_sets_to_add, replica_instances_to_add, [](const auto &node_instance) {
849856
return !node_instance.is_dead();
850857
});
851858
if (!success) {
@@ -1047,7 +1054,11 @@ void PrimaryCluster::ReplenishAllClusterNodeInstances() {
10471054
for (auto &[_, logical_cluster] : logical_clusters_) {
10481055
if (logical_cluster->ReplenishNodeInstances(node_instance_replenish_callback)) {
10491056
// Flush the logical cluster data.
1050-
async_data_flusher_(logical_cluster->ToProto(), nullptr);
1057+
auto status = async_data_flusher_(logical_cluster->ToProto(), nullptr);
1058+
if (!status.ok()) {
1059+
RAY_LOG(ERROR) << "Failed to flush logical cluster " << logical_cluster->GetID()
1060+
<< " data, status: " << status.ToString();
1061+
}
10511062
}
10521063
}
10531064

src/ray/gcs/gcs_server/gcs_virtual_cluster_manager.cc

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,9 @@ void GcsVirtualClusterManager::OnJobFinished(const rpc::JobTableData &job_data)
131131

132132
auto status = divisible_cluster->RemoveJobCluster(
133133
virtual_cluster_id,
134-
[this, job_cluster_id](const Status &status,
135-
std::shared_ptr<rpc::VirtualClusterTableData> data,
136-
const ReplicaSets *replica_sets_to_recommend) {
134+
[job_cluster_id](const Status &status,
135+
std::shared_ptr<rpc::VirtualClusterTableData> data,
136+
const ReplicaSets *replica_sets_to_recommend) {
137137
if (!status.ok() || !data->is_removed()) {
138138
RAY_LOG(WARNING) << "Failed to remove job cluster " << job_cluster_id.Binary()
139139
<< " when handling job finished event. status: "
@@ -450,7 +450,12 @@ void GcsVirtualClusterManager::OnDetachedActorDestroy(
450450
JobCluster *job_cluster = dynamic_cast<JobCluster *>(virtual_cluster.get());
451451
job_cluster->OnDetachedActorDestroy(actor_id);
452452
if (!job_cluster->InUse() && job_cluster->IsFinished()) {
453-
primary_cluster_->RemoveVirtualCluster(virtual_cluster_id, nullptr);
453+
auto status = primary_cluster_->RemoveVirtualCluster(virtual_cluster_id, nullptr);
454+
if (!status.ok()) {
455+
RAY_LOG(WARNING) << "Failed to remove virtual cluster " << virtual_cluster_id
456+
<< " after handling detached actor destroy event. status: "
457+
<< status.message();
458+
}
454459
}
455460
}
456461
}
@@ -483,7 +488,13 @@ void GcsVirtualClusterManager::OnDetachedPlacementGroupDestroy(
483488
JobCluster *job_cluster = dynamic_cast<JobCluster *>(virtual_cluster.get());
484489
job_cluster->OnDetachedPlacementGroupDestroy(placement_group_id);
485490
if (!job_cluster->InUse() && job_cluster->IsFinished()) {
486-
primary_cluster_->RemoveVirtualCluster(virtual_cluster_id, nullptr);
491+
auto status = primary_cluster_->RemoveVirtualCluster(virtual_cluster_id, nullptr);
492+
if (!status.ok()) {
493+
RAY_LOG(WARNING)
494+
<< "Failed to remove virtual cluster " << virtual_cluster_id
495+
<< " after handling detached placement group destroy event. status: "
496+
<< status.message();
497+
}
487498
}
488499
}
489500
}

src/ray/gcs/gcs_server/test/gcs_virtual_cluster_manager_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,7 @@ TEST_F(FailoverTest, FailoverWithDeadNodes) {
10051005
};
10061006
ASSERT_TRUE(
10071007
virtual_cluster_1->ReplenishNodeInstances(node_instance_replenish_callback));
1008-
async_data_flusher_(virtual_cluster_1->ToProto(), nullptr);
1008+
ASSERT_TRUE(async_data_flusher_(virtual_cluster_1->ToProto(), nullptr).ok());
10091009

10101010
// Mock a gcs_init_data.
10111011
instrumented_io_context io_service;

src/ray/protobuf/gcs_service.proto

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -940,7 +940,8 @@ service VirtualClusterInfoGcsService {
940940
rpc CreateOrUpdateVirtualCluster(CreateOrUpdateVirtualClusterRequest)
941941
returns (CreateOrUpdateVirtualClusterReply);
942942
// Remove a virtual cluster.
943-
rpc RemoveVirtualCluster(RemoveVirtualClusterRequest) returns (RemoveVirtualClusterReply);
943+
rpc RemoveVirtualCluster(RemoveVirtualClusterRequest)
944+
returns (RemoveVirtualClusterReply);
944945
// Get virtual clusters.
945946
rpc GetVirtualClusters(GetVirtualClustersRequest) returns (GetVirtualClustersReply);
946947
// Create job cluster.

0 commit comments

Comments
 (0)