Correct PDisk/VDisk metrics reporting (#5776)

alexvru · web-flow · commit f6b3726e6d15 · 2024-06-20T17:12:52.000+03:00
diff --git a/ydb/core/blobstorage/nodewarden/node_warden_impl.cpp b/ydb/core/blobstorage/nodewarden/node_warden_impl.cpp
@@ -338,7 +338,6 @@ void TNodeWarden::Handle(NPDisk::TEvSlayResult::TPtr ev) {
                 SendVDiskReport(vslotId, msg.VDiskId, NKikimrBlobStorage::TEvControllerNodeReport::WIPED);
                 TVDiskRecord& vdisk = vdiskIt->second;
                 StartLocalVDiskActor(vdisk); // restart actor after successful wiping
-                SendDiskMetrics(false);
             }
             break;
 
@@ -540,6 +539,8 @@ void TNodeWarden::Handle(TEvBlobStorage::TEvControllerUpdateDiskStatus::TPtr ev)
 
     auto& record = ev->Get()->Record;
 
+    std::unique_ptr<TEvBlobStorage::TEvControllerUpdateDiskStatus> updateDiskStatus;
+
     for (const NKikimrBlobStorage::TVDiskMetrics& m : record.GetVDisksMetrics()) {
         Y_ABORT_UNLESS(m.HasVSlotId());
         const TVSlotId vslotId(m.GetVSlotId());
@@ -554,8 +555,11 @@ void TNodeWarden::Handle(TEvBlobStorage::TEvControllerUpdateDiskStatus::TPtr ev)
                     VDisksWithUnreportedMetrics.PushBack(&vdisk);
                 }
             } else {
+                if (!updateDiskStatus) {
+                    updateDiskStatus.reset(new TEvBlobStorage::TEvControllerUpdateDiskStatus);
+                }
+                updateDiskStatus->Record.AddVDisksMetrics()->CopyFrom(m);
                 vdisk.VDiskMetrics.emplace(m);
-                VDisksWithUnreportedMetrics.PushBack(&vdisk);
             }
         }
     }
@@ -571,11 +575,18 @@ void TNodeWarden::Handle(TEvBlobStorage::TEvControllerUpdateDiskStatus::TPtr ev)
                     PDisksWithUnreportedMetrics.PushBack(&pdisk);
                 }
             } else {
+                if (!updateDiskStatus) {
+                    updateDiskStatus.reset(new TEvBlobStorage::TEvControllerUpdateDiskStatus);
+                }
+                updateDiskStatus->Record.AddPDisksMetrics()->CopyFrom(m);
                 pdisk.PDiskMetrics.emplace(m);
-                PDisksWithUnreportedMetrics.PushBack(&pdisk);
             }
         }
     }
