From 661aa7b26c24e4fa43e11cf8086a0c1ef9852737 Mon Sep 17 00:00:00 2001 From: Sergey Belyakov Date: Wed, 23 Apr 2025 13:27:26 +0000 Subject: [PATCH 1/6] Limit in flight self-heal reassign requests by 1 --- .../blobstorage/ut_blobstorage/self_heal.cpp | 67 +++++++++++++++ ydb/core/blobstorage/ut_blobstorage/ya.make | 1 + ydb/core/mind/bscontroller/self_heal.cpp | 85 +++++++++++++++---- 3 files changed, 138 insertions(+), 15 deletions(-) create mode 100644 ydb/core/blobstorage/ut_blobstorage/self_heal.cpp diff --git a/ydb/core/blobstorage/ut_blobstorage/self_heal.cpp b/ydb/core/blobstorage/ut_blobstorage/self_heal.cpp new file mode 100644 index 000000000000..ffad5c1b45f9 --- /dev/null +++ b/ydb/core/blobstorage/ut_blobstorage/self_heal.cpp @@ -0,0 +1,67 @@ +#include + +Y_UNIT_TEST_SUITE(SelfHeal) { + void TestReassignThrottling() { + const TBlobStorageGroupType erasure = TBlobStorageGroupType::ErasureMirror3dc; + const ui32 groupsCount = 32; + + TEnvironmentSetup env({ + .NodeCount = erasure.BlobSubgroupSize(), + .Erasure = erasure, + }); + + // create 2 pdisks per node to allow self-healings and + // allocate groups + env.CreateBoxAndPool(2, groupsCount); + env.Sim(TDuration::Minutes(1)); + + auto base = env.FetchBaseConfig(); + UNIT_ASSERT_VALUES_EQUAL(base.GroupSize(), groupsCount); + + ui32 maxReassignsInFlight = 0; + + std::set reassignersInFlight; + + auto catchReassigns = [&](ui32 /*nodeId*/, std::unique_ptr& ev) { + if (ev->GetTypeRewrite() == TEvBlobStorage::TEvControllerConfigRequest::EventType) { + const auto& request = ev->Get()->Record.GetRequest(); + for (const auto& command : request.GetCommand()) { + if (command.GetCommandCase() == NKikimrBlobStorage::TConfigRequest::TCommand::kReassignGroupDisk) { + UNIT_ASSERT(!reassignersInFlight.contains(ev->Sender)); + reassignersInFlight.insert(ev->Sender); + maxReassignsInFlight = std::max(maxReassignsInFlight, (ui32)reassignersInFlight.size()); + } + } + } else if (ev->GetTypeRewrite() == TEvBlobStorage::TEvControllerConfigResponse::EventType) { + auto it = reassignersInFlight.find(ev->Recipient); + if (it != reassignersInFlight.end()) { + reassignersInFlight.erase(it); + } + } + return true; + }; + + env.Runtime->FilterFunction = catchReassigns; + + auto pdisk = base.GetPDisk(0); + // set FAULTY status on the chosen PDisk + { + NKikimrBlobStorage::TConfigRequest request; + auto* cmd = request.AddCommand()->MutableUpdateDriveStatus(); + cmd->MutableHostKey()->SetNodeId(pdisk.GetNodeId()); + cmd->SetPDiskId(pdisk.GetPDiskId()); + cmd->SetStatus(NKikimrBlobStorage::FAULTY); + auto res = env.Invoke(request); + UNIT_ASSERT_C(res.GetSuccess(), res.GetErrorDescription()); + UNIT_ASSERT_C(res.GetStatus(0).GetSuccess(), res.GetStatus(0).GetErrorDescription()); + } + + env.Sim(TDuration::Minutes(15)); + + UNIT_ASSERT_C(maxReassignsInFlight == 1, "maxReassignsInFlight# " << maxReassignsInFlight); + } + + Y_UNIT_TEST(ReassignThrottling) { + TestReassignThrottling(); + } +} diff --git a/ydb/core/blobstorage/ut_blobstorage/ya.make b/ydb/core/blobstorage/ut_blobstorage/ya.make index 8ff918025228..7451f3c12962 100644 --- a/ydb/core/blobstorage/ut_blobstorage/ya.make +++ b/ydb/core/blobstorage/ut_blobstorage/ya.make @@ -41,6 +41,7 @@ SRCS( recovery.cpp sanitize_groups.cpp scrub_fast.cpp + self_heal.cpp shred.cpp snapshots.cpp space_check.cpp diff --git a/ydb/core/mind/bscontroller/self_heal.cpp b/ydb/core/mind/bscontroller/self_heal.cpp index de5282bce921..d446543e1c81 100644 --- a/ydb/core/mind/bscontroller/self_heal.cpp +++ b/ydb/core/mind/bscontroller/self_heal.cpp @@ -253,13 +253,19 @@ namespace NKikimr::NBsController { struct TWithFaultyDisks {}; struct TWithInvalidLayout {}; + enum class EReassignStatus : ui8 { + NotNeeded = 0, + Enqueued, + Active, + }; + struct TGroupRecord : TIntrusiveListItem , TIntrusiveListItem { const TGroupId GroupId; TEvControllerUpdateSelfHealInfo::TGroupContent Content; - TActorId ReassignerActorId; // reassigner in flight + EReassignStatus ReassignStatus = EReassignStatus::NotNeeded; TDuration RetryTimeout = MinRetryTimeout; TMonotonic NextRetryTimestamp = TMonotonic::Zero(); std::shared_ptr Topology; @@ -294,6 +300,9 @@ namespace NKikimr::NBsController { static constexpr uint32_t GroupLayoutSanitizerOperationLogSize = 128; TOperationLog GroupLayoutSanitizerOperationLog; + std::deque ReassignQueue; + std::optional ActiveReassignerActorId = std::nullopt; + public: TSelfHealActor(ui64 tabletId, std::shared_ptr unreassignableGroups, THostRecordMap hostRecords, bool groupLayoutSanitizerEnabled, bool allowMultipleRealmsOccupation, bool donorMode, @@ -385,8 +394,11 @@ namespace NKikimr::NBsController { TGroupRecord& group = it->second; // kill reassigner, if it is working - if (group.ReassignerActorId) { - Send(group.ReassignerActorId, new TEvents::TEvPoison); + if (group.ReassignStatus == EReassignStatus::Active) { + Y_DEBUG_ABORT_UNLESS(ActiveReassignerActorId); + if (ActiveReassignerActorId) { + Send(*ActiveReassignerActorId, new TEvents::TEvPoison); + } } // remove the group @@ -425,8 +437,8 @@ namespace NKikimr::NBsController { ui64 counter = 0; for (TGroupRecord& group : GroupsWithFaultyDisks) { - if (group.ReassignerActorId || now < group.NextRetryTimestamp) { - continue; // we are already running reassigner for this group + if (group.ReassignStatus != EReassignStatus::NotNeeded || now < group.NextRetryTimestamp) { + continue; // reassign is already enqueued } if (group.UpdateConfigTxSeqNo < group.ResponseConfigTxSeqNo) { @@ -436,10 +448,14 @@ namespace NKikimr::NBsController { // check if it is possible to move anything out bool isSelfHealReasonDecommit; bool ignoreDegradedGroupsChecks; - if (const auto v = FindVDiskToReplace(group.Content, now, group.Topology.get(), &isSelfHealReasonDecommit, - &ignoreDegradedGroupsChecks)) { - group.ReassignerActorId = Register(new TReassignerActor(ControllerId, group.GroupId, group.Content, - *v, group.Topology, isSelfHealReasonDecommit, ignoreDegradedGroupsChecks, DonorMode)); + if (const std::optional vdiskId = FindVDiskToReplace(group.Content, now, group.Topology.get(), + &isSelfHealReasonDecommit, &ignoreDegradedGroupsChecks)) { + if (ActiveReassignerActorId) { + group.ReassignStatus = EReassignStatus::Enqueued; + ReassignQueue.push_back(group.GroupId); + } else { + CreateReassignerActor(group, vdiskId, isSelfHealReasonDecommit, ignoreDegradedGroupsChecks); + } } else { ++counter; // this group can't be reassigned right now @@ -488,14 +504,12 @@ namespace NKikimr::NBsController { } Y_ABORT_UNLESS(!group.LayoutValid); - if (group.ReassignerActorId || now < group.NextRetryTimestamp) { + if (group.ReassignStatus != EReassignStatus::NotNeeded || now < group.NextRetryTimestamp) { // nothing to do } else { ADD_RECORD_WITH_TIMESTAMP_TO_OPERATION_LOG(GroupLayoutSanitizerOperationLog, "Start sanitizing GroupId# " << group.GroupId << " GroupGeneration# " << group.Content.Generation); - group.ReassignerActorId = Register(new TReassignerActor(ControllerId, group.GroupId, group.Content, - std::nullopt, group.Topology, false /*isSelfHealReasonDecommit*/, - false /*ignoreDegradedGroupsChecks*/, DonorMode)); + CreateReassignerActor(group, std::nullopt, false, false); } } } @@ -602,9 +616,10 @@ namespace NKikimr::NBsController { } void Handle(TEvReassignerDone::TPtr& ev) { - if (const auto it = Groups.find(ev->Get()->GroupId); it != Groups.end() && it->second.ReassignerActorId == ev->Sender) { + if (const auto it = Groups.find(ev->Get()->GroupId); it != Groups.end()) { auto& group = it->second; - group.ReassignerActorId = {}; + group.ReassignStatus = EReassignStatus::NotNeeded; + ProcessReassignQueue(); const TMonotonic now = TActivationContext::Monotonic(); if (ev->Get()->Success) { @@ -654,6 +669,46 @@ namespace NKikimr::NBsController { Send(ev->Sender, new NMon::TEvRemoteHttpInfoRes(str.Str())); } + void ProcessReassignQueue() { + while (!ReassignQueue.empty()) { + TGroupId groupId = ReassignQueue.front(); + ReassignQueue.pop_front(); + if (CreateReassignerActorIfNeeded(groupId)) { + break; + } + } + } + + bool CreateReassignerActorIfNeeded(TGroupId groupId) { + auto it = Groups.find(groupId); + if (it == Groups.end()) { + // group is deleted + return false; + } + + TGroupRecord& group = it->second; + if (group.ReassignStatus == EReassignStatus::NotNeeded) { + return false; + } + + bool isSelfHealReasonDecommit; + bool ignoreDegradedGroupsChecks; + if (const std::optional vdiskId = FindVDiskToReplace(group.Content, TActivationContext::Monotonic(), + group.Topology.get(), &isSelfHealReasonDecommit, &ignoreDegradedGroupsChecks)) { + CreateReassignerActor(group, vdiskId, isSelfHealReasonDecommit, ignoreDegradedGroupsChecks); + return true; + } + return false; + } + + + void CreateReassignerActor(TGroupRecord& group, std::optional vdiskId, bool isSelfHealReasonDecommit, + bool ignoreDegradedGroupsChecks) { + group.ReassignStatus = EReassignStatus::Active; + ActiveReassignerActorId = Register(new TReassignerActor(ControllerId, group.GroupId, group.Content, + vdiskId, group.Topology, isSelfHealReasonDecommit, ignoreDegradedGroupsChecks, DonorMode)); + } + void RenderMonPage(IOutputStream& out, bool selfHealEnabled) { HTML(out) { TAG(TH2) { From a0981bdc5c9a5ffa49c7280aa867dbe12fa76ee3 Mon Sep 17 00:00:00 2001 From: Sergey Belyakov Date: Wed, 23 Apr 2025 14:06:15 +0000 Subject: [PATCH 2/6] Don't allow GroupLayoutSanitizer to create ReassignerActor if there is active already --- ydb/core/mind/bscontroller/self_heal.cpp | 34 +++++++++++++++++------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/ydb/core/mind/bscontroller/self_heal.cpp b/ydb/core/mind/bscontroller/self_heal.cpp index d446543e1c81..f052f23a6f2a 100644 --- a/ydb/core/mind/bscontroller/self_heal.cpp +++ b/ydb/core/mind/bscontroller/self_heal.cpp @@ -300,7 +300,8 @@ namespace NKikimr::NBsController { static constexpr uint32_t GroupLayoutSanitizerOperationLogSize = 128; TOperationLog GroupLayoutSanitizerOperationLog; - std::deque ReassignQueue; + std::deque SelfHealReassignQueue; + std::deque GroupLayoutSanitizerReassignQueue; std::optional ActiveReassignerActorId = std::nullopt; public: @@ -452,7 +453,7 @@ namespace NKikimr::NBsController { &isSelfHealReasonDecommit, &ignoreDegradedGroupsChecks)) { if (ActiveReassignerActorId) { group.ReassignStatus = EReassignStatus::Enqueued; - ReassignQueue.push_back(group.GroupId); + SelfHealReassignQueue.push_back(group.GroupId); } else { CreateReassignerActor(group, vdiskId, isSelfHealReasonDecommit, ignoreDegradedGroupsChecks); } @@ -509,7 +510,12 @@ namespace NKikimr::NBsController { } else { ADD_RECORD_WITH_TIMESTAMP_TO_OPERATION_LOG(GroupLayoutSanitizerOperationLog, "Start sanitizing GroupId# " << group.GroupId << " GroupGeneration# " << group.Content.Generation); - CreateReassignerActor(group, std::nullopt, false, false); + if (ActiveReassignerActorId) { + group.ReassignStatus = EReassignStatus::Enqueued; + GroupLayoutSanitizerReassignQueue.push_back(group.GroupId); + } else { + CreateReassignerActor(group, std::nullopt, false, false); + } } } } @@ -619,7 +625,7 @@ namespace NKikimr::NBsController { if (const auto it = Groups.find(ev->Get()->GroupId); it != Groups.end()) { auto& group = it->second; group.ReassignStatus = EReassignStatus::NotNeeded; - ProcessReassignQueue(); + ProcessReassignQueues(); const TMonotonic now = TActivationContext::Monotonic(); if (ev->Get()->Success) { @@ -669,12 +675,20 @@ namespace NKikimr::NBsController { Send(ev->Sender, new NMon::TEvRemoteHttpInfoRes(str.Str())); } - void ProcessReassignQueue() { - while (!ReassignQueue.empty()) { - TGroupId groupId = ReassignQueue.front(); - ReassignQueue.pop_front(); - if (CreateReassignerActorIfNeeded(groupId)) { - break; + void ProcessReassignQueues() { + while (!ActiveReassignerActorId && !SelfHealReassignQueue.empty()) { + TGroupId groupId = SelfHealReassignQueue.front(); + SelfHealReassignQueue.pop_front(); + CreateReassignerActorIfNeeded(groupId); + } + + while (!ActiveReassignerActorId && !GroupLayoutSanitizerReassignQueue.empty()) { + TGroupId groupId = GroupLayoutSanitizerReassignQueue.front(); + GroupLayoutSanitizerReassignQueue.pop_front(); + auto it = Groups.find(groupId); + if (it != Groups.end()) { + TGroupRecord& group = it->second; + CreateReassignerActor(group, std::nullopt, false, false); } } } From 2b62f21ebef1f069781c7c006f39d8f499469908 Mon Sep 17 00:00:00 2001 From: Sergey Belyakov Date: Thu, 24 Apr 2025 09:44:35 +0000 Subject: [PATCH 3/6] Fix stuck reassigns --- ydb/core/mind/bscontroller/self_heal.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ydb/core/mind/bscontroller/self_heal.cpp b/ydb/core/mind/bscontroller/self_heal.cpp index f052f23a6f2a..036896b792ea 100644 --- a/ydb/core/mind/bscontroller/self_heal.cpp +++ b/ydb/core/mind/bscontroller/self_heal.cpp @@ -622,6 +622,8 @@ namespace NKikimr::NBsController { } void Handle(TEvReassignerDone::TPtr& ev) { + ActiveReassignerActorId = std::nullopt; + if (const auto it = Groups.find(ev->Get()->GroupId); it != Groups.end()) { auto& group = it->second; group.ReassignStatus = EReassignStatus::NotNeeded; @@ -646,6 +648,8 @@ namespace NKikimr::NBsController { } CheckGroups(); + } else { + ProcessReassignQueues(); } } From 689ccb6b024eee5b5ce97a3b05dd2a422734a389 Mon Sep 17 00:00:00 2001 From: Sergey Belyakov Date: Fri, 25 Apr 2025 08:49:38 +0000 Subject: [PATCH 4/6] Check Reassigner ActorId --- ydb/core/mind/bscontroller/self_heal.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ydb/core/mind/bscontroller/self_heal.cpp b/ydb/core/mind/bscontroller/self_heal.cpp index 036896b792ea..138b27cf8d2f 100644 --- a/ydb/core/mind/bscontroller/self_heal.cpp +++ b/ydb/core/mind/bscontroller/self_heal.cpp @@ -622,9 +622,10 @@ namespace NKikimr::NBsController { } void Handle(TEvReassignerDone::TPtr& ev) { - ActiveReassignerActorId = std::nullopt; + Y_ABORT_UNLESS(ActiveReassignerActorId); + TActorId reassigner = *std::exchange(ActiveReassignerActorId, std::nullopt); - if (const auto it = Groups.find(ev->Get()->GroupId); it != Groups.end()) { + if (const auto it = Groups.find(ev->Get()->GroupId); it != Groups.end() && reassigner == ev->Sender) { auto& group = it->second; group.ReassignStatus = EReassignStatus::NotNeeded; ProcessReassignQueues(); From 4709073db696a409528f22dfc1f112b37458d2ea Mon Sep 17 00:00:00 2001 From: Sergey Belyakov Date: Fri, 25 Apr 2025 10:02:44 +0000 Subject: [PATCH 5/6] Remove copypaste --- ydb/core/mind/bscontroller/self_heal.cpp | 92 +++++++++++------------- 1 file changed, 41 insertions(+), 51 deletions(-) diff --git a/ydb/core/mind/bscontroller/self_heal.cpp b/ydb/core/mind/bscontroller/self_heal.cpp index 138b27cf8d2f..b0a1954acab2 100644 --- a/ydb/core/mind/bscontroller/self_heal.cpp +++ b/ydb/core/mind/bscontroller/self_heal.cpp @@ -284,7 +284,8 @@ namespace NKikimr::NBsController { THashMap Groups; TIntrusiveList GroupsWithFaultyDisks; TIntrusiveList GroupsWithInvalidLayout; - std::shared_ptr UnreassignableGroups; + std::unordered_set UnreassignableGroups; + std::shared_ptr UnreassignableGroupsCount; bool GroupLayoutSanitizerEnabled; bool AllowMultipleRealmsOccupation; bool DonorMode; @@ -310,7 +311,7 @@ namespace NKikimr::NBsController { std::shared_ptr enableSelfHealWithDegraded, std::shared_ptr groupsWithInvalidLayoutCounter) : TabletId(tabletId) - , UnreassignableGroups(std::move(unreassignableGroups)) + , UnreassignableGroupsCount(std::move(unreassignableGroups)) , GroupLayoutSanitizerEnabled(groupLayoutSanitizerEnabled) , AllowMultipleRealmsOccupation(allowMultipleRealmsOccupation) , DonorMode(donorMode) @@ -435,8 +436,6 @@ namespace NKikimr::NBsController { void CheckGroups() { const TMonotonic now = TActivationContext::Monotonic(); - ui64 counter = 0; - for (TGroupRecord& group : GroupsWithFaultyDisks) { if (group.ReassignStatus != EReassignStatus::NotNeeded || now < group.NextRetryTimestamp) { continue; // reassign is already enqueued @@ -446,42 +445,8 @@ namespace NKikimr::NBsController { continue; // response from bsc was received before selfheal info update } - // check if it is possible to move anything out - bool isSelfHealReasonDecommit; - bool ignoreDegradedGroupsChecks; - if (const std::optional vdiskId = FindVDiskToReplace(group.Content, now, group.Topology.get(), - &isSelfHealReasonDecommit, &ignoreDegradedGroupsChecks)) { - if (ActiveReassignerActorId) { - group.ReassignStatus = EReassignStatus::Enqueued; - SelfHealReassignQueue.push_back(group.GroupId); - } else { - CreateReassignerActor(group, vdiskId, isSelfHealReasonDecommit, ignoreDegradedGroupsChecks); - } - } else { - ++counter; // this group can't be reassigned right now - - auto log = [&]() { - TStringStream ss; - ss << "["; - bool first = true; - for (const auto& [vdiskId, vdisk] : group.Content.VDisks) { - if (!std::exchange(first, false)) { - ss << ","; - } - ss << "{"; - ss << vdiskId; - ss << (IsReady(vdisk, now) ? " Ready" : " NotReady"); - ss << (vdisk.Faulty ? " Faulty" : ""); - ss << (vdisk.Bad ? " IsBad" : ""); - ss << (vdisk.Decommitted ? " Decommitted" : ""); - ss << "}"; - } - ss << "]"; - return ss.Str(); - }; - - STLOG(PRI_INFO, BS_SELFHEAL, BSSH11, "group can't be reassigned right now " << log(), (GroupId, group.GroupId)); - } + group.ReassignStatus = EReassignStatus::Enqueued; + SelfHealReassignQueue.push_back(group.GroupId); } if (GroupLayoutSanitizerEnabled) { @@ -510,18 +475,15 @@ namespace NKikimr::NBsController { } else { ADD_RECORD_WITH_TIMESTAMP_TO_OPERATION_LOG(GroupLayoutSanitizerOperationLog, "Start sanitizing GroupId# " << group.GroupId << " GroupGeneration# " << group.Content.Generation); - if (ActiveReassignerActorId) { - group.ReassignStatus = EReassignStatus::Enqueued; - GroupLayoutSanitizerReassignQueue.push_back(group.GroupId); - } else { - CreateReassignerActor(group, std::nullopt, false, false); - } + group.ReassignStatus = EReassignStatus::Enqueued; + GroupLayoutSanitizerReassignQueue.push_back(group.GroupId); } } } + ProcessReassignQueues(); GroupsWithInvalidLayoutCounter->store(GroupsWithInvalidLayout.Size()); - UnreassignableGroups->store(counter); + UnreassignableGroupsCount->store(UnreassignableGroups.size()); } void UpdateGroupLayoutInformation(TGroupRecord& group) { @@ -628,7 +590,6 @@ namespace NKikimr::NBsController { if (const auto it = Groups.find(ev->Get()->GroupId); it != Groups.end() && reassigner == ev->Sender) { auto& group = it->second; group.ReassignStatus = EReassignStatus::NotNeeded; - ProcessReassignQueues(); const TMonotonic now = TActivationContext::Monotonic(); if (ev->Get()->Success) { @@ -647,11 +608,10 @@ namespace NKikimr::NBsController { "Sanitizing failed GroupId# " << group.GroupId << " ErrorReason# " << ev->Get()->ErrorReason); } } - CheckGroups(); - } else { - ProcessReassignQueues(); } + + ProcessReassignQueues(); } using TVDiskInfo = TEvControllerUpdateSelfHealInfo::TGroupContent::TVDiskInfo; @@ -707,15 +667,45 @@ namespace NKikimr::NBsController { TGroupRecord& group = it->second; if (group.ReassignStatus == EReassignStatus::NotNeeded) { + // Group is already fully healed return false; } + // check if it is possible to move anything out bool isSelfHealReasonDecommit; bool ignoreDegradedGroupsChecks; if (const std::optional vdiskId = FindVDiskToReplace(group.Content, TActivationContext::Monotonic(), group.Topology.get(), &isSelfHealReasonDecommit, &ignoreDegradedGroupsChecks)) { + if (auto it = UnreassignableGroups.find(groupId); it != UnreassignableGroups.end()) { + UnreassignableGroups.erase(it); + } CreateReassignerActor(group, vdiskId, isSelfHealReasonDecommit, ignoreDegradedGroupsChecks); return true; + } else { + UnreassignableGroups.insert(groupId); + TMonotonic now = TActivationContext::Monotonic(); + // unable to reassign VDisk + auto log = [&]() { + TStringStream ss; + ss << "["; + bool first = true; + for (const auto& [vdiskId, vdisk] : group.Content.VDisks) { + if (!std::exchange(first, false)) { + ss << ","; + } + ss << "{"; + ss << vdiskId; + ss << (IsReady(vdisk, now) ? " Ready" : " NotReady"); + ss << (vdisk.Faulty ? " Faulty" : ""); + ss << (vdisk.Bad ? " IsBad" : ""); + ss << (vdisk.Decommitted ? " Decommitted" : ""); + ss << "}"; + } + ss << "]"; + return ss.Str(); + }; + + STLOG(PRI_INFO, BS_SELFHEAL, BSSH11, "group can't be reassigned right now " << log(), (GroupId, groupId)); } return false; } From ce0c35f8745623d361eea6e0bc9ca5d02418c6f7 Mon Sep 17 00:00:00 2001 From: Sergey Belyakov Date: Fri, 25 Apr 2025 11:04:29 +0000 Subject: [PATCH 6/6] Don't enqueue unreassignable groups for reassign --- ydb/core/mind/bscontroller/self_heal.cpp | 35 +++++++++++++++++------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/ydb/core/mind/bscontroller/self_heal.cpp b/ydb/core/mind/bscontroller/self_heal.cpp index b0a1954acab2..e59d882c0a2e 100644 --- a/ydb/core/mind/bscontroller/self_heal.cpp +++ b/ydb/core/mind/bscontroller/self_heal.cpp @@ -444,9 +444,8 @@ namespace NKikimr::NBsController { if (group.UpdateConfigTxSeqNo < group.ResponseConfigTxSeqNo) { continue; // response from bsc was received before selfheal info update } - - group.ReassignStatus = EReassignStatus::Enqueued; - SelfHealReassignQueue.push_back(group.GroupId); + + EnqueueReassign(group, EGroupRepairOperation::SelfHeal); } if (GroupLayoutSanitizerEnabled) { @@ -475,8 +474,7 @@ namespace NKikimr::NBsController { } else { ADD_RECORD_WITH_TIMESTAMP_TO_OPERATION_LOG(GroupLayoutSanitizerOperationLog, "Start sanitizing GroupId# " << group.GroupId << " GroupGeneration# " << group.Content.Generation); - group.ReassignStatus = EReassignStatus::Enqueued; - GroupLayoutSanitizerReassignQueue.push_back(group.GroupId); + EnqueueReassign(group, EGroupRepairOperation::GroupLayoutSanitizer); } } } @@ -586,8 +584,9 @@ namespace NKikimr::NBsController { void Handle(TEvReassignerDone::TPtr& ev) { Y_ABORT_UNLESS(ActiveReassignerActorId); TActorId reassigner = *std::exchange(ActiveReassignerActorId, std::nullopt); + Y_ABORT_UNLESS(reassigner == ev->Sender); - if (const auto it = Groups.find(ev->Get()->GroupId); it != Groups.end() && reassigner == ev->Sender) { + if (const auto it = Groups.find(ev->Get()->GroupId); it != Groups.end()) { auto& group = it->second; group.ReassignStatus = EReassignStatus::NotNeeded; @@ -610,7 +609,6 @@ namespace NKikimr::NBsController { } CheckGroups(); } - ProcessReassignQueues(); } @@ -644,7 +642,7 @@ namespace NKikimr::NBsController { while (!ActiveReassignerActorId && !SelfHealReassignQueue.empty()) { TGroupId groupId = SelfHealReassignQueue.front(); SelfHealReassignQueue.pop_front(); - CreateReassignerActorIfNeeded(groupId); + CreateReassignerActorIfNeededForSelfHeal(groupId); } while (!ActiveReassignerActorId && !GroupLayoutSanitizerReassignQueue.empty()) { @@ -658,7 +656,7 @@ namespace NKikimr::NBsController { } } - bool CreateReassignerActorIfNeeded(TGroupId groupId) { + bool CreateReassignerActorIfNeededForSelfHeal(TGroupId groupId) { auto it = Groups.find(groupId); if (it == Groups.end()) { // group is deleted @@ -682,9 +680,11 @@ namespace NKikimr::NBsController { CreateReassignerActor(group, vdiskId, isSelfHealReasonDecommit, ignoreDegradedGroupsChecks); return true; } else { + // unable to reassign VDisk UnreassignableGroups.insert(groupId); + group.ReassignStatus = EReassignStatus::NotNeeded; + TMonotonic now = TActivationContext::Monotonic(); - // unable to reassign VDisk auto log = [&]() { TStringStream ss; ss << "["; @@ -714,10 +714,25 @@ namespace NKikimr::NBsController { void CreateReassignerActor(TGroupRecord& group, std::optional vdiskId, bool isSelfHealReasonDecommit, bool ignoreDegradedGroupsChecks) { group.ReassignStatus = EReassignStatus::Active; + Y_ABORT_UNLESS(!ActiveReassignerActorId); ActiveReassignerActorId = Register(new TReassignerActor(ControllerId, group.GroupId, group.Content, vdiskId, group.Topology, isSelfHealReasonDecommit, ignoreDegradedGroupsChecks, DonorMode)); } + void EnqueueReassign(TGroupRecord& group, EGroupRepairOperation operation) { + group.ReassignStatus = EReassignStatus::Enqueued; + switch (operation) { + case EGroupRepairOperation::SelfHeal: + SelfHealReassignQueue.push_back(group.GroupId); + break; + case EGroupRepairOperation::GroupLayoutSanitizer: + GroupLayoutSanitizerReassignQueue.push_back(group.GroupId); + break; + default: + Y_ABORT("Unknown operation"); + } + } + void RenderMonPage(IOutputStream& out, bool selfHealEnabled) { HTML(out) { TAG(TH2) {