Skip to content

Commit e18ce0f

Browse files
committed
[manager] reconcile break-glass guard release
1 parent c4e3277 commit e18ce0f

2 files changed

Lines changed: 33 additions & 10 deletions

File tree

kv_cache_manager/manager/migration_manager.cc

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -568,18 +568,37 @@ ErrorCode MigrationManager::BreakGlassReleaseAsyncCopy(const std::string &operat
568568
std::vector<std::vector<ErrorCode>> results;
569569
const auto ec = meta_searcher.BatchCASLocationStatus(
570570
request_context.get(), {record.block_key}, {{std::move(task)}}, results, true);
571-
if (ec != EC_OK) {
572-
return ec;
573-
}
574-
if (results.size() != 1 || results[0].size() != 1) {
575-
return EC_MISMATCH;
576-
}
577-
if (results[0][0] != EC_OK) {
571+
const auto transition_is_persistent = [&]() {
572+
CacheLocationMapVector location_maps;
573+
const auto get_result =
574+
indexer->GetLocationsFromPersistent(request_context.get(), {record.block_key}, location_maps);
575+
if (get_result.error_codes.size() != 1 || get_result.error_codes[0] != EC_OK || location_maps.size() != 1) {
576+
return false;
577+
}
578+
const auto location_iter = location_maps[0].find(record.target_location_id);
579+
return location_iter != location_maps[0].end() && location_iter->second &&
580+
location_iter->second->status() == CLS_DELETING &&
581+
!location_iter->second->has_migration_copy_guard();
582+
};
583+
const bool cas_applied = ec == EC_OK && results.size() == 1 && results[0].size() == 1 && results[0][0] == EC_OK;
584+
if (cas_applied) {
585+
// Sync timeout is an ambiguous durability result, not proof that the
586+
// transition failed. Re-read the persistent backend before returning;
587+
// otherwise a retry would CAS guarded WRITING after the first attempt
588+
// had already durably produced guard-free DELETING and cleanup would
589+
// be stranded forever.
590+
if (!indexer->Sync({record.block_key}) && !transition_is_persistent()) {
591+
return EC_ERROR;
592+
}
593+
} else if (!transition_is_persistent()) {
594+
if (ec != EC_OK) {
595+
return ec;
596+
}
597+
if (results.size() != 1 || results[0].size() != 1) {
598+
return EC_MISMATCH;
599+
}
578600
return results[0][0];
579601
}
580-
if (!indexer->Sync({record.block_key})) {
581-
return EC_ERROR;
582-
}
583602

584603
CopyTaskContext ctx;
585604
ctx.instance_group_name = record.instance_group_name;

kv_cache_manager/manager/test/migration_manager_test.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3737,7 +3737,11 @@ TEST_F(MigrationManagerTest, TestBreakGlassReleaseRequiresUnknownGuardAndClearsQ
37373737
ASSERT_EQ(1u, mgr.ListAsyncCopyQuarantine(group_name).size());
37383738
EXPECT_EQ(EC_BADARGS, mgr.BreakGlassReleaseAsyncCopy(operation_id, "", "ticket-89"));
37393739
EXPECT_EQ(EC_NOENT, mgr.BreakGlassReleaseAsyncCopy("missing-operation", "operator-89", "ticket-89"));
3740+
g_guard_sync_call_count.store(0, std::memory_order_release);
3741+
Stub sync_stub;
3742+
sync_stub.set(ADDR(MetaIndexer, Sync), FailFirstGuardSyncStub);
37403743
EXPECT_EQ(EC_OK, mgr.BreakGlassReleaseAsyncCopy(operation_id, "operator-89", "ticket-89"));
3744+
EXPECT_EQ(1, g_guard_sync_call_count.load(std::memory_order_acquire));
37413745
EXPECT_TRUE(mgr.ListAsyncCopyQuarantine(group_name).empty());
37423746
const auto stats = mgr.GetStats();
37433747
EXPECT_EQ(0u, stats.async_copy_quarantine_operations);

0 commit comments

Comments
 (0)