+
+    if (updateDiskStatus) {
+        SendToController(std::move(updateDiskStatus));
+    }
 }
 
 void TNodeWarden::Handle(TEvBlobStorage::TEvControllerGroupMetricsExchange::TPtr ev) {
@@ -637,7 +648,7 @@ void TNodeWarden::Handle(TEvStatusUpdate::TPtr ev) {
         auto& vdisk = it->second;
         vdisk.Status = msg->Status;
         vdisk.OnlyPhantomsRemain = msg->OnlyPhantomsRemain;
-        SendDiskMetrics(false);
+        VDiskStatusChanged = true;
 
         if (msg->Status == NKikimrBlobStorage::EVDiskStatus::READY && vdisk.WhiteboardVDiskId) {
             Send(WhiteboardId, new NNodeWhiteboard::TEvWhiteboard::TEvVDiskDropDonors(*vdisk.WhiteboardVDiskId,
diff --git a/ydb/core/blobstorage/nodewarden/node_warden_impl.h b/ydb/core/blobstorage/nodewarden/node_warden_impl.h
@@ -582,6 +582,8 @@ namespace NKikimr::NStorage {
         IActor *CreateGroupResolverActor(ui32 groupId);
         void Handle(TEvNodeWardenQueryGroupInfo::TPtr ev);
 
+        bool VDiskStatusChanged = false;
+
         STATEFN(StateOnline) {
             switch (ev->GetTypeRewrite()) {
                 fFunc(TEvBlobStorage::TEvPut::EventType, HandleForwarded);
@@ -663,6 +665,11 @@ namespace NKikimr::NStorage {
                     EnqueuePendingMessage(ev);
                     break;
             }
+
+            if (VDiskStatusChanged) {
+                SendDiskMetrics(false);
+                VDiskStatusChanged = false;
+            }
         }
     };
 
diff --git a/ydb/core/blobstorage/nodewarden/node_warden_pdisk.cpp b/ydb/core/blobstorage/nodewarden/node_warden_pdisk.cpp
@@ -254,7 +254,6 @@ namespace NKikimr::NStorage {
                     StartLocalVDiskActor(value);
                 }
             }
-            SendDiskMetrics(false);
 
             vdisks << "}";
             STLOG(PRI_NOTICE, BS_NODE, NW74, "RestartLocalPDisk has finished",
diff --git a/ydb/core/blobstorage/nodewarden/node_warden_pipe.cpp b/ydb/core/blobstorage/nodewarden/node_warden_pipe.cpp
@@ -18,9 +18,22 @@ void TNodeWarden::EstablishPipe() {
     STLOG(PRI_DEBUG, BS_NODE, NW21, "EstablishPipe", (AvailDomainId, AvailDomainId),
         (PipeClientId, PipeClientId), (ControllerId, controllerId));
 
+    for (auto& [key, pdisk] : LocalPDisks) {
+        if (pdisk.PDiskMetrics) {
+            PDisksWithUnreportedMetrics.PushBack(&pdisk);
+        }
+    }
+    for (auto& [key, vdisk] : LocalVDisks) {
+        vdisk.ScrubCookieForController = 0; // invalidate all pending requests to BS_CONTROLLER
+        if (vdisk.VDiskMetrics) {
+            VDisksWithUnreportedMetrics.PushBack(&vdisk);
+        }
+    }
+
     SendRegisterNode();
     SendInitialGroupRequests();
     SendScrubRequests();
+    SendDiskMetrics(true);
 }
 
 void TNodeWarden::Handle(TEvTabletPipe::TEvClientConnected::TPtr ev) {
@@ -44,17 +57,6 @@ void TNodeWarden::Handle(TEvTabletPipe::TEvClientDestroyed::TPtr ev) {
 }
 
 void TNodeWarden::OnPipeError() {
-    for (auto& [key, pdisk] : LocalPDisks) {
-        if (pdisk.PDiskMetrics) {
-            PDisksWithUnreportedMetrics.PushBack(&pdisk);
-        }
-    }
-    for (auto& [key, vdisk] : LocalVDisks) {
-        vdisk.ScrubCookieForController = 0; // invalidate all pending requests to BS_CONTROLLER
-        if (vdisk.VDiskMetrics) {
-            VDisksWithUnreportedMetrics.PushBack(&vdisk);
-        }
-    }
     for (const auto& [cookie, callback] : ConfigInFlight) {
         callback(nullptr);
     }
diff --git a/ydb/core/blobstorage/nodewarden/node_warden_vdisk.cpp b/ydb/core/blobstorage/nodewarden/node_warden_vdisk.cpp
@@ -49,8 +49,7 @@ namespace NKikimr::NStorage {
         vdisk.ScrubCookieForController = 0; // and from controller too
         vdisk.Status = NKikimrBlobStorage::EVDiskStatus::ERROR;
         vdisk.ShutdownPending = true;
-
-        SendDiskMetrics(false);
+        VDiskStatusChanged = true;
     }
 
     void TNodeWarden::StartLocalVDiskActor(TVDiskRecord& vdisk) {
@@ -239,6 +238,7 @@ namespace NKikimr::NStorage {
         vdisk.Status = NKikimrBlobStorage::EVDiskStatus::INIT_PENDING;
         vdisk.ReportedVDiskStatus.reset();
         vdisk.ScrubCookie = scrubCookie;
+        VDiskStatusChanged = true;
     }
 
     void TNodeWarden::HandleGone(STATEFN_SIG) {
@@ -259,7 +259,6 @@ namespace NKikimr::NStorage {
         for (const auto& vdisk : serviceSet.GetVDisks()) {
             ApplyLocalVDiskInfo(vdisk);
         }
-        SendDiskMetrics(false);
     }
 
     void TNodeWarden::ApplyLocalVDiskInfo(const NKikimrBlobStorage::TNodeWardenServiceSet::TVDisk& vdisk) {
diff --git a/ydb/core/mind/bscontroller/register_node.cpp b/ydb/core/mind/bscontroller/register_node.cpp
@@ -10,9 +10,8 @@ class TBlobStorageController::TTxUpdateNodeDrives
 {
     NKikimrBlobStorage::TEvControllerUpdateNodeDrives Record;
     std::optional<TConfigState> State;
-    std::unique_ptr<TEvBlobStorage::TEvControllerNodeServiceSetUpdate> Result;
 
-    void UpdateDevicesInfo(TConfigState& state, TEvBlobStorage::TEvControllerNodeServiceSetUpdate* result) {
+    void UpdateDevicesInfo(TConfigState& state) {
         auto nodeId = Record.GetNodeId();
 
         auto createLog = [&] () {
@@ -83,9 +82,6 @@ class TBlobStorageController::TTxUpdateNodeDrives
             if (pdiskInfo.LastSeenSerial != serial) {
                 auto *item = getMutableItem();
                 item->LastSeenSerial = serial;
-                if (serial) {
-                    Self->ReadPDisk(pdiskId, *item, result, NKikimrBlobStorage::RESTART);
-                }
             }
 
             return true;
@@ -164,14 +160,12 @@ class TBlobStorageController::TTxUpdateNodeDrives
     bool Execute(TTransactionContext& txc, const TActorContext&) override {
         const TNodeId nodeId = Record.GetNodeId();
 
-        Result = std::make_unique<TEvBlobStorage::TEvControllerNodeServiceSetUpdate>(NKikimrProto::OK, nodeId);
-
         State.emplace(*Self, Self->HostRecords, TActivationContext::Now());
         State->CheckConsistency();
 
         auto updateIsSuccessful = true;
         try {
-            UpdateDevicesInfo(*State, Result.get());
+            UpdateDevicesInfo(*State);
             State->CheckConsistency();
         } catch (const TExError& e) {
             updateIsSuccessful = false;
@@ -181,10 +175,6 @@ class TBlobStorageController::TTxUpdateNodeDrives
                     "Error during UpdateDevicesInfo after receiving TEvControllerRegisterNode", (TExError, e.what()));
         }
 
-        Result->Record.SetInstanceId(Self->InstanceId);
-        Result->Record.SetComprehensive(false);
-        Result->Record.SetAvailDomain(AppData()->DomainsInfo->GetDomain()->DomainUid);
-
         TString error;
         if (!updateIsSuccessful || (State->Changed() && !Self->CommitConfigUpdates(*State, false, false, false, txc, &error))) {
             State->Rollback();
@@ -200,9 +190,6 @@ class TBlobStorageController::TTxUpdateNodeDrives
             State->ApplyConfigUpdates();
             State.reset();
         }
-        if (Result) {
-            Self->SendToWarden(Record.GetNodeId(), std::move(Result), 0);
-        }
     }
 };
 

Original file line number	Diff line number	Diff line change
`@@ -338,7 +338,6 @@ void TNodeWarden::Handle(NPDisk::TEvSlayResult::TPtr ev) {`
`338`	`338`	`SendVDiskReport(vslotId, msg.VDiskId, NKikimrBlobStorage::TEvControllerNodeReport::WIPED);`
`339`	`339`	`TVDiskRecord& vdisk = vdiskIt->second;`
`340`	`340`	`StartLocalVDiskActor(vdisk); // restart actor after successful wiping`
`341`		`- SendDiskMetrics(false);`
`342`	`341`	`}`
`343`	`342`	`break;`
`344`	`343`
`@@ -540,6 +539,8 @@ void TNodeWarden::Handle(TEvBlobStorage::TEvControllerUpdateDiskStatus::TPtr ev)`
`540`	`539`
`541`	`540`	`auto& record = ev->Get()->Record;`
`542`	`541`
	`542`	`+ std::unique_ptr<TEvBlobStorage::TEvControllerUpdateDiskStatus> updateDiskStatus;`
	`543`	`+`
`543`	`544`	`for (const NKikimrBlobStorage::TVDiskMetrics& m : record.GetVDisksMetrics()) {`
`544`	`545`	`Y_ABORT_UNLESS(m.HasVSlotId());`
`545`	`546`	`const TVSlotId vslotId(m.GetVSlotId());`
`@@ -554,8 +555,11 @@ void TNodeWarden::Handle(TEvBlobStorage::TEvControllerUpdateDiskStatus::TPtr ev)`
`554`	`555`	`VDisksWithUnreportedMetrics.PushBack(&vdisk);`
`555`	`556`	`}`
`556`	`557`	`} else {`
	`558`	`+ if (!updateDiskStatus) {`
	`559`	`+ updateDiskStatus.reset(new TEvBlobStorage::TEvControllerUpdateDiskStatus);`
	`560`	`+ }`
	`561`	`+ updateDiskStatus->Record.AddVDisksMetrics()->CopyFrom(m);`
`557`	`562`	`vdisk.VDiskMetrics.emplace(m);`
`558`		`- VDisksWithUnreportedMetrics.PushBack(&vdisk);`
`559`	`563`	`}`
`560`	`564`	`}`
`561`	`565`	`}`
`@@ -571,11 +575,18 @@ void TNodeWarden::Handle(TEvBlobStorage::TEvControllerUpdateDiskStatus::TPtr ev)`
`571`	`575`	`PDisksWithUnreportedMetrics.PushBack(&pdisk);`
`572`	`576`	`}`
`573`	`577`	`} else {`
	`578`	`+ if (!updateDiskStatus) {`
	`579`	`+ updateDiskStatus.reset(new TEvBlobStorage::TEvControllerUpdateDiskStatus);`
	`580`	`+ }`
	`581`	`+ updateDiskStatus->Record.AddPDisksMetrics()->CopyFrom(m);`
`574`	`582`	`pdisk.PDiskMetrics.emplace(m);`
`575`		`- PDisksWithUnreportedMetrics.PushBack(&pdisk);`
`576`	`583`	`}`
`577`	`584`	`}`
`578`	`585`	`}`
	`586`	`+`
	`587`	`+ if (updateDiskStatus) {`
	`588`	`+ SendToController(std::move(updateDiskStatus));`
	`589`	`+ }`
`579`	`590`	`}`
`580`	`591`
`581`	`592`	`void TNodeWarden::Handle(TEvBlobStorage::TEvControllerGroupMetricsExchange::TPtr ev) {`
`@@ -637,7 +648,7 @@ void TNodeWarden::Handle(TEvStatusUpdate::TPtr ev) {`
`637`	`648`	`auto& vdisk = it->second;`
`638`	`649`	`vdisk.Status = msg->Status;`
`639`	`650`	`vdisk.OnlyPhantomsRemain = msg->OnlyPhantomsRemain;`
`640`		`- SendDiskMetrics(false);`
	`651`	`+ VDiskStatusChanged = true;`
`641`	`652`
`642`	`653`	`if (msg->Status == NKikimrBlobStorage::EVDiskStatus::READY && vdisk.WhiteboardVDiskId) {`
`643`	`654`	`Send(WhiteboardId, new NNodeWhiteboard::TEvWhiteboard::TEvVDiskDropDonors(*vdisk.WhiteboardVDiskId,`
Original file line number	Diff line number	Diff line change
`@@ -254,7 +254,6 @@ namespace NKikimr::NStorage {`
`254`	`254`	`StartLocalVDiskActor(value);`
`255`	`255`	`}`
`256`	`256`	`}`
`257`		`- SendDiskMetrics(false);`
`258`	`257`
`259`	`258`	`vdisks << "}";`
`260`	`259`	`STLOG(PRI_NOTICE, BS_NODE, NW74, "RestartLocalPDisk has finished",`
Original file line number	Diff line number	Diff line change
`@@ -49,8 +49,7 @@ namespace NKikimr::NStorage {`
`49`	`49`	`vdisk.ScrubCookieForController = 0; // and from controller too`
`50`	`50`	`vdisk.Status = NKikimrBlobStorage::EVDiskStatus::ERROR;`
`51`	`51`	`vdisk.ShutdownPending = true;`
`52`		`-`
`53`		`- SendDiskMetrics(false);`
	`52`	`+ VDiskStatusChanged = true;`
`54`	`53`	`}`
`55`	`54`
`56`	`55`	`void TNodeWarden::StartLocalVDiskActor(TVDiskRecord& vdisk) {`
`@@ -239,6 +238,7 @@ namespace NKikimr::NStorage {`
`239`	`238`	`vdisk.Status = NKikimrBlobStorage::EVDiskStatus::INIT_PENDING;`
`240`	`239`	`vdisk.ReportedVDiskStatus.reset();`
`241`	`240`	`vdisk.ScrubCookie = scrubCookie;`
	`241`	`+ VDiskStatusChanged = true;`
`242`	`242`	`}`
`243`	`243`
`244`	`244`	`void TNodeWarden::HandleGone(STATEFN_SIG) {`
`@@ -259,7 +259,6 @@ namespace NKikimr::NStorage {`
`259`	`259`	`for (const auto& vdisk : serviceSet.GetVDisks()) {`
`260`	`260`	`ApplyLocalVDiskInfo(vdisk);`
`261`	`261`	`}`
`262`		`- SendDiskMetrics(false);`
`263`	`262`	`}`
`264`	`263`
`265`	`264`	`void TNodeWarden::ApplyLocalVDiskInfo(const NKikimrBlobStorage::TNodeWardenServiceSet::TVDisk& vdisk) {`