issue-122: merge to stable-23-3 (#687)

* issue-122: ReadBlocks should use commit id to better understand whether it overlaps with unconfirmed blobs or not (#457) * issue #122: DescribeBlocks should respect 'unconfirmed blobs' feature (#473) * issue #122: GetChangedBlocks should respect 'unconfirmed blobs' feature (#484) * issue #122: get rid of OverlapsUnconfirmedBlobs overloading; fix naming inconsistency (#494) * issue #122: limit 'unconfirmed' blob count (#617) * issue #122: add sensors for unconfirmed/confirmed blob count (#659) * issue #122: ConfirmBlobs should be visible in profile log (#670) * issue-122: AddingUnconfirmedBlobs feature to FeaturesConfig (#676) * fix compilation --------- Co-authored-by: Mikhail Montsev <[email protected]>
ydb-platform · Mar 8, 2024 · b9e8b20 · b9e8b20
1 parent 38b8930
commit b9e8b20
Show file tree

Hide file tree

Showing 33 changed files with 407 additions and 91 deletions.
diff --git a/cloud/blockstore/config/storage.proto b/cloud/blockstore/config/storage.proto
@@ -917,4 +917,7 @@ message TStorageServiceConfig
     // Note: it should be larger than ClientRemountPeriod, otherwise it's
     // useless.
     optional uint32 CachedAcquireRequestLifetime = 348;
+
+    // Limits unconfirmed (and confirmed but not added to the index) blob count.
+    optional uint32 UnconfirmedBlobCountHardLimit = 349;
 }
diff --git a/cloud/blockstore/libs/service/request.cpp b/cloud/blockstore/libs/service/request.cpp
@@ -35,6 +35,8 @@ TStringBuf GetSysRequestName(ESysRequestType requestType)
         case ESysRequestType::Migration: return "Migration";
         case ESysRequestType::WriteDeviceBlocks: return "WriteDeviceBlocks";
         case ESysRequestType::ZeroDeviceBlocks: return "ZeroDeviceBlocks";
+        case ESysRequestType::Resync: return "Resync";
+        case ESysRequestType::ConfirmBlobs: return "ConfirmBlobs";
         default: return "unknown";
     }
 }

diff --git a/cloud/blockstore/libs/service/request.h b/cloud/blockstore/libs/service/request.h
@@ -161,6 +161,7 @@ enum class ESysRequestType
     WriteDeviceBlocks = 10006,
     ZeroDeviceBlocks = 10007,
     Resync = 10008,
+    ConfirmBlobs = 10009,
     MAX
 };
 

diff --git a/cloud/blockstore/libs/storage/core/config.cpp b/cloud/blockstore/libs/storage/core/config.cpp
@@ -464,6 +464,8 @@ TDuration MSeconds(ui32 value)
     xxx(CheckBlockChecksumsInBlobsUponRead,        bool,      false           )\
     xxx(ConfigsDispatcherServiceEnabled,           bool,      false           )\
     xxx(CachedAcquireRequestLifetime,              TDuration, Seconds(40)     )\
+                                                                               \
+    xxx(UnconfirmedBlobCountHardLimit,             ui32,      1000            )\
 // BLOCKSTORE_STORAGE_CONFIG_RW
 
 #define BLOCKSTORE_STORAGE_CONFIG(xxx)                                         \
@@ -492,6 +494,7 @@ BLOCKSTORE_STORAGE_CONFIG(BLOCKSTORE_STORAGE_DECLARE_CONFIG)
     xxx(ChangeThrottlingPolicy)                                                \
     xxx(ReplaceDevice)                                                         \
     xxx(UseNonReplicatedHDDInsteadOfReplicated)                                \
+    xxx(AddingUnconfirmedBlobs)                                                \
 
 // BLOCKSTORE_BINARY_FEATURES
 

diff --git a/cloud/blockstore/libs/storage/core/config.h b/cloud/blockstore/libs/storage/core/config.h
@@ -348,6 +348,10 @@ class TStorageConfig
         const TString& cloudId,
         const TString& folderId,
         const TString& diskId) const;
