Skip to content

Commit 27fbaaf

Browse files
committed
fix review feedback on integrity follow-ups
1 parent 3369178 commit 27fbaaf

5 files changed

Lines changed: 73 additions & 0 deletions

File tree

kv_cache_manager/client/src/transfer_client_impl.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,11 @@ ClientErrorCode TransferClientImpl::Init(const std::string &client_config, const
219219
KVCM_LOG_ERROR("sdk_buffer_check_pool init faild, sdk_check_cell_num[%lu], max_check_iov_num[%lu]",
220220
sdk_check_cell_num,
221221
max_check_iov_num_);
222+
client_config_.reset();
223+
sdk_wrapper_.reset();
224+
sdk_buffer_check_pool_.reset();
225+
meta_checksum_enabled_ = false;
226+
is_check_buffer_ = false;
222227
return ER_INIT_CHECK_BUFFER_ERROR;
223228
}
224229
}

kv_cache_manager/config/registry_manager.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,11 @@ ErrorCode RegistryManager::UpdateStorage(RequestContext *request_context,
195195
std::unique_lock<std::shared_mutex> lock(mutex_);
196196
const auto &trace_id = request_context->request_id();
197197
const auto &global_unique_name = storage_config.global_unique_name();
198+
std::string invalid_fields;
199+
if (!storage_config.ValidateRequiredFields(invalid_fields)) {
200+
PREFIX_LOG_S(WARN, "update storage failed: invalid config, fields[%s]", invalid_fields.c_str());
201+
return EC_BADARGS;
202+
}
198203
// 重建期间短暂不可用
199204
auto ec = RemoveStorage(request_context, global_unique_name);
200205
RETURN_IF_EC_NOT_OK_WITH_LOG_S(WARN, ec, "update storage failed: remove storage failed");

kv_cache_manager/config/test/registry_manager_local_backend_test.cc

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -743,4 +743,31 @@ TEST_F(RegistryManagerLocalBackendTest, TestAddStorageRejectsInvalidIntegrity) {
743743
}
744744
}
745745

746+
TEST_F(RegistryManagerLocalBackendTest, TestUpdateStorageRejectsInvalidIntegrityBeforeRemovingExistingStorage) {
747+
std::string local_path = GetPrivateTestRuntimeDataPath() + "_registry_local_backend_update_reject_integrity";
748+
std::string uri = "local://" + local_path + "?cluster_name=test";
749+
ASSERT_TRUE(InitRegistryManager(uri));
750+
751+
std::shared_ptr<NfsStorageSpec> original_spec = GetDefaultNfsStorageSpec();
752+
original_spec->set_root_path(GetPrivateTestRuntimeDataPath() + "/original_nfs_root/");
753+
StorageConfig original_config(DataStorageType::DATA_STORAGE_TYPE_NFS, "storage1", original_spec);
754+
ASSERT_EQ(EC_OK, registry_manager_->AddStorage(request_context_.get(), original_config));
755+
ASSERT_TRUE(registry_manager_->data_storage_manager()->GetDataStorageBackend("storage1"));
756+
757+
DataIntegrityConfig integrity;
758+
integrity.set_enable_inline_header(true);
759+
std::shared_ptr<NfsStorageSpec> invalid_spec = GetDefaultNfsStorageSpec();
760+
invalid_spec->set_root_path(GetPrivateTestRuntimeDataPath() + "/invalid_nfs_root/");
761+
StorageConfig invalid_config(DataStorageType::DATA_STORAGE_TYPE_NFS, "storage1", invalid_spec);
762+
invalid_config.set_integrity(integrity);
763+
764+
EXPECT_EQ(EC_BADARGS, registry_manager_->UpdateStorage(request_context_.get(), invalid_config, true));
765+
766+
auto storage = registry_manager_->data_storage_manager()->GetDataStorageBackend("storage1");
767+
ASSERT_TRUE(storage);
768+
auto spec = std::dynamic_pointer_cast<NfsStorageSpec>(storage->GetStorageConfig().storage_spec());
769+
ASSERT_TRUE(spec);
770+
EXPECT_EQ(GetPrivateTestRuntimeDataPath() + "/original_nfs_root/", spec->root_path());
771+
}
772+
746773
} // namespace kv_cache_manager

