Make BSC report about group failure model violations more detailed

alexvru · alexvru · commit 7a94da4ecb56 · 2024-05-31T12:42:44.000Z
diff --git a/ydb/core/blobstorage/ut_blobstorage/restart_pdisk.cpp b/ydb/core/blobstorage/ut_blobstorage/restart_pdisk.cpp
@@ -35,6 +35,7 @@ Y_UNIT_TEST_SUITE(BSCRestartPDisk) {
             auto& diskId =  it->first;
 
             NKikimrBlobStorage::TConfigRequest request;
+            request.SetIgnoreDegradedGroupsChecks(true);
 
             NKikimrBlobStorage::TRestartPDisk* cmd = request.AddCommand()->MutableRestartPDisk();
             auto pdiskId = cmd->MutableTargetPDiskId();
@@ -50,7 +51,7 @@ Y_UNIT_TEST_SUITE(BSCRestartPDisk) {
                 // Restarting third disk will not be allowed.
                 UNIT_ASSERT_C(!response.GetSuccess(), "Restart should've been prohibited");
 
-                UNIT_ASSERT_STRING_CONTAINS(response.GetErrorDescription(), "ExpectedStatus# DISINTEGRATED");
+                UNIT_ASSERT_STRING_CONTAINS(response.GetErrorDescription(), "Disintegrated");
                 break;
             }
         }
diff --git a/ydb/core/cms/sentinel.cpp b/ydb/core/cms/sentinel.cpp
@@ -984,6 +984,7 @@ class TSentinel: public TActorBootstrapped<TSentinel> {
             command.SetPDiskId(id.DiskId);
             command.SetStatus(info->GetStatus());
         }
+        request->Record.MutableRequest()->SetIgnoreDisintegratedGroupsChecks(true);
 
         NTabletPipe::SendData(SelfId(), CmsState->BSControllerPipe, request.Release(), ++SentinelState->ChangeRequestId);
     }