+    bool IsAddingUnconfirmedBlobsFeatureEnabled(
+        const TString& cloudId,
+        const TString& folderId,
+        const TString& diskId) const;
 
     TDuration GetMaxTimedOutDeviceStateDurationFeatureValue(
         const TString& cloudId,
@@ -544,6 +548,8 @@ class TStorageConfig
     bool GetConfigsDispatcherServiceEnabled() const;
 
     TDuration GetCachedAcquireRequestLifetime() const;
+
+    ui32 GetUnconfirmedBlobCountHardLimit() const;
 };
 
 ui64 GetAllocationUnit(

diff --git a/cloud/blockstore/libs/storage/core/disk_counters.h b/cloud/blockstore/libs/storage/core/disk_counters.h
@@ -45,6 +45,8 @@ enum class EPublishingPolicy
     xxx(CompactionGarbageScore, Max,     Permanent,                __VA_ARGS__)\
     xxx(ChannelHistorySize,     Max,     Permanent,                __VA_ARGS__)\
     xxx(CompactionRangeCountPerRun, Max, Permanent,                __VA_ARGS__)\
+    xxx(UnconfirmedBlobCount,   Generic, Permanent,                __VA_ARGS__)\
+    xxx(ConfirmedBlobCount,     Generic, Permanent,                __VA_ARGS__)\
 // BLOCKSTORE_REPL_PART_SIMPLE_COUNTERS
 
 ////////////////////////////////////////////////////////////////////////////////

diff --git a/cloud/blockstore/libs/storage/partition/model/CMakeLists.darwin-x86_64.txt b/cloud/blockstore/libs/storage/partition/model/CMakeLists.darwin-x86_64.txt
@@ -45,7 +45,7 @@ target_sources(storage-partition-model PRIVATE
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/garbage_queue.cpp
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/mixed_index_cache.cpp
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/operation_status.cpp
-  ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/unconfirmed_blob.cpp
+  ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/blob_unique_id_with_range.cpp
 )
 generate_enum_serilization(storage-partition-model
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/mixed_index_cache.h

diff --git a/cloud/blockstore/libs/storage/partition/model/CMakeLists.linux-aarch64.txt b/cloud/blockstore/libs/storage/partition/model/CMakeLists.linux-aarch64.txt
@@ -46,7 +46,7 @@ target_sources(storage-partition-model PRIVATE
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/garbage_queue.cpp
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/mixed_index_cache.cpp
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/operation_status.cpp
-  ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/unconfirmed_blob.cpp
+  ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/blob_unique_id_with_range.cpp
 )
 generate_enum_serilization(storage-partition-model
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/mixed_index_cache.h

diff --git a/cloud/blockstore/libs/storage/partition/model/CMakeLists.linux-x86_64.txt b/cloud/blockstore/libs/storage/partition/model/CMakeLists.linux-x86_64.txt
@@ -46,7 +46,7 @@ target_sources(storage-partition-model PRIVATE
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/garbage_queue.cpp
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/mixed_index_cache.cpp
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/operation_status.cpp
-  ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/unconfirmed_blob.cpp
+  ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/blob_unique_id_with_range.cpp
 )
 generate_enum_serilization(storage-partition-model
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/mixed_index_cache.h

diff --git a/cloud/blockstore/libs/storage/partition/model/CMakeLists.windows-x86_64.txt b/cloud/blockstore/libs/storage/partition/model/CMakeLists.windows-x86_64.txt
@@ -45,7 +45,7 @@ target_sources(storage-partition-model PRIVATE
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/garbage_queue.cpp
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/mixed_index_cache.cpp
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/operation_status.cpp
-  ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/unconfirmed_blob.cpp
+  ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/blob_unique_id_with_range.cpp
 )
 generate_enum_serilization(storage-partition-model
   ${CMAKE_SOURCE_DIR}/cloud/blockstore/libs/storage/partition/model/mixed_index_cache.h

diff --git a/cloud/blockstore/libs/storage/partition/model/blob_unique_id_with_range.cpp b/cloud/blockstore/libs/storage/partition/model/blob_unique_id_with_range.cpp
@@ -0,0 +1,34 @@
+#include "blob_unique_id_with_range.h"
+
+namespace NCloud::NBlockStore::NStorage::NPartition {
+
+////////////////////////////////////////////////////////////////////////////////
+
+bool Overlaps(
+    const TCommitIdToBlobUniqueIdWithRange& blobs,
+    ui64 lowCommitId,
+    ui64 highCommitId,
+    const TBlockRange32& blockRange)
+{
+    for (const auto& [entryCommitId, entryBlobs]: blobs) {
+        if (entryCommitId > highCommitId) {
+            // entry is too new, thus does not affect this commit range
+            continue;
+        }
+
+        if (entryCommitId < lowCommitId) {
+            // entry is too old, thus does not affect this commit range
+            continue;
+        }
+
+        for (const auto& blob: entryBlobs) {
+            if (blob.BlockRange.Overlaps(blockRange)) {
+                return true;
+            }
+        }
+    }
+
+    return false;
+}
+
+}   // namespace NCloud::NBlockStore::NStorage::NPartition
diff --git a/...torage/partition/model/unconfirmed_blob.h → ...rtition/model/blob_unique_id_with_range.h b/...torage/partition/model/unconfirmed_blob.h → ...rtition/model/blob_unique_id_with_range.h
@@ -11,21 +11,26 @@ namespace NCloud::NBlockStore::NStorage::NPartition {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-struct TUnconfirmedBlob
+struct TBlobUniqueIdWithRange
 {
     ui64 UniqueId = 0;
     TBlockRange32 BlockRange;
 
-    TUnconfirmedBlob() = default;
+    TBlobUniqueIdWithRange() = default;
 
-    TUnconfirmedBlob(ui64 uniqueId, const TBlockRange32& blockRange)
+    TBlobUniqueIdWithRange(ui64 uniqueId, const TBlockRange32& blockRange)
         : UniqueId(uniqueId)
         , BlockRange(blockRange)
     {}
 };
 
-// mapping from CommitId
-using TUnconfirmedBlobs = THashMap<ui64, TVector<TUnconfirmedBlob>>;
-using TConfirmedBlobs = THashMap<ui64, TVector<TUnconfirmedBlob>>;
+using TCommitIdToBlobUniqueIdWithRange =
+    THashMap<ui64, TVector<TBlobUniqueIdWithRange>>;
+
+bool Overlaps(
+    const TCommitIdToBlobUniqueIdWithRange& blobs,
+    ui64 lowCommitId,
+    ui64 highCommitId,
+    const TBlockRange32& blockRange);
 
 }   // namespace NCloud::NBlockStore::NStorage::NPartition
diff --git a/cloud/blockstore/libs/storage/partition/model/unconfirmed_blob.cpp b/cloud/blockstore/libs/storage/partition/model/unconfirmed_blob.cpp
diff --git a/cloud/blockstore/libs/storage/partition/model/ya.make b/cloud/blockstore/libs/storage/partition/model/ya.make
@@ -8,6 +8,7 @@ GENERATE_ENUM_SERIALIZATION(operation_status.h)
 SRCS(
     barrier.cpp
     blob_index.cpp
+    blob_unique_id_with_range.cpp
     block.cpp
     block_index.cpp
     block_mask.cpp
@@ -18,7 +19,6 @@ SRCS(
     garbage_queue.cpp
     mixed_index_cache.cpp
     operation_status.cpp
-    unconfirmed_blob.cpp
 )
 
 PEERDIR(

diff --git a/cloud/blockstore/libs/storage/partition/part_actor_changedblocks.cpp b/cloud/blockstore/libs/storage/partition/part_actor_changedblocks.cpp
@@ -449,6 +449,26 @@ bool TPartitionActor::PrepareGetChangedBlocks(
     TRequestScope timer(*args.RequestInfo);
     TPartitionDatabase db(tx.DB);
 
+    if (State->OverlapsUnconfirmedBlobs(
+            args.LowCommitId,
+            args.HighCommitId,
+            args.ReadRange))
+    {
+        args.Interrupted = true;
+        return true;
+    }
+
+    // NOTE: we should also look in confirmed blobs because they are not added
+    // yet
+    if (State->OverlapsConfirmedBlobs(
+            args.LowCommitId,
+            args.HighCommitId,
+            args.ReadRange))
+    {
+        args.Interrupted = true;
+        return true;
+    }
+
     TChangedBlocksVisitor visitor(args);
     State->FindFreshBlocks(visitor, args.ReadRange, args.HighCommitId);
     auto ready = db.FindMixedBlocks(
@@ -493,6 +513,20 @@ void TPartitionActor::CompleteGetChangedBlocks(
         args.HighCommitId,
         DescribeRange(args.ReadRange).data());
 
+    if (args.Interrupted) {
+        LWTRACK(
+            ResponseSent_Partition,
+            args.RequestInfo->CallContext->LWOrbit,
+            "ChangedBlocks",
+            args.RequestInfo->CallContext->RequestId);
+
+        auto response = std::make_unique<TEvService::TEvGetChangedBlocksResponse>(
+            MakeError(E_REJECTED, "GetChangedBlocks transaction was interrupted")
+        );
+        NCloud::Reply(ctx, *args.RequestInfo, std::move(response));
+        return;
+    }
+
     if (!args.LowCommitId && State->GetBaseDiskId() && !args.IgnoreBaseDisk) {
         auto actor = NCloud::Register<TGetChangedBlocksActor>(
             ctx,

diff --git a/cloud/blockstore/libs/storage/partition/part_actor_confirmblobs.cpp b/cloud/blockstore/libs/storage/partition/part_actor_confirmblobs.cpp
@@ -12,6 +12,8 @@
 #include <library/cpp/actors/core/actor_bootstrapped.h>
 #include <library/cpp/actors/core/hfunc.h>
 
+#include <util/system/datetime.h>
+
 namespace NCloud::NBlockStore::NStorage::NPartition {
 
 using namespace NActors;
@@ -37,6 +39,7 @@ class TConfirmBlobsActor final
     : public TActorBootstrapped<TConfirmBlobsActor>
 {
 private:
+    const ui64 StartCycleCount = GetCycleCount();
     const ui64 TabletId = 0;
     const TActorId Tablet;
     const TVector<TRequest> Requests;
@@ -110,6 +113,7 @@ void TConfirmBlobsActor::NotifyAndDie(const TActorContext& ctx)
 {
     auto ev = std::make_unique<TEvPartitionPrivate::TEvConfirmBlobsCompleted>(
         std::move(Error),
+        StartCycleCount,
         std::move(UnrecoverableBlobs));
     NCloud::Send(ctx, Tablet, std::move(ev));
     Die(ctx);
@@ -191,10 +195,8 @@ void TPartitionActor::ConfirmBlobs(const TActorContext& ctx)
 
     TVector<TRequest> requests;
 
-    for (const auto& entry: State->GetUnconfirmedBlobs()) {
-        auto commitId = entry.first;
-
-        for (const auto& blob: entry.second) {
+    for (const auto& [commitId, blobs]: State->GetUnconfirmedBlobs()) {
+        for (const auto& blob: blobs) {
             auto blobId = MakePartialBlobId(commitId, blob.UniqueId);
             auto proxy = Info()->BSProxyIDForChannel(
                 blobId.Channel(), blobId.Generation()
@@ -235,7 +237,10 @@ void TPartitionActor::HandleConfirmBlobsCompleted(
         "[%lu] ConfirmBlobs: start tx",
         TabletID());
 
-    ExecuteTx<TConfirmBlobs>(ctx, std::move(msg->UnrecoverableBlobs));
+    ExecuteTx<TConfirmBlobs>(
+        ctx,
+        msg->StartCycleCount,
+        std::move(msg->UnrecoverableBlobs));
 }
 
 bool TPartitionActor::PrepareConfirmBlobs(
@@ -270,13 +275,30 @@ void TPartitionActor::CompleteConfirmBlobs(
     const TActorContext& ctx,
     TTxPartition::TConfirmBlobs& args)
 {
-    Y_UNUSED(args);
-
     LOG_INFO(ctx, TBlockStoreComponents::PARTITION,
         "[%lu] ConfirmBlobs: complete tx",
         TabletID());
 
     BlobsConfirmed(ctx);
+
+    const auto duration =
+        CyclesToDurationSafe(GetCycleCount() - args.StartCycleCount);
+
+    IProfileLog::TSysReadWriteRequest request;
+    request.RequestType = ESysRequestType::ConfirmBlobs;
+    request.Duration = duration;
+
+    for (const auto& [_, blobs]: State->GetConfirmedBlobs()) {
+        for (const auto& blob: blobs) {
+            request.Ranges.push_back(ConvertRangeSafe(blob.BlockRange));
+        }
+    }
+
+    IProfileLog::TRecord record;
+    record.DiskId = State->GetConfig().GetDiskId();
+    record.Ts = ctx.Now() - duration;
+    record.Request = request;
+    ProfileLog->Write(std::move(record));
 }
 
 }   // namespace NCloud::NBlockStore::NStorage::NPartition
diff --git a/cloud/blockstore/libs/storage/partition/part_actor_describeblocks.cpp b/cloud/blockstore/libs/storage/partition/part_actor_describeblocks.cpp
@@ -203,6 +203,18 @@ bool TPartitionActor::PrepareDescribeBlocks(
 
     ui64 commitId = args.CommitId;
 
+    if (State->OverlapsUnconfirmedBlobs(0, commitId, args.DescribeRange)) {
+        args.Interrupted = true;
+        return true;
+    }
+
+    // NOTE: we should also look in confirmed blobs because they are not added
+    // yet
+    if (State->OverlapsConfirmedBlobs(0, commitId, args.DescribeRange)) {
+        args.Interrupted = true;
+        return true;
+    }
+
     TDescribeBlocksVisitor visitor(args);
     State->FindFreshBlocks(visitor, args.DescribeRange, commitId);
     auto ready = db.FindMixedBlocks(
@@ -238,9 +250,6 @@ void TPartitionActor::CompleteDescribeBlocks(
 {
     TRequestScope timer(*args.RequestInfo);
 
-    auto response = std::make_unique<TEvVolume::TEvDescribeBlocksResponse>();
-    FillDescribeBlocksResponse(args, response.get());
-
     RemoveTransaction(*args.RequestInfo);
 
     const ui64 commitId = args.CommitId;
@@ -255,6 +264,19 @@ void TPartitionActor::CompleteDescribeBlocks(
         "DescribeBlocks",
         args.RequestInfo->CallContext->RequestId);
 
+    State->GetCleanupQueue().ReleaseBarrier(commitId);
+
+    if (args.Interrupted) {
+        auto response = std::make_unique<TEvVolume::TEvDescribeBlocksResponse>(
+            MakeError(E_REJECTED, "DescribeBlocks transaction was interrupted")
+        );
+        NCloud::Reply(ctx, *args.RequestInfo, std::move(response));
+        return;
+    }
+
+    auto response = std::make_unique<TEvVolume::TEvDescribeBlocksResponse>();
+    FillDescribeBlocksResponse(args, response.get());
+
     const ui64 responseBytes = response->Record.ByteSizeLong();
 
     NCloud::Reply(ctx, *args.RequestInfo, std::move(response));
@@ -278,8 +300,6 @@ void TPartitionActor::CompleteDescribeBlocks(
     record.Ts = ctx.Now() - duration;
     record.Request = request;
     ProfileLog->Write(std::move(record));
-
-    State->GetCleanupQueue().ReleaseBarrier(args.CommitId);
 }
 
 void TPartitionActor::FillDescribeBlocksResponse(