NBS-4827 Add DiskId to partition statistics #601 #600 (#627)

* Fix poison pill helper (#600) * NBS-4827 Add DiskId to partition statistics (#601) * NBS-4827 Add DiskId to partition statistics * Fix review issues
ydb-platform · Mar 4, 2024 · 78fdc80 · 78fdc80
1 parent 2f2dee3
commit 78fdc80
Show file tree

Hide file tree

Showing 12 changed files with 75 additions and 39 deletions.
diff --git a/cloud/blockstore/libs/storage/api/volume.h b/cloud/blockstore/libs/storage/api/volume.h
@@ -133,14 +133,20 @@ struct TEvVolume
     struct TDiskRegistryBasedPartitionCounters
     {
         TPartitionDiskCountersPtr DiskCounters;
+        TString DiskId;
         ui64 NetworkBytes = 0;
         TDuration CpuUsage;
 
         TDiskRegistryBasedPartitionCounters(
-                TPartitionDiskCountersPtr diskCounters)
+                TPartitionDiskCountersPtr diskCounters,
+                TString diskId,
+                ui64 networkBytes,
+                TDuration cpuUsage)
             : DiskCounters(std::move(diskCounters))
-        {
-        }
+            , DiskId(std::move(diskId))
+            , NetworkBytes(networkBytes)
+            , CpuUsage(cpuUsage)
+        {}
     };
 
     //

diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_mirror_actor_stats.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_mirror_actor_stats.cpp
@@ -62,12 +62,14 @@ void TMirrorPartitionActor::SendStats(const TActorContext& ctx)
         }
     }
 
-    auto request = std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>(
-        MakeIntrusive<TCallContext>(),
-        std::move(stats));
+    auto request =
+        std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>(
+            MakeIntrusive<TCallContext>(),
+            std::move(stats),
+            DiskId,
+            NetworkBytes,
+            CpuUsage);
 
-    request->NetworkBytes = NetworkBytes;
-    request->CpuUsage = CpuUsage;
     NetworkBytes = 0;
     CpuUsage = {};
 

diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_mirror_resync_actor_stats.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_mirror_resync_actor_stats.cpp
@@ -39,12 +39,13 @@ void TMirrorPartitionResyncActor::SendStats(const TActorContext& ctx)
         stats->AggregateWith(*MirrorCounters);
     }
 
-    auto request = std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>(
-        MakeIntrusive<TCallContext>(),
-        std::move(stats));
-
-    request->NetworkBytes = NetworkBytes;
-    request->CpuUsage = CpuUsage;
+    auto request =
+        std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>(
+            MakeIntrusive<TCallContext>(),
+            std::move(stats),
+            PartConfig->GetName(),
+            NetworkBytes,
+            CpuUsage);
 
     NCloud::Send(ctx, StatActorId, std::move(request));
 }

diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_stats.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_stats.cpp
@@ -40,12 +40,14 @@ void TNonreplicatedPartitionActor::SendStats(const TActorContext& ctx)
         && IOErrorCooldownPassed(ctx.Now()));
     PartCounters->Simple.HasBrokenDeviceSilent.Set(HasBrokenDevice);
 
-    auto request = std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>(
-        MakeIntrusive<TCallContext>(),
-        std::move(PartCounters));
+    auto request =
+        std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>(
+            MakeIntrusive<TCallContext>(),
+            std::move(PartCounters),
+            PartConfig->GetName(),
+            NetworkBytes,
+            CpuUsage);
 
-    request->NetworkBytes = NetworkBytes;
-    request->CpuUsage = CpuUsage;
     NetworkBytes = 0;
     CpuUsage = {};
 

diff --git a/...d/blockstore/libs/storage/partition_nonrepl/part_nonrepl_migration_common_actor_stats.cpp b/...d/blockstore/libs/storage/partition_nonrepl/part_nonrepl_migration_common_actor_stats.cpp
@@ -57,12 +57,14 @@ void TNonreplicatedPartitionMigrationCommonActor::SendStats(
             DstCounters->Simple.IORequestsInFlight.Value);
     }
 
-    auto request = std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>(
-        MakeIntrusive<TCallContext>(),
-        std::move(stats));
+    auto request =
+        std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>(
+            MakeIntrusive<TCallContext>(),
+            std::move(stats),
+            DiskId,
+            NetworkBytes,
+            CpuUsage);
 
-    request->NetworkBytes = NetworkBytes;
-    request->CpuUsage = CpuUsage;
     NetworkBytes = 0;
     CpuUsage = {};
 

diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_rdma_actor_stats.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_rdma_actor_stats.cpp
@@ -29,12 +29,14 @@ void TNonreplicatedPartitionRdmaActor::SendStats(const TActorContext& ctx)
         PartConfig->GetBlockCount() * PartConfig->GetBlockSize()
     );
 
-    auto request = std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>(
-        MakeIntrusive<TCallContext>(),
-        std::move(PartCounters));
+    auto request =
+        std::make_unique<TEvVolume::TEvDiskRegistryBasedPartitionCounters>(
+            MakeIntrusive<TCallContext>(),
+            std::move(PartCounters),
+            PartConfig->GetName(),
+            NetworkBytes,
+            CpuUsage);
 
