Fix DiskRegistry-based devices request tracking (#4575)

drbasic · web-flow · commit ffd016bb6ab3 · 2025-10-29T17:30:14.000+07:00
continue #4381 1. Fixed operationId reusing 2. Fixed missed OnRequestFinished() when operation finished with error
diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor.cpp
@@ -661,14 +661,15 @@ bool TNonreplicatedPartitionActor::HandleRequests(STFUNC_SIG)
 ui64 TNonreplicatedPartitionActor::GenerateOperationId(
     size_t deviceRequestsCount)
 {
+    static std::atomic<ui64> DeviceOperationIdGenerator = 1;
+
+    const ui64 val = DeviceOperationIdGenerator.fetch_add(deviceRequestsCount);
     const ui32 trackingFreq = Config->GetDeviceOperationTrackingFrequency();
-    const ui64 operationId =
-        (trackingFreq > 0 && DeviceOperationIdGenerator % trackingFreq == 0)
-            ? DeviceOperationIdGenerator
-            : 0;
+    if (trackingFreq > 0 && val % trackingFreq == 0) {
+        return val;
+    }
 
-    DeviceOperationIdGenerator += deviceRequestsCount;
-    return operationId;
+    return 0;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor.h b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor.h
@@ -89,8 +89,6 @@ class TNonreplicatedPartitionActor final
         GetCycleCount(),
         TLogTitle::TPartitionNonrepl{.DiskId = PartConfig->GetName()}};
 
-    ui64 DeviceOperationIdGenerator = 1;
-
 public:
     TNonreplicatedPartitionActor(
         TStorageConfigPtr config,
diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_base_request.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_base_request.cpp
@@ -120,7 +120,7 @@ void TDiskAgentBaseRequestActor::OnRequestStarted(
     const NActors::TActorContext& ctx,
     const TString& deviceUUID,
     TDeviceOperationTracker::ERequestType requestType,
-    ui32 cookie)
+    size_t requestIndex)
 {
     if (!DeviceOperationId) {
         return;
@@ -130,24 +130,24 @@ void TDiskAgentBaseRequestActor::OnRequestStarted(
         TEvVolumePrivate::TEvDiskRegistryDeviceOperationStarted>(
         deviceUUID,
         requestType,
-        DeviceOperationId + cookie);
+        DeviceOperationId + requestIndex);
 
-    ctx.Send(VolumeActorId, startEvent.release());
+    ctx.Send(VolumeActorId, std::move(startEvent));
 }
 
 void TDiskAgentBaseRequestActor::OnRequestFinished(
     const NActors::TActorContext& ctx,
-    ui32 cookie)
+    size_t requestIndex)
 {
     if (!DeviceOperationId) {
         return;
     }
 
     auto finishEvent = std::make_unique<
         TEvVolumePrivate::TEvDiskRegistryDeviceOperationFinished>(
-        DeviceOperationId + cookie);
+        DeviceOperationId + requestIndex);
 
-    ctx.Send(VolumeActorId, finishEvent.release());
+    ctx.Send(VolumeActorId, std::move(finishEvent));
 }
 
 void TDiskAgentBaseRequestActor::HandleCancelRequest(
@@ -156,6 +156,12 @@ void TDiskAgentBaseRequestActor::HandleCancelRequest(
 {
     const auto* msg = ev->Get();
 
+    for (size_t requestIndex = 0; requestIndex < DeviceRequests.size();
+         ++requestIndex)
+    {
+        OnRequestFinished(ctx, requestIndex);
+    }
+
     TVector<TString> devices;
     for (const auto& request: DeviceRequests) {
         devices.push_back(request.Device.GetDeviceUUID());
diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_base_request.h b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_base_request.h
@@ -84,9 +84,11 @@ class TDiskAgentBaseRequestActor
         const NActors::TActorContext& ctx,
         const TString& deviceUUID,
         TDeviceOperationTracker::ERequestType requestType,
-        ui32 cookie);
+        size_t requestIndex);
 
-    void OnRequestFinished(const NActors::TActorContext& ctx, ui32 cookie);
+    void OnRequestFinished(
+        const NActors::TActorContext& ctx,
+        size_t requestIndex);
 
 private:
     void StateWork(TAutoPtr<NActors::IEventHandle>& ev);
diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_checksumblocks.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_checksumblocks.cpp
@@ -146,6 +146,8 @@ void TDiskAgentChecksumActor::HandleChecksumDeviceBlocksUndelivery(
     const TEvDiskAgent::TEvChecksumDeviceBlocksRequest::TPtr& ev,
     const TActorContext& ctx)
 {
+    OnRequestFinished(ctx, ev->Cookie);
+
     const auto& device = DeviceRequests[ev->Cookie].Device;
     LOG_WARN(
         ctx,
@@ -164,6 +166,8 @@ void TDiskAgentChecksumActor::HandleChecksumDeviceBlocksResponse(
 {
     auto* msg = ev->Get();
 
+    OnRequestFinished(ctx, ev->Cookie);
+
     if (HasError(msg->GetError())) {
         HandleError(ctx, msg->GetError(), EStatus::Fail);
         return;
@@ -174,8 +178,6 @@ void TDiskAgentChecksumActor::HandleChecksumDeviceBlocksResponse(
     const auto rangeSize = deviceRequest.DeviceBlockRange.Size() * PartConfig->GetBlockSize();
     Checksums[rangeStart] = {msg->Record.GetChecksum(), rangeSize};
 
-    OnRequestFinished(ctx, ev->Cookie);
-
     if (++RequestsCompleted < DeviceRequests.size()) {
         return;
     }
diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_readblocks.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_readblocks.cpp
@@ -168,6 +168,8 @@ void TDiskAgentReadActor::HandleReadDeviceBlocksUndelivery(
     const TEvDiskAgent::TEvReadDeviceBlocksRequest::TPtr& ev,
     const TActorContext& ctx)
 {
+    OnRequestFinished(ctx, ev->Cookie);
+
     const auto& device = DeviceRequests[ev->Cookie].Device;
     LOG_WARN(
         ctx,
@@ -186,6 +188,8 @@ void TDiskAgentReadActor::HandleReadDeviceBlocksResponse(
 {
     auto* msg = ev->Get();
 
+    OnRequestFinished(ctx, ev->Cookie);
+
     if (HasError(msg->GetError())) {
         HandleError(ctx, msg->GetError(), EStatus::Fail);
         return;
@@ -212,8 +216,6 @@ void TDiskAgentReadActor::HandleReadDeviceBlocksResponse(
         }
     }
 
-    OnRequestFinished(ctx, ev->Cookie);
-
     if (++RequestsCompleted < DeviceRequests.size()) {
         return;
     }
diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_readblocks_local.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_readblocks_local.cpp
@@ -170,6 +170,8 @@ void TDiskAgentReadLocalActor::HandleReadDeviceBlocksUndelivery(
     const TEvDiskAgent::TEvReadDeviceBlocksRequest::TPtr& ev,
     const TActorContext& ctx)
 {
+    OnRequestFinished(ctx, ev->Cookie);
+
     const auto& device = DeviceRequests[ev->Cookie].Device;
     LOG_WARN(
         ctx,
@@ -188,6 +190,8 @@ void TDiskAgentReadLocalActor::HandleReadDeviceBlocksResponse(
 {
     auto* msg = ev->Get();
 
+    OnRequestFinished(ctx, ev->Cookie);
+
     if (HasError(msg->GetError())) {
         HandleError(ctx, msg->GetError(), EStatus::Fail);
         return;
@@ -225,8 +229,6 @@ void TDiskAgentReadLocalActor::HandleReadDeviceBlocksResponse(
         }
     }
 
-    OnRequestFinished(ctx, ev->Cookie);
-
     if (++RequestsCompleted < DeviceRequests.size()) {
         return;
     }
diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_writeblocks.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_writeblocks.cpp
@@ -179,6 +179,8 @@ void TDiskAgentWriteActor::HandleWriteDeviceBlocksUndelivery(
     const TEvDiskAgent::TEvWriteDeviceBlocksRequest::TPtr& ev,
     const TActorContext& ctx)
 {
+    OnRequestFinished(ctx, ev->Cookie);
+
     const auto& device = DeviceRequests[ev->Cookie].Device;
     LOG_WARN(
         ctx,
@@ -197,13 +199,13 @@ void TDiskAgentWriteActor::HandleWriteDeviceBlocksResponse(
 {
     auto* msg = ev->Get();
 
+    OnRequestFinished(ctx, ev->Cookie);
+
     if (HasError(msg->GetError())) {
         HandleError(ctx, msg->GetError(), EStatus::Fail);
         return;
     }
 
-    OnRequestFinished(ctx, ev->Cookie);
-
     if (++RequestsCompleted < DeviceRequests.size()) {
         return;
     }
diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_writeblocks_multi_agent.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_writeblocks_multi_agent.cpp
@@ -182,6 +182,8 @@ void TDiskAgentMultiWriteActor::HandleWriteDeviceBlocksUndelivery(
 {
     Y_UNUSED(ev);
 
+    OnRequestFinished(ctx, ev->Cookie);
+
     LOG_WARN(
         ctx,
         TBlockStoreComponents::PARTITION_WORKER,
@@ -199,6 +201,8 @@ void TDiskAgentMultiWriteActor::HandleWriteDeviceBlocksResponse(
 {
     const auto* msg = ev->Get();
 
+    OnRequestFinished(ctx, ev->Cookie);
+
     auto replyInconsistentError = [&]()
     {
         auto response = std::make_unique<
@@ -240,8 +244,6 @@ void TDiskAgentMultiWriteActor::HandleWriteDeviceBlocksResponse(
         return;
     }
 
-    OnRequestFinished(ctx, 0);
-
     Y_DEBUG_ABORT_UNLESS(error.GetCode() == S_OK);
     Done(ctx, MakeResponse(std::move(error)), EStatus::Success);
 }
diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_zeroblocks.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_actor_zeroblocks.cpp
@@ -144,6 +144,8 @@ void TDiskAgentZeroActor::HandleZeroDeviceBlocksUndelivery(
     const TEvDiskAgent::TEvZeroDeviceBlocksRequest::TPtr& ev,
     const TActorContext& ctx)
 {
+    OnRequestFinished(ctx, ev->Cookie);
+
     const auto& device = DeviceRequests[ev->Cookie].Device;
     LOG_WARN(
         ctx,
@@ -162,13 +164,13 @@ void TDiskAgentZeroActor::HandleZeroDeviceBlocksResponse(
 {
     auto* msg = ev->Get();
 
+    OnRequestFinished(ctx, ev->Cookie);
+
     if (HasError(msg->GetError())) {
         HandleError(ctx, msg->GetError(), EStatus::Fail);
         return;
     }
 
-    OnRequestFinished(ctx, ev->Cookie);
-
     if (++RequestsCompleted < DeviceRequests.size()) {
         return;
     }
diff --git a/cloud/blockstore/libs/storage/volume/volume_ut.cpp b/cloud/blockstore/libs/storage/volume/volume_ut.cpp

Original file line number	Diff line number	Diff line change
`@@ -168,6 +168,8 @@ void TDiskAgentReadActor::HandleReadDeviceBlocksUndelivery(`
`168`	`168`	`const TEvDiskAgent::TEvReadDeviceBlocksRequest::TPtr& ev,`
`169`	`169`	`const TActorContext& ctx)`
`170`	`170`	`{`
	`171`	`+ OnRequestFinished(ctx, ev->Cookie);`
	`172`	`+`
`171`	`173`	`const auto& device = DeviceRequests[ev->Cookie].Device;`
`172`	`174`	`LOG_WARN(`
`173`	`175`	`ctx,`
`@@ -186,6 +188,8 @@ void TDiskAgentReadActor::HandleReadDeviceBlocksResponse(`
`186`	`188`	`{`
`187`	`189`	`auto* msg = ev->Get();`
`188`	`190`
	`191`	`+ OnRequestFinished(ctx, ev->Cookie);`
	`192`	`+`
`189`	`193`	`if (HasError(msg->GetError())) {`
`190`	`194`	`HandleError(ctx, msg->GetError(), EStatus::Fail);`
`191`	`195`	`return;`
`@@ -212,8 +216,6 @@ void TDiskAgentReadActor::HandleReadDeviceBlocksResponse(`
`212`	`216`	`}`
`213`	`217`	`}`
`214`	`218`
`215`		`- OnRequestFinished(ctx, ev->Cookie);`
`216`		`-`
`217`	`219`	`if (++RequestsCompleted < DeviceRequests.size()) {`
`218`	`220`	`return;`
`219`	`221`	`}`
Original file line number	Diff line number	Diff line change
`@@ -170,6 +170,8 @@ void TDiskAgentReadLocalActor::HandleReadDeviceBlocksUndelivery(`
`170`	`170`	`const TEvDiskAgent::TEvReadDeviceBlocksRequest::TPtr& ev,`
`171`	`171`	`const TActorContext& ctx)`
`172`	`172`	`{`
	`173`	`+ OnRequestFinished(ctx, ev->Cookie);`
	`174`	`+`
`173`	`175`	`const auto& device = DeviceRequests[ev->Cookie].Device;`
`174`	`176`	`LOG_WARN(`
`175`	`177`	`ctx,`
`@@ -188,6 +190,8 @@ void TDiskAgentReadLocalActor::HandleReadDeviceBlocksResponse(`
`188`	`190`	`{`
`189`	`191`	`auto* msg = ev->Get();`
`190`	`192`
	`193`	`+ OnRequestFinished(ctx, ev->Cookie);`
	`194`	`+`
`191`	`195`	`if (HasError(msg->GetError())) {`
`192`	`196`	`HandleError(ctx, msg->GetError(), EStatus::Fail);`
`193`	`197`	`return;`
`@@ -225,8 +229,6 @@ void TDiskAgentReadLocalActor::HandleReadDeviceBlocksResponse(`
`225`	`229`	`}`
`226`	`230`	`}`
`227`	`231`
`228`		`- OnRequestFinished(ctx, ev->Cookie);`
`229`		`-`
`230`	`232`	`if (++RequestsCompleted < DeviceRequests.size()) {`
`231`	`233`	`return;`
`232`	`234`	`}`
Original file line number	Diff line number	Diff line change
`@@ -179,6 +179,8 @@ void TDiskAgentWriteActor::HandleWriteDeviceBlocksUndelivery(`
`179`	`179`	`const TEvDiskAgent::TEvWriteDeviceBlocksRequest::TPtr& ev,`
`180`	`180`	`const TActorContext& ctx)`
`181`	`181`	`{`
	`182`	`+ OnRequestFinished(ctx, ev->Cookie);`
	`183`	`+`
`182`	`184`	`const auto& device = DeviceRequests[ev->Cookie].Device;`
`183`	`185`	`LOG_WARN(`
`184`	`186`	`ctx,`
`@@ -197,13 +199,13 @@ void TDiskAgentWriteActor::HandleWriteDeviceBlocksResponse(`
`197`	`199`	`{`
`198`	`200`	`auto* msg = ev->Get();`
`199`	`201`
	`202`	`+ OnRequestFinished(ctx, ev->Cookie);`
	`203`	`+`
`200`	`204`	`if (HasError(msg->GetError())) {`
`201`	`205`	`HandleError(ctx, msg->GetError(), EStatus::Fail);`
`202`	`206`	`return;`
`203`	`207`	`}`
`204`	`208`
`205`		`- OnRequestFinished(ctx, ev->Cookie);`
`206`		`-`
`207`	`209`	`if (++RequestsCompleted < DeviceRequests.size()) {`
`208`	`210`	`return;`
`209`	`211`	`}`
Original file line number	Diff line number	Diff line change
`@@ -182,6 +182,8 @@ void TDiskAgentMultiWriteActor::HandleWriteDeviceBlocksUndelivery(`
`182`	`182`	`{`
`183`	`183`	`Y_UNUSED(ev);`
`184`	`184`
	`185`	`+ OnRequestFinished(ctx, ev->Cookie);`
	`186`	`+`
`185`	`187`	`LOG_WARN(`
`186`	`188`	`ctx,`
`187`	`189`	`TBlockStoreComponents::PARTITION_WORKER,`
`@@ -199,6 +201,8 @@ void TDiskAgentMultiWriteActor::HandleWriteDeviceBlocksResponse(`
`199`	`201`	`{`
`200`	`202`	`const auto* msg = ev->Get();`
`201`	`203`
	`204`	`+ OnRequestFinished(ctx, ev->Cookie);`
	`205`	`+`
`202`	`206`	`auto replyInconsistentError = [&]()`
`203`	`207`	`{`
`204`	`208`	`auto response = std::make_unique<`
`@@ -240,8 +244,6 @@ void TDiskAgentMultiWriteActor::HandleWriteDeviceBlocksResponse(`
`240`	`244`	`return;`
`241`	`245`	`}`
`242`	`246`
`243`		`- OnRequestFinished(ctx, 0);`
`244`		`-`
`245`	`247`	`Y_DEBUG_ABORT_UNLESS(error.GetCode() == S_OK);`
`246`	`248`	`Done(ctx, MakeResponse(std::move(error)), EStatus::Success);`
`247`	`249`	`}`
Original file line number	Diff line number	Diff line change
`@@ -144,6 +144,8 @@ void TDiskAgentZeroActor::HandleZeroDeviceBlocksUndelivery(`
`144`	`144`	`const TEvDiskAgent::TEvZeroDeviceBlocksRequest::TPtr& ev,`
`145`	`145`	`const TActorContext& ctx)`
`146`	`146`	`{`
	`147`	`+ OnRequestFinished(ctx, ev->Cookie);`
	`148`	`+`
`147`	`149`	`const auto& device = DeviceRequests[ev->Cookie].Device;`
`148`	`150`	`LOG_WARN(`
`149`	`151`	`ctx,`
`@@ -162,13 +164,13 @@ void TDiskAgentZeroActor::HandleZeroDeviceBlocksResponse(`
`162`	`164`	`{`
`163`	`165`	`auto* msg = ev->Get();`
`164`	`166`
	`167`	`+ OnRequestFinished(ctx, ev->Cookie);`
	`168`	`+`
`165`	`169`	`if (HasError(msg->GetError())) {`
`166`	`170`	`HandleError(ctx, msg->GetError(), EStatus::Fail);`
`167`	`171`	`return;`
`168`	`172`	`}`
`169`	`173`
`170`		`- OnRequestFinished(ctx, ev->Cookie);`
`171`		`-`
`172`	`174`	`if (++RequestsCompleted < DeviceRequests.size()) {`
`173`	`175`	`return;`
`174`	`176`	`}`