Fix cost calculation for Patch events, add separate VDisk internal cost counter, KIKIMR-17759

Fix cost calculation for Patch events, add separate VDisk internal cost counter, KIKIMR-17759

4 files changed, 109 insertions, 15 deletions

diff --git a/ydb/core/blobstorage/ut_blobstorage/monitoring.cpp b/ydb/core/blobstorage/ut_blobstorage/monitoring.cpp
index e96a088a0e..c872b87c48 100644
--- a/ydb/core/blobstorage/ut_blobstorage/monitoring.cpp
+++ b/ydb/core/blobstorage/ut_blobstorage/monitoring.cpp
@@ -1,7 +1,7 @@
 #include <ydb/core/blobstorage/ut_blobstorage/lib/env.h>
 #include <ydb/core/blobstorage/groupinfo/blobstorage_groupinfo.h>
 
-constexpr bool VERBOSE = true;
+constexpr bool VERBOSE = false;
 
 TString MakeData(ui32 dataSize) {
     TString data(dataSize, '\0');
@@ -88,8 +88,6 @@ void Test(const TBlobStorageGroupInfo::TTopology& topology, TInflightActor* acto
 
     ui64 dsproxyCost = 0;
     ui64 vdiskCost = 0;
-    ui64 hugePuts = 0;
-    ui64 logPuts = 0;
 
     auto* appData = env.Runtime->GetAppData();
     Y_VERIFY(appData);
@@ -114,9 +112,7 @@ void Test(const TBlobStorageGroupInfo::TTopology& topology, TInflightActor* acto
                 GetSubgroup("subsystem", "request")->
                 GetSubgroup("storagePool", env.StoragePoolName)->
                 GetCounter("DSProxyDiskCostNs")->Val();
-        vdiskCost = vdisksTotal("outofspace", "EstimatedDiskTimeConsumptionNs");
-        logPuts = vdisksTotal("skeletonfront", "SkeletonFront/LogPuts/CostProcessed");
-        hugePuts = vdisksTotal("skeletonfront", "SkeletonFront/HugePutsForeground/CostProcessed");
+        vdiskCost = vdisksTotal("cost", "VDiskUserCostNs");
     };
 
     updateCounters();
@@ -133,7 +129,7 @@ void Test(const TBlobStorageGroupInfo::TTopology& topology, TInflightActor* acto
     i64 diff = (i64)dsproxyCost - vdiskCost;
     str << "OKs# " << actor->OKs << ", Fails# " << actor->Fails << ", Cost on dsproxy# "
             << dsproxyCost << ", Cost on vdisks# " << vdiskCost << ", proportion# " << proportion
-            << " diff# " << diff << " hugePuts# " << hugePuts << " logPuts# " << logPuts;
+            << " diff# " << diff;
 
     if constexpr(VERBOSE) {
         Cerr << str.Str() << Endl;
@@ -234,6 +230,62 @@ private:
     ui32 DataSize;
 };
 
+class TInflightActorPatch : public TInflightActor {
+public:
+    TInflightActorPatch(ui32 requests, ui32 inflight, ui32 dataSize = 1024)
+        : TInflightActor(requests, inflight)
+        , DataSize(dataSize)
+    {}
+
+    STRICT_STFUNC(StateWork,
+        hFunc(TEvBlobStorage::TEvPatchResult, Handle);
+        hFunc(TEvBlobStorage::TEvPutResult, Handle);
+    )
+
+    virtual void BootstrapImpl(const TActorContext&/* ctx*/) override {
+        TString data = MakeData(DataSize);
+        for (ui32 i = 0; i < RequestInflight; ++i) {
+            TLogoBlobID blobId(1, 1, 1, 10, DataSize, 1 + i);
+            auto ev = new TEvBlobStorage::TEvPut(blobId, data, TInstant::Max());
+            SendToBSProxy(SelfId(), GroupId, ev, 0);
+        }
+        Become(&TInflightActorPatch::StateWork);
+    }
+
+protected:
+    virtual void SendRequest() override {
+        TLogoBlobID oldId = Blobs.front();
+        Blobs.pop_front();
+        TLogoBlobID newId(1, 1, oldId.Step() + 1, 10, DataSize, oldId.Cookie());
+        Y_VERIFY(TEvBlobStorage::TEvPatch::GetBlobIdWithSamePlacement(oldId, &newId, BlobIdMask, GroupId, GroupId));
+        TArrayHolder<TEvBlobStorage::TEvPatch::TDiff> diffs(new TEvBlobStorage::TEvPatch::TDiff[1]);
+        char c = 'a' + RequestCount % 26;
+        diffs[0].Set(TString(DataSize, c), 0);
+        auto ev = new TEvBlobStorage::TEvPatch(GroupId, oldId, newId, BlobIdMask, std::move(diffs), 1, TInstant::Max());
+        SendToBSProxy(SelfId(), GroupId, ev, 0);
+    }
+
+
+    void Handle(TEvBlobStorage::TEvPatchResult::TPtr res) {
+        Blobs.push_back(res->Get()->Id);
+        HandleReply(res->Get()->Status);
+    }
+
+    void Handle(TEvBlobStorage::TEvPutResult::TPtr res) {
+        Blobs.push_back(res->Get()->Id);
+        if (++BlobsWritten == RequestInflight) {
+            SendRequests();
+        }
+    }
+
+protected:
+    std::deque<TLogoBlobID> Blobs;
+    ui32 BlobIdMask = TLogoBlobID::MaxCookie & 0xfffff000;
+    ui32 BlobsWritten = 0;
+    std::string Data;
+    ui32 DataSize;
+};
+
 Y_UNIT_TEST_SUITE(CostMetricsPutMirror3dc) {
     MAKE_TEST_W_DATASIZE(Mirror3dc, Put, 1, 1, 1000);
     MAKE_TEST_W_DATASIZE(Mirror3dc, Put, 10, 1, 1000);
@@ -292,3 +344,23 @@ Y_UNIT_TEST_SUITE(CostMetricsGetHugeMirror3dc) {
     MAKE_TEST_W_DATASIZE(Mirror3dc, Get, 100, 10, 2000000);
     MAKE_TEST_W_DATASIZE(Mirror3dc, Get, 10000, 100, 2000000);
 }
+
+Y_UNIT_TEST_SUITE(CostMetricsPatchMirror3dc) {
+    MAKE_TEST_W_DATASIZE(Mirror3dc, Patch, 1, 1, 1000);
+    MAKE_TEST_W_DATASIZE(Mirror3dc, Patch, 10, 1, 1000);
+    MAKE_TEST_W_DATASIZE(Mirror3dc, Patch, 100, 1, 1000);
+    MAKE_TEST_W_DATASIZE(Mirror3dc, Patch, 2, 2, 1000);
+    MAKE_TEST_W_DATASIZE(Mirror3dc, Patch, 10, 10, 1000);
+    MAKE_TEST_W_DATASIZE(Mirror3dc, Patch, 100, 10, 1000);
+    MAKE_TEST_W_DATASIZE(Mirror3dc, Patch, 10000, 100, 1000);
+}
+
+Y_UNIT_TEST_SUITE(CostMetricsPatchBlock4Plus2) {
+    MAKE_TEST_W_DATASIZE(4Plus2Block, Patch, 1, 1, 1000);
+    MAKE_TEST_W_DATASIZE(4Plus2Block, Patch, 10, 1, 1000);
+    MAKE_TEST_W_DATASIZE(4Plus2Block, Patch, 100, 1, 1000);
+    MAKE_TEST_W_DATASIZE(4Plus2Block, Patch, 2, 2, 1000);
+    MAKE_TEST_W_DATASIZE(4Plus2Block, Patch, 10, 10, 1000);
+    MAKE_TEST_W_DATASIZE(4Plus2Block, Patch, 100, 10, 1000);
+    MAKE_TEST_W_DATASIZE(4Plus2Block, Patch, 10000, 100, 1000);
+}
diff --git a/ydb/core/blobstorage/vdisk/common/vdisk_mongroups.h b/ydb/core/blobstorage/vdisk/common/vdisk_mongroups.h
index d0f81cc049..f72b3ded08 100644
--- a/ydb/core/blobstorage/vdisk/common/vdisk_mongroups.h
+++ b/ydb/core/blobstorage/vdisk/common/vdisk_mongroups.h
@@ -146,8 +146,6 @@ public:
                 COUNTER_INIT(HugeUsedChunks, false);
                 COUNTER_INIT(HugeCanBeFreedChunks, false);
                 COUNTER_INIT(HugeLockedChunks, false);
-                COUNTER_INIT(DiskTimeAvailableNs, false);
-                COUNTER_INIT(EstimatedDiskTimeConsumptionNs, true);
             }
 
             COUNTER_DEF(DskOutOfSpace);
@@ -158,9 +156,22 @@ public:
             COUNTER_DEF(HugeUsedChunks);       // chunks used by huge heap
             COUNTER_DEF(HugeCanBeFreedChunks); // number of chunks that can be freed after defragmentation
             COUNTER_DEF(HugeLockedChunks);
-            // PDisk time
+        };
+
+        ///////////////////////////////////////////////////////////////////////////////////
+        // TCostGroup
+        ///////////////////////////////////////////////////////////////////////////////////
+        class TCostGroup : public TBase {
+        public:
+            GROUP_CONSTRUCTOR(TCostGroup)
+            {
+                COUNTER_INIT(DiskTimeAvailableNs, false);
+                COUNTER_INIT(VDiskUserCostNs, true);
+                COUNTER_INIT(VDiskInternalCostNs, true);
+            }
             COUNTER_DEF(DiskTimeAvailableNs);
-            COUNTER_DEF(EstimatedDiskTimeConsumptionNs);
+            COUNTER_DEF(VDiskUserCostNs);
+            COUNTER_DEF(VDiskInternalCostNs);
         };
 
         ///////////////////////////////////////////////////////////////////////////////////
diff --git a/ydb/core/blobstorage/vdisk/skeleton/blobstorage_skeletonfront.cpp b/ydb/core/blobstorage/vdisk/skeleton/blobstorage_skeletonfront.cpp
index 98d4771ab7..531bd4936e 100644
--- a/ydb/core/blobstorage/vdisk/skeleton/blobstorage_skeletonfront.cpp
+++ b/ydb/core/blobstorage/vdisk/skeleton/blobstorage_skeletonfront.cpp
@@ -703,7 +703,7 @@ namespace NKikimr {
         NMonGroup::TReplGroup ReplMonGroup;
         NMonGroup::TSyncerGroup SyncerMonGroup;
         NMonGroup::TVDiskStateGroup VDiskMonGroup;
-        NMonGroup::TDskOutOfSpaceGroup DskOutOfSpaceGroup;
+        NMonGroup::TCostGroup CostGroup;
         TVDiskIncarnationGuid VDiskIncarnationGuid;
         bool HasUnreadableBlobs = false;
         TInstant LastSanitizeTime = TInstant::Zero();
@@ -1210,7 +1210,16 @@ namespace NKikimr {
                 // good, enqueue it in intQueue
                 intQueue.Enqueue(ctx, recByteSize, std::move(event), msgId, cost,
                         deadline, extQueueId, *this, clientId, std::move(trace));
-                DskOutOfSpaceGroup.EstimatedDiskTimeConsumptionNs() += cost;
+
+                if constexpr (std::is_same_v<std::decay_t<decltype(*ev->Get())>, TEvBlobStorage::TEvVPatchXorDiff>) {
+                    // TEvVPatchXorDiff's cost is included in cost of other Patch operations
+                } else {
+                    if (clientId.GetType() == NBackpressure::EQueueClientType::DSProxy) {
+                        CostGroup.VDiskUserCostNs() += cost;
+                    } else {
+                        CostGroup.VDiskInternalCostNs() += cost;
+                    }
+                }
             }
 
             Sanitize(ctx);
@@ -2123,7 +2132,7 @@ namespace NKikimr {
             , ReplMonGroup(VDiskCounters, "subsystem", "repl")
             , SyncerMonGroup(VDiskCounters, "subsystem", "syncer")
             , VDiskMonGroup(VDiskCounters, "subsystem", "state")
-            , DskOutOfSpaceGroup(VDiskCounters, "subsystem", "outofspace")
+            , CostGroup(VDiskCounters, "subsystem", "cost")
         {
             ReplMonGroup.ReplUnreplicatedVDisks() = 1;
             VDiskMonGroup.VDiskState(NKikimrWhiteboard::EVDiskState::Initial);
diff --git a/ydb/core/blobstorage/vdisk/skeleton/skeleton_oos_tracker.cpp b/ydb/core/blobstorage/vdisk/skeleton/skeleton_oos_tracker.cpp
index cdcf289427..67d8a14fbb 100644
--- a/ydb/core/blobstorage/vdisk/skeleton/skeleton_oos_tracker.cpp
+++ b/ydb/core/blobstorage/vdisk/skeleton/skeleton_oos_tracker.cpp
@@ -18,6 +18,7 @@ namespace NKikimr {
         TPDiskCtxPtr PDiskCtx;
         const TDuration DskTrackerInterval;
         NMonGroup::TDskOutOfSpaceGroup MonGroup;
+        NMonGroup::TCostGroup CostGroup;
         // how many 'wait intervals' we spent in 'bad' zones
         ui64 YellowZonePeriods = 0;
         ui64 OrangeZonePeriods = 0;
@@ -91,7 +92,7 @@ namespace NKikimr {
             MonGroup.DskTotalBytes() = msg->TotalChunks * PDiskCtx->Dsk->ChunkSize;
             MonGroup.DskFreeBytes() = msg->FreeChunks * PDiskCtx->Dsk->ChunkSize;
             if (msg->NumSlots > 0) {
-                MonGroup.DiskTimeAvailableNs() = 1'000'000'000ull / msg->NumSlots;
+                CostGroup.DiskTimeAvailableNs() = 1'000'000'000ull / msg->NumSlots;
             }
 
             Become(&TThis::WaitFunc);
@@ -201,6 +202,7 @@ namespace NKikimr {
             , PDiskCtx(pdiskCtx)
             , DskTrackerInterval(dskTrackerInterval)
             , MonGroup(VCtx->VDiskCounters, "subsystem", "outofspace")
+            , CostGroup(VCtx->VDiskCounters, "subsystem", "cost")
         {}
     };