kv_cache_manager/manager/cache_manager.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,6 +1413,8 @@ ErrorCode CacheManager::GenWriteLocation(RequestContext *request_context,
14131413

14141414
namespace {
14151415

1416+
std::string LegacyVineyardStorageNameFromInstance(const std::string &instance_id) { return "v6d_" + instance_id; }
1417+
14161418
std::shared_ptr<DataStorageBackend>
14171419
LookupEventReportingBackend(const std::shared_ptr<RegistryManager> &registry_manager, const std::string &instance_id) {
14181420
if (!registry_manager || !registry_manager->data_storage_manager()) {
@@ -1424,6 +1426,11 @@ LookupEventReportingBackend(const std::shared_ptr<RegistryManager> &registry_man
14241426
}
14251427
auto ig = registry_manager->GetInstanceGroupConfig(group_name);
14261428
if (!ig || ig->event_reporting_storage_candidates().empty()) {
1429+
auto legacy_backend = registry_manager->data_storage_manager()->GetDataStorageBackend(
1430+
LegacyVineyardStorageNameFromInstance(instance_id));
1431+
if (legacy_backend && dynamic_cast<EventReportingBackend *>(legacy_backend.get())) {
1432+
return legacy_backend;
1433+
}
14271434
return nullptr;
14281435
}
14291436
auto dsm = registry_manager->data_storage_manager();

kv_cache_manager/manager/test/cache_manager_test.cc

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3264,4 +3264,33 @@ TEST_F(CacheManagerTest, TestGetCacheLocationsByBackendWithBackendSelectors) {
32643264

32653265
dsm->storage_map_.erase("vineyard_default");
32663266
}
3267+
3268+
TEST_F(CacheManagerTest, TestReportEventUsesLegacyVineyardBackendWhenEventCandidatesMissing) {
3269+
auto metrics_registry = cache_manager_->metrics_registry_;
3270+
auto vineyard_backend = std::make_shared<VineyardBackend>(metrics_registry);
3271+
StorageConfig v6d_config;
3272+
v6d_config.set_global_unique_name("v6d_test_instance");
3273+
v6d_config.set_type(DataStorageType::DATA_STORAGE_TYPE_VINEYARD);
3274+
v6d_config.set_storage_spec(std::make_shared<VineyardStorageSpec>());
3275+
ASSERT_EQ(EC_OK, vineyard_backend->Open(v6d_config, "test_trace"));
3276+
3277+
auto dsm = registry_manager_->data_storage_manager_;
3278+
dsm->storage_map_["v6d_test_instance"] = vineyard_backend;
3279+
registry_manager_->instance_group_configs_["default"]->set_event_reporting_storage_candidates({});
3280+
3281+
proto::meta::ReportEventRequest req;
3282+
req.set_instance_id("test_instance");
3283+
req.set_host_ip_port("192.168.10.1:8080");
3284+
req.set_storage_type(proto::meta::ST_VINEYARD);
3285+
auto *reg = req.add_events();
3286+
reg->set_event_type(proto::meta::EVENT_NODE_REGISTER);
3287+
reg->mutable_node_register()->add_mediums("mem");
3288+
3289+
proto::meta::ReportEventResponse resp;
3290+
ASSERT_EQ(EC_OK, cache_manager_->ReportEvent(request_context_.get(), &req, &resp));
3291+
EXPECT_TRUE(vineyard_backend->IsNodeAvailable("test_instance", "192.168.10.1:8080"));
3292+
3293+
dsm->storage_map_.erase("v6d_test_instance");
3294+
}
3295+
32673296
} // namespace kv_cache_manager

0 commit comments

Comments
 (0)