-    request->NetworkBytes = NetworkBytes;
-    request->CpuUsage = CpuUsage;
     NetworkBytes = 0;
     CpuUsage = {};
 

diff --git a/cloud/blockstore/libs/storage/protos/disk.proto b/cloud/blockstore/libs/storage/protos/disk.proto
@@ -742,6 +742,7 @@ message TAcquireDiskResponse
     NCloud.NProto.TError Error = 1;
 
     // List of devices that make up the disk.
+    // Note. Contains only the devices located at available agents.
     repeated TDeviceConfig Devices = 2;
 
     // Migration configuration.

diff --git a/cloud/blockstore/libs/storage/volume/volume_actor_checkpoint.cpp b/cloud/blockstore/libs/storage/volume/volume_actor_checkpoint.cpp
@@ -1428,7 +1428,8 @@ void TVolumeActor::CompleteUpdateShadowDiskState(
         ctx,
         *args.RequestInfo,
         std::make_unique<TEvVolumePrivate::TEvUpdateShadowDiskStateResponse>(
-            newState));
+            newState,
+            args.ProcessedBlockCount));
 }
 
 void TVolumeActor::HandleUpdateShadowDiskState(
@@ -1448,7 +1449,9 @@ void TVolumeActor::HandleUpdateShadowDiskState(
     auto reply = [&](EShadowDiskState newState)
     {
         auto response = std::make_unique<
-            TEvVolumePrivate::TEvUpdateShadowDiskStateResponse>(newState);
+            TEvVolumePrivate::TEvUpdateShadowDiskStateResponse>(
+            newState,
+            msg->ProcessedBlockCount);
         NCloud::Reply(ctx, *ev, std::move(response));
     };
 

diff --git a/cloud/blockstore/libs/storage/volume/volume_actor_stats.cpp b/cloud/blockstore/libs/storage/volume/volume_actor_stats.cpp
@@ -127,10 +127,16 @@ void TVolumeActor::HandleDiskRegistryBasedPartCounters(
         msg->CallContext
     );
 
-    if (State->GetDiskRegistryBasedPartitionActor() != ev->Sender) {
-        LOG_INFO(ctx, TBlockStoreComponents::VOLUME,
-            "Partition %s for disk %s counters not found",
+    const bool doesPartitionBelongToDisk =
+        State->GetDiskRegistryBasedPartitionActor() == ev->Sender ||
+        State->GetDiskId() == msg->DiskId;
+    if (!doesPartitionBelongToDisk) {
+        LOG_INFO(
+            ctx,
+            TBlockStoreComponents::VOLUME,
+            "Counters from partition %s (%s) do not belong to disk %s",
             ToString(ev->Sender).c_str(),
+            msg->DiskId.Quote().c_str(),
             State->GetDiskId().Quote().c_str());
         return;
     }

diff --git a/cloud/blockstore/libs/storage/volume/volume_events_private.h b/cloud/blockstore/libs/storage/volume/volume_events_private.h
@@ -247,11 +247,15 @@ struct TEvVolumePrivate
     struct TUpdateShadowDiskStateResponse
     {
         EShadowDiskState NewState = EShadowDiskState::None;
+        ui64 ProcessedBlockCount = 0;
 
-        TUpdateShadowDiskStateResponse() {}
+        TUpdateShadowDiskStateResponse() = default;
 
-        TUpdateShadowDiskStateResponse(EShadowDiskState newState)
+        TUpdateShadowDiskStateResponse(
+                EShadowDiskState newState,
+                ui64 processedBlockCount)
             : NewState(newState)
+            , ProcessedBlockCount(processedBlockCount)
         {}
     };
 

diff --git a/cloud/storage/core/libs/actors/poison_pill_helper.cpp b/cloud/storage/core/libs/actors/poison_pill_helper.cpp
@@ -63,9 +63,7 @@ void TPoisonPillHelper::KillActors(const TActorContext& ctx)
 
 void TPoisonPillHelper::ReplyAndDie(const TActorContext& ctx)
 {
-    Y_DEBUG_ABORT_UNLESS(Poisoner);
-
-    if (!OwnedActors.empty()) {
+    if (!Poisoner || !OwnedActors.empty()) {
         return;
     }
 

diff --git a/cloud/storage/core/libs/actors/poison_pill_helper_ut.cpp b/cloud/storage/core/libs/actors/poison_pill_helper_ut.cpp
@@ -125,11 +125,20 @@ class TParentActor
     {
         Y_UNUSED(ev);
 
+        {   // We give ownership and take it away immediately.
+            auto childId = ctx.Register(new TChildActor());
+            PoisonPillHelper.TakeOwnership(ctx, childId);
+            ctx.Send(childId, std::make_unique<TEvents::TEvPoisonPill>());
+            PoisonPillHelper.ReleaseOwnership(ctx, childId);
+        }
+
+        // Give ownership for long time.
         for (ui32 i = 0; i < ChildCount; ++i) {
             PoisonPillHelper.TakeOwnership(
                 ctx,
                 ctx.Register(new TChildActor()));
         }
+
     }
 };