diff --git a/ydb/core/mind/bscontroller/cmds_box.cpp b/ydb/core/mind/bscontroller/cmds_box.cpp
@@ -212,8 +212,11 @@ namespace NKikimr::NBsController {
 
         for (const auto& [id, slot] : pdisk->VSlotsOnPDisk) {
             if (slot->Group) {
+                auto *m = VSlots.FindForUpdate(slot->VSlotId);
+                m->Status = NKikimrBlobStorage::EVDiskStatus::INIT_PENDING;
+                m->IsReady = false;
                 TGroupInfo *group = Groups.FindForUpdate(slot->Group->ID);
-
+                GroupFailureModelChanged.insert(slot->Group->ID);
                 group->CalculateGroupStatus();
             }
         }
diff --git a/ydb/core/mind/bscontroller/cmds_drive_status.cpp b/ydb/core/mind/bscontroller/cmds_drive_status.cpp
@@ -39,6 +39,7 @@ namespace NKikimr::NBsController {
             for (const auto& [id, slot] : pdisk->VSlotsOnPDisk) {
                 if (slot->Group) {
                     TGroupInfo *group = Groups.FindForUpdate(slot->Group->ID);
+                    GroupFailureModelChanged.insert(group->ID);
                     group->CalculateGroupStatus();
                 }
             }
diff --git a/ydb/core/mind/bscontroller/config.cpp b/ydb/core/mind/bscontroller/config.cpp
@@ -293,7 +293,7 @@ namespace NKikimr::NBsController {
 
         bool TBlobStorageController::CommitConfigUpdates(TConfigState& state, bool suppressFailModelChecking,
                 bool suppressDegradedGroupsChecking, bool suppressDisintegratedGroupsChecking,
-                TTransactionContext& txc, TString *errorDescription) {
+                TTransactionContext& txc, TString *errorDescription, NKikimrBlobStorage::TConfigResponse *response) {
             NIceDb::TNiceDb db(txc.DB);
 
             for (TGroupId groupId : state.GroupContentChanged) {
@@ -309,16 +309,20 @@ namespace NKikimr::NBsController {
                 }
             }
 
+            bool errors = false;
+            std::vector<TGroupId> disintegratedByExpectedStatus;
+            std::vector<TGroupId> disintegrated;
+            std::vector<TGroupId> degraded;
+
             if (!suppressDisintegratedGroupsChecking) {
                 for (auto&& [base, overlay] : state.Groups.Diff()) {
                     if (base && overlay->second) {
                         const TGroupInfo::TGroupStatus& prev = base->second->Status;
                         const TGroupInfo::TGroupStatus& status = overlay->second->Status;
                         if (status.ExpectedStatus == NKikimrBlobStorage::TGroupStatus::DISINTEGRATED &&
                                 status.ExpectedStatus != prev.ExpectedStatus) { // status did really change
-                            *errorDescription = TStringBuilder() << "GroupId# " << overlay->first
-                                << " ExpectedStatus# DISINTEGRATED";
-                           return false;
+                            disintegratedByExpectedStatus.push_back(overlay->first);
+                            errors = true;
                         }
                      }
                 }
@@ -340,20 +344,44 @@ namespace NKikimr::NBsController {
                         // check the failure model
                         auto& checker = *topology.QuorumChecker;
                         if (!checker.CheckFailModelForGroup(failed)) {
-                            *errorDescription = TStringBuilder() << "GroupId# " << groupId
-                                << " may lose data while modifying group";
-                            return false;
+                            disintegrated.push_back(groupId);
+                            errors = true;
                         } else if (!suppressDegradedGroupsChecking && checker.IsDegraded(failed)) {
-                            *errorDescription = TStringBuilder() << "GroupId# " << groupId
-                                << " may become DEGRADED while modifying group";
-                            return false;
+                            degraded.push_back(groupId);
+                            errors = true;
                         }
                     } else {
                         Y_ABORT_UNLESS(group); // group must exist
                     }
                 }
             }
 
+            if (errors) {
+                TStringStream msg;
+                if (!degraded.empty()) {
+                    msg << "Degraded GroupIds# " << FormatList(degraded) << ' ';
+                    if (response) {
+                        response->MutableGroupsGetDegraded()->Add(degraded.begin(), degraded.end());
+                    }
+                }
+                if (!disintegrated.empty()) {
+                    msg << "Disintegrated GroupIds# " << FormatList(disintegrated) << ' ';
+                    if (response) {
+                        response->MutableGroupsGetDisintegrated()->Add(disintegrated.begin(), disintegrated.end());
+                    }
+                }
+                if (!disintegratedByExpectedStatus.empty()) {
+                    msg << "DisintegratedByExpectedStatus GroupIds# " << FormatList(disintegratedByExpectedStatus) << ' ';
+                    if (response) {
+                        response->MutableGroupsGetDisintegratedByExpectedStatus()->Add(disintegratedByExpectedStatus.begin(),
+                            disintegratedByExpectedStatus.end());
+                    }
+                }
+                *errorDescription = msg.Str();
+                errorDescription->pop_back();
+                return false;
+            }
+
             // trim PDisks awaiting deletion
             for (const TPDiskId& pdiskId : state.PDisksToRemove) {
                 TPDiskInfo *pdiskInfo = state.PDisks.FindForUpdate(pdiskId);
diff --git a/ydb/core/mind/bscontroller/config_cmd.cpp b/ydb/core/mind/bscontroller/config_cmd.cpp
@@ -265,7 +265,8 @@ namespace NKikimr::NBsController {
 
                 const bool doLogCommand = Success && State->Changed();
                 Success = Success && Self->CommitConfigUpdates(*State, Cmd.GetIgnoreGroupFailModelChecks(),
-                    Cmd.GetIgnoreDegradedGroupsChecks(), Cmd.GetIgnoreDisintegratedGroupsChecks(), txc, &Error);
+                    Cmd.GetIgnoreDegradedGroupsChecks(), Cmd.GetIgnoreDisintegratedGroupsChecks(), txc, &Error,
+                    Response);
 
                 Finish();
                 if (doLogCommand) {
diff --git a/ydb/core/mind/bscontroller/defs.h b/ydb/core/mind/bscontroller/defs.h
@@ -12,6 +12,7 @@
 #include <ydb/core/base/tablet_pipe.h>
 #include <ydb/core/blobstorage/base/blobstorage_events.h>
 #include <ydb/core/blobstorage/base/blobstorage_vdiskid.h>
+#include <ydb/core/blobstorage/base/utility.h>
 #include <ydb/core/blobstorage/groupinfo/blobstorage_groupinfo.h>
 #include <ydb/core/blobstorage/groupinfo/blobstorage_groupinfo_blobmap.h>
 #include <ydb/core/blobstorage/groupinfo/blobstorage_groupinfo_sets.h>
diff --git a/ydb/core/mind/bscontroller/impl.h b/ydb/core/mind/bscontroller/impl.h
@@ -499,12 +499,12 @@ class TBlobStorageController : public TActor<TBlobStorageController>, public TTa
             switch (Status) {
                 case NKikimrBlobStorage::EDriveStatus::UNKNOWN:
                 case NKikimrBlobStorage::EDriveStatus::BROKEN:
-                    return false;
-
-                case NKikimrBlobStorage::EDriveStatus::ACTIVE:
                 case NKikimrBlobStorage::EDriveStatus::INACTIVE:
                 case NKikimrBlobStorage::EDriveStatus::FAULTY:
                 case NKikimrBlobStorage::EDriveStatus::TO_BE_REMOVED:
+                    return false;
+
+                case NKikimrBlobStorage::EDriveStatus::ACTIVE:
                     return true;
 
                 case NKikimrBlobStorage::EDriveStatus::EDriveStatus_INT_MIN_SENTINEL_DO_NOT_USE_:
@@ -1572,7 +1572,8 @@ class TBlobStorageController : public TActor<TBlobStorageController>, public TTa
     void UpdateSystemViews();
 
     bool CommitConfigUpdates(TConfigState& state, bool suppressFailModelChecking, bool suppressDegradedGroupsChecking,
-        bool suppressDisintegratedGroupsChecking, TTransactionContext& txc, TString *errorDescription);
+        bool suppressDisintegratedGroupsChecking, TTransactionContext& txc, TString *errorDescription,
+        NKikimrBlobStorage::TConfigResponse *response = nullptr);
 
     void CommitSelfHealUpdates(TConfigState& state);
     void CommitScrubUpdates(TConfigState& state, TTransactionContext& txc);
diff --git a/ydb/core/mind/bscontroller/virtual_group.cpp b/ydb/core/mind/bscontroller/virtual_group.cpp
@@ -87,6 +87,7 @@ namespace NKikimr::NBsController {
             group->SeenOperational = true;
         }
 
+        GroupFailureModelChanged.insert(group->ID);
         group->CalculateGroupStatus();
 
         NKikimrBlobDepot::TBlobDepotConfig config;
@@ -127,6 +128,7 @@ namespace NKikimr::NBsController {
             group->HiveId = cmd.HasHiveId() ? MakeMaybe(cmd.GetHiveId()) : Nothing();
             group->Database = cmd.HasDatabase() ? MakeMaybe(cmd.GetDatabase()) : Nothing();
             group->NeedAlter = true;
+            GroupFailureModelChanged.insert(group->ID);
             group->CalculateGroupStatus();
 
             NKikimrBlobDepot::TBlobDepotConfig config;
diff --git a/ydb/core/protos/blobstorage_config.proto b/ydb/core/protos/blobstorage_config.proto
@@ -771,4 +771,7 @@ message TConfigResponse {
     bool Success = 2;
     string ErrorDescription = 3;
     uint64 ConfigTxSeqNo = 4;
+    repeated uint32 GroupsGetDegraded = 5;
+    repeated uint32 GroupsGetDisintegrated = 6;
+    repeated uint32 GroupsGetDisintegratedByExpectedStatus = 7;
 }

Original file line number	Diff line number	Diff line change
`@@ -35,6 +35,7 @@ Y_UNIT_TEST_SUITE(BSCRestartPDisk) {`
`35`	`35`	`auto& diskId = it->first;`
`36`	`36`
`37`	`37`	`NKikimrBlobStorage::TConfigRequest request;`
	`38`	`+ request.SetIgnoreDegradedGroupsChecks(true);`
`38`	`39`
`39`	`40`	`NKikimrBlobStorage::TRestartPDisk* cmd = request.AddCommand()->MutableRestartPDisk();`
`40`	`41`	`auto pdiskId = cmd->MutableTargetPDiskId();`
`@@ -50,7 +51,7 @@ Y_UNIT_TEST_SUITE(BSCRestartPDisk) {`
`50`	`51`	`// Restarting third disk will not be allowed.`
`51`	`52`	`UNIT_ASSERT_C(!response.GetSuccess(), "Restart should've been prohibited");`
`52`	`53`
`53`		`- UNIT_ASSERT_STRING_CONTAINS(response.GetErrorDescription(), "ExpectedStatus# DISINTEGRATED");`
	`54`	`+ UNIT_ASSERT_STRING_CONTAINS(response.GetErrorDescription(), "Disintegrated");`
`54`	`55`	`break;`
`55`	`56`	`}`
`56`	`57`	`}`
Original file line number	Diff line number	Diff line change
`@@ -984,6 +984,7 @@ class TSentinel: public TActorBootstrapped<TSentinel> {`
`984`	`984`	`command.SetPDiskId(id.DiskId);`
`985`	`985`	`command.SetStatus(info->GetStatus());`
`986`	`986`	`}`
	`987`	`+ request->Record.MutableRequest()->SetIgnoreDisintegratedGroupsChecks(true);`
`987`	`988`
`988`	`989`	`NTabletPipe::SendData(SelfId(), CmsState->BSControllerPipe, request.Release(), ++SentinelState->ChangeRequestId);`
`989`	`990`	`}`
Original file line number	Diff line number	Diff line change
`@@ -212,8 +212,11 @@ namespace NKikimr::NBsController {`
`212`	`212`
`213`	`213`	`for (const auto& [id, slot] : pdisk->VSlotsOnPDisk) {`
`214`	`214`	`if (slot->Group) {`
	`215`	`+ auto *m = VSlots.FindForUpdate(slot->VSlotId);`
	`216`	`+ m->Status = NKikimrBlobStorage::EVDiskStatus::INIT_PENDING;`
	`217`	`+ m->IsReady = false;`
`215`	`218`	`TGroupInfo *group = Groups.FindForUpdate(slot->Group->ID);`
`216`		`-`
	`219`	`+ GroupFailureModelChanged.insert(slot->Group->ID);`
`217`	`220`	`group->CalculateGroupStatus();`
`218`	`221`	`}`
`219`	`222`	`}`
Original file line number	Diff line number	Diff line change
`@@ -39,6 +39,7 @@ namespace NKikimr::NBsController {`
`39`	`39`	`for (const auto& [id, slot] : pdisk->VSlotsOnPDisk) {`
`40`	`40`	`if (slot->Group) {`
`41`	`41`	`TGroupInfo *group = Groups.FindForUpdate(slot->Group->ID);`
	`42`	`+ GroupFailureModelChanged.insert(group->ID);`
`42`	`43`	`group->CalculateGroupStatus();`
`43`	`44`	`}`
`44`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -771,4 +771,7 @@ message TConfigResponse {`
`771`	`771`	`bool Success = 2;`
`772`	`772`	`string ErrorDescription = 3;`
`773`	`773`	`uint64 ConfigTxSeqNo = 4;`
	`774`	`+ repeated uint32 GroupsGetDegraded = 5;`
	`775`	`+ repeated uint32 GroupsGetDisintegrated = 6;`
	`776`	`+ repeated uint32 GroupsGetDisintegratedByExpectedStatus = 7;`
`774`	`777`	`}`