diff --git a/cloud/blockstore/config/storage.proto b/cloud/blockstore/config/storage.proto index aa25381fb06..36189761043 100644 --- a/cloud/blockstore/config/storage.proto +++ b/cloud/blockstore/config/storage.proto @@ -1103,4 +1103,7 @@ message TStorageServiceConfig optional uint32 ForcedCompactionRangeCountPerRun = 401; optional bool YdbViewerServiceEnabled = 402; + + // Maximum number of pending allocation requests per one disk. + optional uint32 MaxNonReplicatedDiskAllocationRequests = 403; } diff --git a/cloud/blockstore/libs/storage/core/config.cpp b/cloud/blockstore/libs/storage/core/config.cpp index ab3be9af149..d383fbd48dc 100644 --- a/cloud/blockstore/libs/storage/core/config.cpp +++ b/cloud/blockstore/libs/storage/core/config.cpp @@ -459,6 +459,7 @@ TDuration MSeconds(ui32 value) xxx(NonReplicatedDontSuspendDevices, bool, false )\ xxx(AddClientRetryTimeoutIncrement, TDuration, MSeconds(100) )\ xxx(MaxNonReplicatedDiskDeallocationRequests, ui32, 16 )\ + xxx(MaxNonReplicatedDiskAllocationRequests, ui32, 16 )\ xxx(BalancerActionDelayInterval, TDuration, Seconds(3) )\ \ xxx(UseMirrorResync, bool, false )\ diff --git a/cloud/blockstore/libs/storage/core/config.h b/cloud/blockstore/libs/storage/core/config.h index 94f5fe3b970..45a45ac1325 100644 --- a/cloud/blockstore/libs/storage/core/config.h +++ b/cloud/blockstore/libs/storage/core/config.h @@ -518,6 +518,7 @@ class TStorageConfig TDuration GetDiskRegistryBackupPeriod() const; TString GetDiskRegistryBackupDirPath() const; + ui32 GetMaxNonReplicatedDiskAllocationRequests() const; ui32 GetMaxNonReplicatedDiskDeallocationRequests() const; TDuration GetDiskRegistryMetricsCachePeriod() const; diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor.h b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor.h index 951be732909..abddf45eb43 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor.h +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor.h @@ -83,6 +83,7 @@ class TDiskRegistryActor final // Pending requests TDeque PendingRequests; + THashMap> PendingDiskAllocationRequests; THashMap> PendingDiskDeallocationRequests; bool BrokenDisksDestructionInProgress = false; @@ -227,6 +228,21 @@ class TDiskRegistryActor final TDiskRegistryDatabase& db, TDiskRegistryStateSnapshot& args); + void AddPendingAllocation( + const NActors::TActorContext& ctx, + const TString& diskId, + TRequestInfoPtr requestInfoPtr); + + void ReplyToPendingAllocations( + const NActors::TActorContext& ctx, + const TString& diskId, + NProto::TError error = MakeError(S_OK)); + + void ReplyToPendingAllocations( + const NActors::TActorContext& ctx, + TVector& requestInfos, + NProto::TError error); + void AddPendingDeallocation( const NActors::TActorContext& ctx, const TString& diskId, diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_allocate.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_allocate.cpp index 06ad674e2a9..ee348474eb5 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_allocate.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_allocate.cpp @@ -131,6 +131,7 @@ void TDiskRegistryActor::ExecuteAddDisk( &result); args.Devices = std::move(result.Devices); + args.DirtyDevices = std::move(result.DirtyDevices); args.DeviceMigrations = std::move(result.Migrations); args.Replicas = std::move(result.Replicas); args.DeviceReplacementUUIDs = std::move(result.DeviceReplacementIds); @@ -157,6 +158,9 @@ void TDiskRegistryActor::CompleteAddDisk( TStringBuilder devices; OutputDevices(args.Devices, devices); + TStringBuilder dirtyDevices; + OutputDevices(args.DirtyDevices, dirtyDevices); + TStringBuilder replicas; replicas << "["; if (!args.Replicas.empty()) { @@ -180,11 +184,12 @@ void TDiskRegistryActor::CompleteAddDisk( migrations << "]"; LOG_INFO(ctx, TBlockStoreComponents::DISK_REGISTRY, - "[%lu] AddDisk success. DiskId=%s Devices=%s Replicas=%s" - " Migrations=%s", + "[%lu] AddDisk success. DiskId=%s Devices=%s DirtyDevices=%s" + " Replicas=%s Migrations=%s", TabletID(), args.DiskId.Quote().c_str(), devices.c_str(), + dirtyDevices.c_str(), replicas.c_str(), migrations.c_str() ); @@ -240,12 +245,71 @@ void TDiskRegistryActor::CompleteAddDisk( response->Record.SetMuteIOErrors(args.MuteIOErrors); } - NCloud::Reply(ctx, *args.RequestInfo, std::move(response)); + if (HasError(args.Error) || args.DirtyDevices.empty()) { + NCloud::Reply(ctx, *args.RequestInfo, std::move(response)); + } else { + AddPendingAllocation(ctx, args.DiskId, args.RequestInfo); + SecureErase(ctx); + } DestroyBrokenDisks(ctx); NotifyUsers(ctx); } +void TDiskRegistryActor::AddPendingAllocation( + const NActors::TActorContext& ctx, + const TString& diskId, + TRequestInfoPtr requestInfo) +{ + auto& requestInfos = PendingDiskAllocationRequests[diskId]; + + if (requestInfos.size() > Config->GetMaxNonReplicatedDiskAllocationRequests()) { + LOG_WARN(ctx, TBlockStoreComponents::DISK_REGISTRY, + "Too many pending allocation requests (%lu) for disk %s. " + "Reject all requests.", + requestInfos.size(), + diskId.Quote().c_str()); + + ReplyToPendingAllocations(ctx, requestInfos, MakeError(E_REJECTED)); + } + + requestInfos.emplace_back(std::move(requestInfo)); +} + +void TDiskRegistryActor::ReplyToPendingAllocations( + const NActors::TActorContext& ctx, + TVector& requestInfos, + NProto::TError error) +{ + for (auto& requestInfo: requestInfos) { + NCloud::Reply( + ctx, + *requestInfo, + std::make_unique(error)); + } + requestInfos.clear(); +} + +void TDiskRegistryActor::ReplyToPendingAllocations( + const NActors::TActorContext& ctx, + const TString& diskId, + NProto::TError error) +{ + auto it = PendingDiskAllocationRequests.find(diskId); + if (it == PendingDiskAllocationRequests.end()) { + return; + } + + LOG_INFO(ctx, TBlockStoreComponents::DISK_REGISTRY, + "Reply to pending allocation requests. DiskId=%s PendingRequests=%d", + diskId.Quote().c_str(), + static_cast(it->second.size())); + + ReplyToPendingAllocations(ctx, it->second, std::move(error)); + + PendingDiskAllocationRequests.erase(it); +} + //////////////////////////////////////////////////////////////////////////////// void TDiskRegistryActor::HandleDeallocateDisk( @@ -379,7 +443,7 @@ void TDiskRegistryActor::ReplyToPendingDeallocations( } LOG_INFO(ctx, TBlockStoreComponents::DISK_REGISTRY, - "Reply to pending deallocation requests. DiskId=%s PendingRquests=%d", + "Reply to pending deallocation requests. DiskId=%s PendingRequests=%d", diskId.Quote().c_str(), static_cast(it->second.size())); diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_secure_erase.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_secure_erase.cpp index 39bca3994cc..5838d12c7b0 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_secure_erase.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_secure_erase.cpp @@ -279,7 +279,7 @@ void TDiskRegistryActor::ExecuteCleanupDevices( TTxDiskRegistry::TCleanupDevices& args) { TDiskRegistryDatabase db(tx.DB); - args.SyncDeallocatedDisks = + std::tie(args.SyncAllocatedDisks, args.SyncDeallocatedDisks) = State->MarkDevicesAsClean(ctx.Now(), db, args.Devices); } @@ -293,6 +293,10 @@ void TDiskRegistryActor::CompleteCleanupDevices( for (const auto& diskId: args.SyncDeallocatedDisks) { ReplyToPendingDeallocations(ctx, diskId); } + + for (const auto& diskId: args.SyncAllocatedDisks) { + ReplyToPendingAllocations(ctx, diskId); + } } //////////////////////////////////////////////////////////////////////////////// diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_update_agent_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_update_agent_state.cpp index c006d40bd77..d28defbf468 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_update_agent_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_update_agent_state.cpp @@ -197,8 +197,19 @@ void TDiskRegistryActor::CompleteUpdateAgentState( NCloud::Reply(ctx, *args.RequestInfo, std::move(response)); + TVector failedAllocationDisks; if (args.State == NProto::AGENT_STATE_UNAVAILABLE) { ScheduleSwitchAgentDisksToReadOnly(ctx, args.AgentId); + failedAllocationDisks = State->CheckPendingAllocations(args.AgentId); + } + + for (const auto& diskId: failedAllocationDisks) { + ReplyToPendingAllocations( + ctx, + diskId, + MakeError( + E_REJECTED, + "Allocation failed due to disk agent problems")); } } diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_update_device_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_update_device_state.cpp index e5ee97bcac9..804d8864352 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_update_device_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_actor_update_device_state.cpp @@ -89,6 +89,20 @@ void TDiskRegistryActor::CompleteUpdateDeviceState( SecureErase(ctx); StartMigration(ctx); + TDiskId failedAllocationDisk; + if (args.State == NProto::DEVICE_STATE_ERROR) { + failedAllocationDisk = State->CheckPendingAllocation(args.DeviceId); + } + + if (failedAllocationDisk) { + ReplyToPendingAllocations( + ctx, + failedAllocationDisk, + MakeError( + E_REJECTED, + "Allocation failed due to disk agent problems")); + } + auto response = std::make_unique( std::move(args.Error)); NCloud::Reply(ctx, *args.RequestInfo, std::move(response)); diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state.cpp index be4236d81e4..7572f51668a 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state.cpp @@ -1299,9 +1299,16 @@ NProto::TError TDiskRegistryState::ReplaceDeviceWithoutDiskStateUpdate( } if (!manual && !deviceReplacementId.empty()) { - auto cleaningDiskId = + TDiskId cleaningDiskId; + auto [allocating, deallocating] = PendingCleanup.FindDiskId(deviceReplacementId); - if (!cleaningDiskId.empty() && cleaningDiskId != diskId) { + if (allocating && allocating != diskId) { + cleaningDiskId = allocating; + } + if (deallocating && deallocating != diskId) { + cleaningDiskId = deallocating; + } + if (cleaningDiskId) { return MakeError( E_ARGUMENT, TStringBuilder() @@ -2765,6 +2772,37 @@ NProto::TError TDiskRegistryState::AllocateSimpleDisk( params.BlockSize << " bytes"); } + // check that we can secure erase each dirty allocated device + TVector dirtyDevices; + for (const auto& device: allocatedDevices) { + const auto& uuid = device.GetDeviceUUID(); + if (!IsDirtyDevice(uuid)) { + continue; + } + // if we can't secure erase one of allocated disk's dirty devices, we can't allocate the disk + if (!CanSecureErase(uuid)) { + onError(); + + return MakeError(E_BS_DISK_ALLOCATION_FAILED, TStringBuilder() << + "can't secure erase device " << uuid); + } + dirtyDevices.push_back(uuid); + result->DirtyDevices.push_back(device); + } + + if (dirtyDevices) { + NProto::TError cleanupError = PendingCleanup.Insert(params.DiskId, std::move(dirtyDevices), /*allocation=*/true); + // if we can't secure erase one of allocated disk's dirty devices, we can't allocate the disk + if (HasError(cleanupError)) { + onError(); + + ReportDiskRegistryInsertToPendingCleanupFailed( + TStringBuilder() << "An error occurred while allocating disk: " + << FormatError(cleanupError)); + return cleanupError; + } + } + for (const auto& device: allocatedDevices) { disk.Devices.push_back(device.GetDeviceUUID()); } @@ -2987,6 +3025,11 @@ auto TDiskRegistryState::DeallocateSimpleDisk( db.UpdateDirtyDevice(uuid, diskId); } + auto agents = FindDiskDevicesAgents(disk); + for (const auto* agent: agents) { + DeviceList.UpdateDevices(*agent, DevicePoolConfigs); + } + DeleteAllDeviceMigrations(diskId); DeleteDisk(db, diskId); @@ -3851,11 +3894,13 @@ TDiskRegistryState::TDiskId TDiskRegistryState::MarkDeviceAsClean( TDiskRegistryDatabase& db, const TDeviceId& uuid) { - auto ret = MarkDevicesAsClean(now, db, TVector{uuid}); - return ret.empty() ? "" : ret[0]; + auto [alloc, dealloc] = MarkDevicesAsClean(now, db, TVector{uuid}); + Y_UNUSED(alloc); + return dealloc.empty() ? "" : dealloc[0]; } -TVector TDiskRegistryState::MarkDevicesAsClean( +std::pair +TDiskRegistryState::MarkDevicesAsClean( TInstant now, TDiskRegistryDatabase& db, const TVector& uuids) @@ -3869,14 +3914,19 @@ TVector TDiskRegistryState::MarkDevicesAsClean( } } - TVector ret; + TAllocatedDisksList allocatedDisks; + TDellocatedDisksList dellocatedDisks; for (const auto& uuid: TryUpdateDevices(now, db, uuids)) { - if (auto diskId = PendingCleanup.EraseDevice(uuid); !diskId.empty()) { - ret.push_back(std::move(diskId)); + auto [allocatedDisk, deallocatedDisk] = PendingCleanup.EraseDevice(uuid); + if (allocatedDisk) { + allocatedDisks.push_back(std::move(allocatedDisk)); + } + if (deallocatedDisk) { + dellocatedDisks.push_back(std::move(deallocatedDisk)); } } - return ret; + return {std::move(allocatedDisks), std::move(dellocatedDisks)}; } bool TDiskRegistryState::TryUpdateDevice( @@ -5076,8 +5126,9 @@ bool TDiskRegistryState::HasDependentSsdDisks( continue; } + auto [allocating, deallocating] = PendingCleanup.FindDiskId(d.GetDeviceUUID()); if (d.GetPoolKind() == NProto::DEVICE_POOL_KIND_LOCAL && - PendingCleanup.FindDiskId(d.GetDeviceUUID())) + (allocating || deallocating)) { return true; } @@ -5602,6 +5653,28 @@ auto TDiskRegistryState::FindDeviceLocation(const TDeviceId& deviceId) const return const_cast(this)->FindDeviceLocation(deviceId); } +auto TDiskRegistryState::FindDiskDevicesAgents(const TDiskState& disk) const + -> THashSet +{ + THashSet diskAgentIds; + for (const auto& uuid: disk.Devices) { + auto diskAgentId = DeviceList.FindAgentId(uuid); + if (diskAgentId) { + diskAgentIds.insert(diskAgentId); + } + } + + THashSet agents; + for (const auto& agentId: diskAgentIds) { + const auto *agent = AgentList.FindAgent(agentId); + if (agent) { + agents.insert(agent); + } + } + + return agents; +} + auto TDiskRegistryState::FindDeviceLocation(const TDeviceId& deviceId) -> std::pair { @@ -5626,6 +5699,47 @@ auto TDiskRegistryState::FindDeviceLocation(const TDeviceId& deviceId) return {agent, device}; } +TVector +TDiskRegistryState::CheckPendingAllocations(const TAgentId& agentId) +{ + auto* agent = AgentList.FindAgent(agentId); + + if (!agent) { + return {}; + } + + if (agent->GetState() != NProto::AGENT_STATE_UNAVAILABLE) { + return {}; + } + + // If device is unavailable pending allocation is failed + TVector failedAllocationDisks; + for (const auto& d: agent->GetDevices()) { + const auto& deviceId = d.GetDeviceUUID(); + auto diskId = PendingCleanup.CancelPendingAllocation(deviceId); + if (diskId) { + failedAllocationDisks.push_back(std::move(diskId)); + } + } + return failedAllocationDisks; +} + +TDiskRegistryState::TDiskId TDiskRegistryState::CheckPendingAllocation( + const TString& deviceId) +{ + auto [agentPtr, devicePtr] = FindDeviceLocation(deviceId); + if (!agentPtr || !devicePtr) { + return {}; + } + + if (devicePtr->GetState() != NProto::DEVICE_STATE_ERROR) { + return {}; + } + + // If device is in error state pending allocation is failed + return PendingCleanup.CancelPendingAllocation(devicePtr->GetDeviceUUID()); +} + NProto::TError TDiskRegistryState::UpdateDeviceState( TDiskRegistryDatabase& db, const TString& deviceId, @@ -6358,7 +6472,9 @@ NProto::TDiskRegistryStateBackup TDiskRegistryState::BackupState() const transform(GetDirtyDevices(), backup.MutableDirtyDevices(), [this] (auto& x) { NProto::TDiskRegistryStateBackup::TDirtyDevice dd; dd.SetId(x.GetDeviceUUID()); - dd.SetDiskId(PendingCleanup.FindDiskId(x.GetDeviceUUID())); + auto [allocating, deallocating] = PendingCleanup.FindDiskId(x.GetDeviceUUID()); // TODO: need to backup + Y_UNUSED(allocating); + dd.SetDiskId(deallocating); return dd; }); diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state.h b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state.h index 249906cb29f..31a09cc106d 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state.h +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state.h @@ -281,6 +281,18 @@ class TDiskRegistryState using TCheckpoints = THashMap; using TPlacementGroups = THashMap; + using TAllocatingDiskId = TDiskRegistryState::TDiskId; + using TDeallocatingDiskId = TDiskRegistryState::TDiskId; + using TAllocatedDisksList = TVector; + using TDellocatedDisksList = TVector; + + // both values are optional with empty string being empty value + struct TOpt2Disk + { + TAllocatingDiskId AllocatingDiskId; + TDeallocatingDiskId DeallocatingDiskId; + }; + private: TLog Log; @@ -392,6 +404,7 @@ class TDiskRegistryState struct TAllocateDiskResult { TVector Devices; + TVector DirtyDevices; TVector Migrations; TVector> Replicas; TVector DeviceReplacementIds; @@ -503,7 +516,7 @@ class TDiskRegistryState /// Mark selected device as clean and remove it /// from lists of suspended/dirty/pending cleanup devices - /// @return disk id where selected device was allocated + /// @return disk id where selected device was deallocated TDiskId MarkDeviceAsClean( TInstant now, TDiskRegistryDatabase& db, @@ -511,8 +524,9 @@ class TDiskRegistryState /// Mark selected devices as clean and remove them /// from lists of suspended/dirty/pending cleanup devices - /// @return vector of disk ids where selected devices were allocated - TVector MarkDevicesAsClean( + /// @return vector of allocated/deallocated disk ids where selected devices were allocated/deallocated + std::pair + MarkDevicesAsClean( TInstant now, TDiskRegistryDatabase& db, const TVector& uuids); @@ -621,6 +635,9 @@ class TDiskRegistryState TMaybe GetAgentState(const TString& agentId) const; TMaybe GetAgentCmsTs(const TString& agentId) const; + TDiskId CheckPendingAllocation(const TString& deviceId); + TVector CheckPendingAllocations(const TAgentId& agentId); + NProto::TError UpdateDeviceState( TDiskRegistryDatabase& db, const TString& deviceId, @@ -939,6 +956,9 @@ class TDiskRegistryState auto FindDeviceLocation(const TDeviceId& uuid) const -> std::pair; + auto FindDiskDevicesAgents(const TDiskState& disk) const + -> THashSet; + TVector FindDevices( const TString& agentId, const TString& path) const; diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut.cpp index c6871eee036..2e5dec50576 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut.cpp @@ -11795,7 +11795,7 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateTest) const auto& cleanDisks = state.MarkDevicesAsClean( Now(), db, - TVector{"uuid-1", "uuid-2"}); + TVector{"uuid-1", "uuid-2"}).second; UNIT_ASSERT_EQUAL(cleanDisks.size(), 1); UNIT_ASSERT_EQUAL(cleanDisks[0], "disk-1"); }); @@ -11834,7 +11834,7 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateTest) const auto& cleanDisks = state.MarkDevicesAsClean( Now(), db, - TVector{"uuid-1", "uuid-2"}); + TVector{"uuid-1", "uuid-2"}).second; UNIT_ASSERT_EQUAL(cleanDisks.size(), 2); UNIT_ASSERT_EQUAL(cleanDisks[0], "disk-1"); UNIT_ASSERT_EQUAL(cleanDisks[1], "disk-2"); diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_create.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_create.cpp index 52db53ee190..568935722a5 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_create.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_create.cpp @@ -492,6 +492,283 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateCreateTest) } } + Y_UNIT_TEST(ShouldAllocateLocalWithDirtyDevices) + { + TTestExecutor executor; + executor.WriteTx([&](TDiskRegistryDatabase db) { db.InitSchema(); }); + + constexpr ui64 LocalDeviceSize = 99999997952; // ~ 93.13 GiB + + auto makeLocalDevice = [](const auto* name, const auto* uuid) + { + return Device(name, uuid) | + WithPool("local-ssd", NProto::DEVICE_POOL_KIND_LOCAL) | + WithTotalSize(LocalDeviceSize); + }; + + auto agentConfig = AgentConfig( + 1, + { + makeLocalDevice("NVMELOCAL01", "uuid-1"), + makeLocalDevice("NVMELOCAL02", "uuid-2"), + makeLocalDevice("NVMELOCAL03", "uuid-3"), + }); + const TVector agents{ + agentConfig, + }; + + TDiskRegistryState state = + TDiskRegistryStateBuilder() + .WithConfig( + [&] + { + auto config = MakeConfig(0, agents); + + auto* pool = config.AddDevicePoolConfigs(); + pool->SetName("local-ssd"); + pool->SetKind(NProto::DEVICE_POOL_KIND_LOCAL); + pool->SetAllocationUnit(LocalDeviceSize); + + return config; + }()) + .WithAgents(agents) + .WithDirtyDevices( + {TDirtyDevice{"uuid-1", {}}, + TDirtyDevice{"uuid-2", {}}, + TDirtyDevice{"uuid-3", {}}}) + .Build(); + + auto allocate = [&](auto db, ui32 deviceCount) + { + TDiskRegistryState::TAllocateDiskResult result; + + auto error = state.AllocateDisk( + TInstant::Zero(), + db, + TDiskRegistryState::TAllocateDiskParams{ + .DiskId = "local0", + .BlockSize = DefaultLogicalBlockSize, + .BlocksCount = + deviceCount * LocalDeviceSize / DefaultLogicalBlockSize, + .MediaKind = NProto::STORAGE_MEDIA_SSD_LOCAL}, + &result); + + return std::make_pair(std::move(result), error); + }; + + // Register agents. + executor.WriteTx( + [&](TDiskRegistryDatabase db) { + UNIT_ASSERT_SUCCESS( + RegisterAgent(state, db, agentConfig, Now())); + }); + + UNIT_ASSERT_VALUES_EQUAL(1, state.GetConfig().KnownAgentsSize()); + UNIT_ASSERT_VALUES_EQUAL(1, state.GetAgents().size()); + UNIT_ASSERT_VALUES_EQUAL(0, state.GetSuspendedDevices().size()); + UNIT_ASSERT_VALUES_EQUAL(3, state.GetDirtyDevices().size()); + UNIT_ASSERT_VALUES_EQUAL(0, state.GetBrokenDevices().size()); + + // Allocate disk with dirty devices + executor.WriteTx( + [&](TDiskRegistryDatabase db) + { + auto [result, error] = allocate(db, 3U); + UNIT_ASSERT_SUCCESS(error); + UNIT_ASSERT_VALUES_EQUAL(3U, result.Devices.size()); + UNIT_ASSERT_VALUES_EQUAL(3U, result.DirtyDevices.size()); + UNIT_ASSERT_VALUES_EQUAL( + "NVMELOCAL01", + result.Devices[0].GetDeviceName()); + }); + } + + Y_UNIT_TEST(ShouldFavorCleanLocalDevices) + { + TTestExecutor executor; + executor.WriteTx([&](TDiskRegistryDatabase db) { db.InitSchema(); }); + + constexpr ui64 LocalDeviceSize = 99999997952; // ~ 93.13 GiB + + auto makeLocalDevice = [](const auto* name, const auto* uuid) + { + return Device(name, uuid) | + WithPool("local-ssd", NProto::DEVICE_POOL_KIND_LOCAL) | + WithTotalSize(LocalDeviceSize); + }; + + auto agentConfig = AgentConfig( + 1, + { + makeLocalDevice("NVMELOCAL01", "uuid-1"), + makeLocalDevice("NVMELOCAL02", "uuid-2"), + makeLocalDevice("NVMELOCAL03", "uuid-3"), + }); + const TVector agents{ + agentConfig, + }; + + TDiskRegistryState state = + TDiskRegistryStateBuilder() + .WithConfig( + [&] + { + auto config = MakeConfig(0, agents); + + auto* pool = config.AddDevicePoolConfigs(); + pool->SetName("local-ssd"); + pool->SetKind(NProto::DEVICE_POOL_KIND_LOCAL); + pool->SetAllocationUnit(LocalDeviceSize); + + return config; + }()) + .WithAgents(agents) + .WithDirtyDevices( + {TDirtyDevice{"uuid-1", {}}, + TDirtyDevice{"uuid-3", {}}}) + .Build(); + + auto allocate = [&](auto db, ui32 deviceCount) + { + TDiskRegistryState::TAllocateDiskResult result; + + auto error = state.AllocateDisk( + TInstant::Zero(), + db, + TDiskRegistryState::TAllocateDiskParams{ + .DiskId = "local0", + .BlockSize = DefaultLogicalBlockSize, + .BlocksCount = + deviceCount * LocalDeviceSize / DefaultLogicalBlockSize, + .MediaKind = NProto::STORAGE_MEDIA_SSD_LOCAL}, + &result); + + return std::make_pair(std::move(result), error); + }; + + // Register agents. + executor.WriteTx( + [&](TDiskRegistryDatabase db) { + UNIT_ASSERT_SUCCESS( + RegisterAgent(state, db, agentConfig, Now())); + }); + + UNIT_ASSERT_VALUES_EQUAL(1, state.GetConfig().KnownAgentsSize()); + UNIT_ASSERT_VALUES_EQUAL(1, state.GetAgents().size()); + UNIT_ASSERT_VALUES_EQUAL(0, state.GetSuspendedDevices().size()); + UNIT_ASSERT_VALUES_EQUAL(2, state.GetDirtyDevices().size()); + UNIT_ASSERT_VALUES_EQUAL(0, state.GetBrokenDevices().size()); + + // Allocate disk with dirty devices + executor.WriteTx( + [&](TDiskRegistryDatabase db) + { + auto [result, error] = allocate(db, 1U); + UNIT_ASSERT_SUCCESS(error); + UNIT_ASSERT_VALUES_EQUAL(1U, result.Devices.size()); + UNIT_ASSERT_VALUES_EQUAL(0U, result.DirtyDevices.size()); + UNIT_ASSERT_VALUES_EQUAL( + "NVMELOCAL02", + result.Devices[0].GetDeviceName()); + }); + } + + Y_UNIT_TEST(ShouldFavorCleanDevicesAndAllocateDirty) + { + TTestExecutor executor; + executor.WriteTx([&](TDiskRegistryDatabase db) { db.InitSchema(); }); + + constexpr ui64 LocalDeviceSize = 99999997952; // ~ 93.13 GiB + + auto makeLocalDevice = [](const auto* name, const auto* uuid) + { + return Device(name, uuid) | + WithPool("local-ssd", NProto::DEVICE_POOL_KIND_LOCAL) | + WithTotalSize(LocalDeviceSize); + }; + + auto agentConfig = AgentConfig( + 1, + { + makeLocalDevice("NVMELOCAL01", "uuid-1"), + makeLocalDevice("NVMELOCAL02", "uuid-2"), + makeLocalDevice("NVMELOCAL03", "uuid-3"), + }); + const TVector agents{ + agentConfig, + }; + + TDiskRegistryState state = + TDiskRegistryStateBuilder() + .WithConfig( + [&] + { + auto config = MakeConfig(0, agents); + + auto* pool = config.AddDevicePoolConfigs(); + pool->SetName("local-ssd"); + pool->SetKind(NProto::DEVICE_POOL_KIND_LOCAL); + pool->SetAllocationUnit(LocalDeviceSize); + + return config; + }()) + .WithAgents(agents) + .WithDirtyDevices( + {TDirtyDevice{"uuid-1", {}}, + TDirtyDevice{"uuid-3", {}}}) + .Build(); + + auto allocate = [&](auto db, ui32 deviceCount) + { + TDiskRegistryState::TAllocateDiskResult result; + + auto error = state.AllocateDisk( + TInstant::Zero(), + db, + TDiskRegistryState::TAllocateDiskParams{ + .DiskId = "local0", + .BlockSize = DefaultLogicalBlockSize, + .BlocksCount = + deviceCount * LocalDeviceSize / DefaultLogicalBlockSize, + .MediaKind = NProto::STORAGE_MEDIA_SSD_LOCAL}, + &result); + + return std::make_pair(std::move(result), error); + }; + + // Register agents. + executor.WriteTx( + [&](TDiskRegistryDatabase db) { + UNIT_ASSERT_SUCCESS( + RegisterAgent(state, db, agentConfig, Now())); + }); + + UNIT_ASSERT_VALUES_EQUAL(1, state.GetConfig().KnownAgentsSize()); + UNIT_ASSERT_VALUES_EQUAL(1, state.GetAgents().size()); + UNIT_ASSERT_VALUES_EQUAL(0, state.GetSuspendedDevices().size()); + UNIT_ASSERT_VALUES_EQUAL(2, state.GetDirtyDevices().size()); + UNIT_ASSERT_VALUES_EQUAL(0, state.GetBrokenDevices().size()); + + // Allocate disk with dirty devices + executor.WriteTx( + [&](TDiskRegistryDatabase db) + { + auto [result, error] = allocate(db, 2U); + UNIT_ASSERT_SUCCESS(error); + UNIT_ASSERT_VALUES_EQUAL(2U, result.Devices.size()); + UNIT_ASSERT_VALUES_EQUAL(1U, result.DirtyDevices.size()); + UNIT_ASSERT_VALUES_EQUAL( + "NVMELOCAL01", + result.DirtyDevices[0].GetDeviceName()); + UNIT_ASSERT_VALUES_EQUAL( + "NVMELOCAL02", + result.Devices[0].GetDeviceName()); + UNIT_ASSERT_VALUES_EQUAL( + "NVMELOCAL01", + result.Devices[1].GetDeviceName()); + }); + } + Y_UNIT_TEST(ShouldAllocateDiskWithAdjustedBlockCount) { TTestExecutor executor; diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_pending_cleanup.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_pending_cleanup.cpp index eba9d0bd536..5ae7a1590c8 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_pending_cleanup.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_pending_cleanup.cpp @@ -209,6 +209,116 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStatePendingCleanupTest) UNIT_ASSERT_VALUES_EQUAL("vol0", diskId); }); } + + Y_UNIT_TEST(ShouldCreateLocalDiskFromDirtyDevicesAndWaitForCleanup) + { + TTestExecutor executor; + executor.WriteTx([&](TDiskRegistryDatabase db) { db.InitSchema(); }); + + constexpr ui64 LocalDeviceSize = 99999997952; // ~ 93.13 GiB + + auto makeLocalDevice = [](const auto* name, const auto* uuid) + { + return Device(name, uuid) | + WithPool("local-ssd", NProto::DEVICE_POOL_KIND_LOCAL) | + WithTotalSize(LocalDeviceSize); + }; + + const TVector agents{ + AgentConfig( + 1, + { + makeLocalDevice("NVMELOCAL01", "uuid-1"), + makeLocalDevice("NVMELOCAL02", "uuid-2"), + makeLocalDevice("NVMELOCAL03", "uuid-3"), + }), + }; + + TDiskRegistryState state = + TDiskRegistryStateBuilder() + .WithConfig( + [&] + { + auto config = MakeConfig(0, agents); + + auto* pool = config.AddDevicePoolConfigs(); + pool->SetName("local-ssd"); + pool->SetKind(NProto::DEVICE_POOL_KIND_LOCAL); + pool->SetAllocationUnit(LocalDeviceSize); + + return config; + }()) + .WithAgents(agents) + .WithDirtyDevices( + {TDirtyDevice{"uuid-1", {}}, + TDirtyDevice{"uuid-3", {}},}) + .Build(); + + auto allocate = [&](auto db, ui32 deviceCount) + { + TDiskRegistryState::TAllocateDiskResult result; + + auto error = state.AllocateDisk( + TInstant::Zero(), + db, + TDiskRegistryState::TAllocateDiskParams{ + .DiskId = "local0", + .BlockSize = DefaultLogicalBlockSize, + .BlocksCount = + deviceCount * LocalDeviceSize / DefaultLogicalBlockSize, + .MediaKind = NProto::STORAGE_MEDIA_SSD_LOCAL}, + &result); + + return std::make_pair(std::move(result), error); + }; + + // Register agents. + executor.WriteTx( + [&](TDiskRegistryDatabase db) { + UNIT_ASSERT_SUCCESS( + RegisterAgent(state, db, agents[0], Now())); + }); + + UNIT_ASSERT_VALUES_EQUAL(1, state.GetConfig().KnownAgentsSize()); + UNIT_ASSERT_VALUES_EQUAL(1, state.GetAgents().size()); + UNIT_ASSERT_VALUES_EQUAL(0, state.GetSuspendedDevices().size()); + UNIT_ASSERT_VALUES_EQUAL(2, state.GetDirtyDevices().size()); + UNIT_ASSERT_VALUES_EQUAL(0, state.GetBrokenDevices().size()); + + // Create a disk creates pending allocation with the disk + executor.WriteTx( + [&](TDiskRegistryDatabase db) + { + auto [result, error] = allocate(db, 3U); + UNIT_ASSERT_SUCCESS(error); + UNIT_ASSERT_VALUES_EQUAL(3U, result.Devices.size()); + UNIT_ASSERT_VALUES_EQUAL(2U, result.DirtyDevices.size()); + UNIT_ASSERT_VALUES_EQUAL( + "NVMELOCAL02", + result.Devices[0].GetDeviceName()); + UNIT_ASSERT_VALUES_EQUAL( + "NVMELOCAL01", + result.DirtyDevices[0].GetDeviceName()); + UNIT_ASSERT_VALUES_EQUAL( + "NVMELOCAL03", + result.DirtyDevices[1].GetDeviceName()); + }); + + // Marking devices as clean removes the disk from PendingCleanup. + executor.WriteTx( + [&](TDiskRegistryDatabase db) + { + auto [allocatingDisks, deallocatingDisks] = state.MarkDevicesAsClean(Now(), db, {"uuid-1"}); + TVector empty; + UNIT_ASSERT_VALUES_EQUAL(empty, allocatingDisks); + UNIT_ASSERT_VALUES_EQUAL(empty, deallocatingDisks); + + std::tie(allocatingDisks, deallocatingDisks) = state.MarkDevicesAsClean(Now(), db, {"uuid-3"}); + TVector expectedAllocatedDisks {"local0",}; + UNIT_ASSERT_VALUES_EQUAL(empty, deallocatingDisks); + UNIT_ASSERT_VALUES_EQUAL(expectedAllocatedDisks, allocatingDisks); + }); + } } } // namespace NCloud::NBlockStore::NStorage diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_tx.h b/cloud/blockstore/libs/storage/disk_registry/disk_registry_tx.h index 59eea0ce587..9f6ef5553e1 100644 --- a/cloud/blockstore/libs/storage/disk_registry/disk_registry_tx.h +++ b/cloud/blockstore/libs/storage/disk_registry/disk_registry_tx.h @@ -128,6 +128,7 @@ struct TTxDiskRegistry NProto::TError Error; TVector Devices; + TVector DirtyDevices; TVector DeviceMigrations; TVector> Replicas; TVector DeviceReplacementUUIDs; @@ -306,6 +307,7 @@ struct TTxDiskRegistry const TRequestInfoPtr RequestInfo; const TVector Devices; + TVector SyncAllocatedDisks; TVector SyncDeallocatedDisks; explicit TCleanupDevices( @@ -317,6 +319,7 @@ struct TTxDiskRegistry void Clear() { + SyncAllocatedDisks.clear(); SyncDeallocatedDisks.clear(); } }; diff --git a/cloud/blockstore/libs/storage/disk_registry/model/device_list.cpp b/cloud/blockstore/libs/storage/disk_registry/model/device_list.cpp index e02f063f8af..09fd800675e 100644 --- a/cloud/blockstore/libs/storage/disk_registry/model/device_list.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/model/device_list.cpp @@ -14,6 +14,7 @@ namespace { //////////////////////////////////////////////////////////////////////////////// using TSortQueryKey = std::tuple< + bool, NProto::EDevicePoolKind, TString, ui32, @@ -21,9 +22,11 @@ using TSortQueryKey = std::tuple< struct TBySortQueryKey { + std::function IsDirty; auto operator () (const NProto::TDeviceConfig& config) const { return TSortQueryKey { + IsDirty(config.GetDeviceUUID()), config.GetPoolKind(), config.GetPoolName(), config.GetBlockSize(), @@ -165,7 +168,8 @@ void TDeviceList::UpdateDevices( const bool isFree = DevicesAllocationAllowed(device.GetPoolKind(), agent.GetState()) && device.GetState() == NProto::DEVICE_STATE_ONLINE && - !AllocatedDevices.contains(uuid) && !DirtyDevices.contains(uuid) && + !AllocatedDevices.contains(uuid) && + (device.GetPoolKind() == NProto::DEVICE_POOL_KIND_LOCAL || !DirtyDevices.contains(uuid)) && !SuspendedDevices.contains(uuid); const auto* poolConfig = poolConfigs.FindPtr(device.GetPoolName()); @@ -196,7 +200,12 @@ void TDeviceList::UpdateDevices( nodeDevices.TotalSize += deviceSize; } - SortBy(nodeDevices.FreeDevices, TBySortQueryKey()); + std::function isDirty = + [&](const TDeviceId& uuid) + { + return DirtyDevices.contains(uuid); + }; + SortBy(nodeDevices.FreeDevices, TBySortQueryKey{.IsDirty = isDirty}); } void TDeviceList::RemoveDevices(const NProto::TAgentConfig& agent) @@ -539,13 +548,12 @@ TVector TDeviceList::CollectDevices( for (const auto& rack: SelectRacks(query, poolName)) { for (const auto& node: rack.Nodes) { - const auto* nodeDevices = NodeDevices.FindPtr(node.NodeId); + auto* nodeDevices = NodeDevices.FindPtr(node.NodeId); Y_ABORT_UNLESS(nodeDevices); // finding free devices belonging to this node that match our // query - auto [begin, end] = - FindDeviceRange(query, poolName, nodeDevices->FreeDevices); + auto [begin, end] = FindDeviceRange(query, poolName, nodeDevices->FreeDevices); using TDeviceIter = decltype(begin); struct TDeviceInfo @@ -611,7 +619,6 @@ TVector TDeviceList::CollectDevices( if (query.PoolKind == NProto::DEVICE_POOL_KIND_LOCAL) { // here we go again - ranges.clear(); totalSize = query.GetTotalByteCount(); } @@ -670,7 +677,6 @@ TVector TDeviceList::AllocateDevices( }); auto& nodeDevices = NodeDevices[nodeId]; - for (const auto& arange: aranges) { nodeDevices.FreeDevices.erase(arange.first, arange.second); } @@ -716,15 +722,14 @@ void TDeviceList::MarkDeviceAsDirty(const TDeviceId& id) void TDeviceList::RemoveDeviceFromFreeList(const TDeviceId& id) { auto nodeId = FindNodeId(id); - + const auto& predicate = [&](const auto& x) + { + return x.GetDeviceUUID() == id; + }; if (nodeId) { auto& devices = NodeDevices[nodeId].FreeDevices; - auto it = FindIf(devices, [&] (const auto& x) { - return x.GetDeviceUUID() == id; - }); - - if (it != devices.end()) { + if (auto* it = FindIf(devices, predicate); it != devices.end()) { devices.erase(it); } } diff --git a/cloud/blockstore/libs/storage/disk_registry/model/device_list.h b/cloud/blockstore/libs/storage/disk_registry/model/device_list.h index b345f04263b..67f25e19fb5 100644 --- a/cloud/blockstore/libs/storage/disk_registry/model/device_list.h +++ b/cloud/blockstore/libs/storage/disk_registry/model/device_list.h @@ -45,7 +45,7 @@ class TDeviceList { TString Rack; - // sorted by {PoolKind, BlockSize} + // sorted by {IsDirty, PoolKind, BlockSize} TVector FreeDevices; ui64 TotalSize = 0; diff --git a/cloud/blockstore/libs/storage/disk_registry/model/pending_cleanup.cpp b/cloud/blockstore/libs/storage/disk_registry/model/pending_cleanup.cpp index d96bdf1211f..3749c5ca5ae 100644 --- a/cloud/blockstore/libs/storage/disk_registry/model/pending_cleanup.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/model/pending_cleanup.cpp @@ -10,9 +10,10 @@ namespace NCloud::NBlockStore::NStorage { NProto::TError TPendingCleanup::Insert( const TString& diskId, - TVector uuids) + TVector uuids, + bool allocation) { - auto error = ValidateInsertion(diskId, uuids); + auto error = ValidateInsertion(diskId, uuids, allocation); if (HasError(error)) { return error; } @@ -21,21 +22,22 @@ NProto::TError TPendingCleanup::Insert( for (auto& uuid: uuids) { Y_DEBUG_ABORT_UNLESS(!uuid.empty()); - auto [_, success] = DeviceToDisk.emplace(std::move(uuid), diskId); - Y_DEBUG_ABORT_UNLESS(success); + auto& [allocatingDiskId, deallocatingDiskId] = DeviceToDisk[uuid]; + (allocation ? allocatingDiskId : deallocatingDiskId) = diskId; } return {}; } -NProto::TError TPendingCleanup::Insert(const TString& diskId, TString uuid) +NProto::TError TPendingCleanup::Insert(const TString& diskId, TString uuid, bool allocation) { - return Insert(diskId, TVector{std::move(uuid)}); + return Insert(diskId, TVector{std::move(uuid)}, allocation); } [[nodiscard]] NProto::TError TPendingCleanup::ValidateInsertion( const TString& diskId, - const TVector& uuids) const + const TVector& uuids, + bool allocation) const { if (diskId.empty() || uuids.empty()) { return MakeError( @@ -55,7 +57,12 @@ NProto::TError TPendingCleanup::Insert(const TString& diskId, TString uuid) << JoinStrings(uuids, ", ") << "]"); } - const auto* foundDiskId = DeviceToDisk.FindPtr(uuid); + const auto* foundDisks = DeviceToDisk.FindPtr(uuid); + if (!foundDisks) { + continue; + } + const auto& foundDiskId = allocation ? foundDisks->AllocatingDiskId + : foundDisks->DeallocatingDiskId; if (foundDiskId) { return MakeError( E_ARGUMENT, @@ -63,34 +70,41 @@ NProto::TError TPendingCleanup::Insert(const TString& diskId, TString uuid) << "; diskId: " << diskId << " to PendingCleanup. It was already " "inserted with the diskId: " - << *foundDiskId); + << foundDiskId); } } return {}; } -TString TPendingCleanup::EraseDevice(const TString& uuid) +TPendingCleanup::TOpt2Disk TPendingCleanup::EraseDevice(const TString& uuid) { auto it = DeviceToDisk.find(uuid); if (it == DeviceToDisk.end()) { return {}; } - auto diskId = std::move(it->second); + auto [allocatingDiskId, deallocatingDiskId] = std::move(it->second); DeviceToDisk.erase(it); - - Y_DEBUG_ABORT_UNLESS(DiskToDeviceCount.contains(diskId)); - if (--DiskToDeviceCount[diskId] > 0) { - return {}; + TOpt2Disk ret; + + Y_DEBUG_ABORT_UNLESS(allocatingDiskId || deallocatingDiskId); + Y_DEBUG_ABORT_UNLESS(!allocatingDiskId || DiskToDeviceCount.contains(allocatingDiskId)); + Y_DEBUG_ABORT_UNLESS(!deallocatingDiskId || DiskToDeviceCount.contains(deallocatingDiskId)); + if (allocatingDiskId && --DiskToDeviceCount[allocatingDiskId] <= 0) { + DiskToDeviceCount.erase(allocatingDiskId); + ret.AllocatingDiskId = std::move(allocatingDiskId); } - DiskToDeviceCount.erase(diskId); + if (deallocatingDiskId && --DiskToDeviceCount[deallocatingDiskId] <= 0) { + DiskToDeviceCount.erase(deallocatingDiskId); + ret.DeallocatingDiskId = std::move(deallocatingDiskId); + } - return diskId; + return ret; } -TString TPendingCleanup::FindDiskId(const TString& uuid) const +TPendingCleanup::TOpt2Disk TPendingCleanup::FindDiskId(const TString& uuid) const { auto it = DeviceToDisk.find(uuid); if (it == DeviceToDisk.end()) { @@ -106,8 +120,19 @@ bool TPendingCleanup::EraseDisk(const TString& diskId) return false; } + for (auto& [device, opt2Disk] : DeviceToDisk) { + auto& [allocatingDiskId, deallocatingDiskId] = opt2Disk; + if (allocatingDiskId == diskId) { + allocatingDiskId.clear(); + } + if (deallocatingDiskId == diskId) { + deallocatingDiskId.clear(); + } + } + EraseNodesIf(DeviceToDisk, [&] (auto& x) { - return x.second == diskId; + auto& [allocating, deallocating] = x.second; + return !allocating && !deallocating; }); return true; @@ -123,4 +148,13 @@ bool TPendingCleanup::Contains(const TString& diskId) const return DiskToDeviceCount.contains(diskId); } +TPendingCleanup::TAllocatingDiskId TPendingCleanup::CancelPendingAllocation(const TString& uuid) { + const auto [allocating, deallocating] = FindDiskId(uuid); + Y_UNUSED(deallocating); + if (allocating) { + EraseDisk(allocating); + } + return allocating; +} + } // namespace NCloud::NBlockStore::NStorage diff --git a/cloud/blockstore/libs/storage/disk_registry/model/pending_cleanup.h b/cloud/blockstore/libs/storage/disk_registry/model/pending_cleanup.h index 841184ce555..60bfc9786bb 100644 --- a/cloud/blockstore/libs/storage/disk_registry/model/pending_cleanup.h +++ b/cloud/blockstore/libs/storage/disk_registry/model/pending_cleanup.h @@ -15,27 +15,49 @@ namespace NCloud::NBlockStore::NStorage { class TPendingCleanup { +public: + using TAllocatingDiskId = TString; + using TDeallocatingDiskId = TString; + + // both values are optional with empty string being empty value + struct TOpt2Disk + { + TAllocatingDiskId AllocatingDiskId; + TDeallocatingDiskId DeallocatingDiskId; + }; + private: THashMap DiskToDeviceCount; - THashMap DeviceToDisk; + THashMap DeviceToDisk; public: [[nodiscard]] NProto::TError Insert( const TString& diskId, - TVector uuids); - [[nodiscard]] NProto::TError Insert(const TString& diskId, TString uuid); - - TString EraseDevice(const TString& uuid); + TVector uuids, + bool allocation = false); + [[nodiscard]] NProto::TError + Insert(const TString& diskId, TString uuid, bool allocation = false); + + /// Removes the device from deallocating disk (for pending deallocation + /// disks) + /// @return diskId of allocated/deallocated disk if the device with the + /// given UUID was the last to complete its allocation/deallocation + TOpt2Disk EraseDevice(const TString& uuid); bool EraseDisk(const TString& diskId); - [[nodiscard]] TString FindDiskId(const TString& uuid) const; + [[nodiscard]] TOpt2Disk FindDiskId(const TString& uuid) const; [[nodiscard]] bool IsEmpty() const; [[nodiscard]] bool Contains(const TString& diskId) const; + /// If device belongs to pending allocation disk, forget this disk + /// @return diskId of allocating disk if existed, empty string otherwise + [[nodiscard]] TAllocatingDiskId CancelPendingAllocation(const TString& uuid); + private: [[nodiscard]] NProto::TError ValidateInsertion( const TString& diskId, - const TVector& uuids) const; + const TVector& uuids, + bool allocation) const; }; } // namespace NCloud::NBlockStore::NStorage diff --git a/cloud/blockstore/libs/storage/disk_registry/model/pending_cleanup_ut.cpp b/cloud/blockstore/libs/storage/disk_registry/model/pending_cleanup_ut.cpp index 9b46b9c050c..0a5d937eda9 100644 --- a/cloud/blockstore/libs/storage/disk_registry/model/pending_cleanup_ut.cpp +++ b/cloud/blockstore/libs/storage/disk_registry/model/pending_cleanup_ut.cpp @@ -2,8 +2,23 @@ #include +using TOpt2Disk = NCloud::NBlockStore::NStorage::TPendingCleanup::TOpt2Disk; + +IOutputStream& operator<<(IOutputStream& os, const TOpt2Disk& pair) +{ + os << "{ '" << pair.AllocatingDiskId << "', '" << pair.DeallocatingDiskId + << "'}"; + return os; +} + namespace NCloud::NBlockStore::NStorage { +bool operator==(const TOpt2Disk& lhs, const TOpt2Disk& rhs) +{ + return lhs.AllocatingDiskId == rhs.AllocatingDiskId && + lhs.DeallocatingDiskId == rhs.DeallocatingDiskId; +} + //////////////////////////////////////////////////////////////////////////////// Y_UNIT_TEST_SUITE(TPendingCleanupTest) @@ -28,31 +43,42 @@ Y_UNIT_TEST_SUITE(TPendingCleanupTest) UNIT_ASSERT(!cleanup.IsEmpty()); UNIT_ASSERT(cleanup.Contains("foo")); - UNIT_ASSERT_VALUES_EQUAL("foo", cleanup.FindDiskId("x")); - UNIT_ASSERT_VALUES_EQUAL("foo", cleanup.FindDiskId("y")); - UNIT_ASSERT_VALUES_EQUAL("", cleanup.FindDiskId("z")); - UNIT_ASSERT_VALUES_EQUAL("", cleanup.FindDiskId("w")); + error = cleanup.Insert("bar", TVector {"x", "z"}, /*allocation=*/true); + UNIT_ASSERT_VALUES_EQUAL(S_OK, error.GetCode()); + UNIT_ASSERT(cleanup.Contains("bar")); + + using TOpt2Disk = TPendingCleanup::TOpt2Disk; + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk("bar", "foo"), cleanup.FindDiskId("x")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk("", "foo"), cleanup.FindDiskId("y")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk("bar", ""), cleanup.FindDiskId("z")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk(), cleanup.FindDiskId("a")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk(), cleanup.FindDiskId("b")); error = cleanup.Insert("foo", "z"); UNIT_ASSERT_VALUES_EQUAL(S_OK, error.GetCode()); - UNIT_ASSERT_VALUES_EQUAL("foo", cleanup.FindDiskId("z")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk("bar", "foo"), cleanup.FindDiskId("z")); - UNIT_ASSERT_VALUES_EQUAL("", cleanup.EraseDevice("w")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk(), cleanup.EraseDevice("w")); - UNIT_ASSERT_VALUES_EQUAL("", cleanup.EraseDevice("x")); - UNIT_ASSERT_VALUES_EQUAL("", cleanup.FindDiskId("x")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk(), cleanup.EraseDevice("x")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk(), cleanup.FindDiskId("x")); UNIT_ASSERT(!cleanup.IsEmpty()); - UNIT_ASSERT_VALUES_EQUAL("", cleanup.EraseDevice("y")); - UNIT_ASSERT_VALUES_EQUAL("", cleanup.FindDiskId("y")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk(), cleanup.EraseDevice("y")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk(), cleanup.FindDiskId("y")); UNIT_ASSERT(!cleanup.IsEmpty()); - UNIT_ASSERT_VALUES_EQUAL("foo", cleanup.EraseDevice("z")); - UNIT_ASSERT_VALUES_EQUAL("", cleanup.FindDiskId("z")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk("bar", "foo"), cleanup.EraseDevice("z")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk(), cleanup.FindDiskId("z")); UNIT_ASSERT(cleanup.IsEmpty()); UNIT_ASSERT(!cleanup.Contains("foo")); + error = cleanup.Insert("foo", "a", /*allocation=*/true); + UNIT_ASSERT_VALUES_EQUAL(S_OK, error.GetCode()); + error = cleanup.Insert("foo", "y", /*allocation=*/true); + UNIT_ASSERT_VALUES_EQUAL(S_OK, error.GetCode()); + error = cleanup.Insert("bar", "x"); UNIT_ASSERT_VALUES_EQUAL(S_OK, error.GetCode()); error = cleanup.Insert("bar", "y"); @@ -62,17 +88,20 @@ Y_UNIT_TEST_SUITE(TPendingCleanupTest) UNIT_ASSERT(!cleanup.IsEmpty()); UNIT_ASSERT(cleanup.Contains("bar")); - UNIT_ASSERT_VALUES_EQUAL("bar", cleanup.FindDiskId("x")); - UNIT_ASSERT_VALUES_EQUAL("bar", cleanup.FindDiskId("y")); - UNIT_ASSERT_VALUES_EQUAL("bar", cleanup.FindDiskId("z")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk("foo", "bar"), cleanup.FindDiskId("y")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk("foo", ""), cleanup.FindDiskId("a")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk("", "bar"), cleanup.FindDiskId("x")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk("", "bar"), cleanup.FindDiskId("z")); - UNIT_ASSERT(!cleanup.EraseDisk("foo")); + UNIT_ASSERT(!cleanup.EraseDisk("baz")); + UNIT_ASSERT(cleanup.EraseDisk("foo")); UNIT_ASSERT(cleanup.EraseDisk("bar")); UNIT_ASSERT(cleanup.IsEmpty()); - UNIT_ASSERT_VALUES_EQUAL("", cleanup.FindDiskId("x")); - UNIT_ASSERT_VALUES_EQUAL("", cleanup.FindDiskId("y")); - UNIT_ASSERT_VALUES_EQUAL("", cleanup.FindDiskId("z")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk(), cleanup.FindDiskId("a")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk(), cleanup.FindDiskId("y")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk(), cleanup.FindDiskId("x")); + UNIT_ASSERT_VALUES_EQUAL(TOpt2Disk(), cleanup.FindDiskId("z")); } }