NBS-4827 fix review issues #2

ydb-platform · Mar 13, 2024 · 7695bbd · 7695bbd
1 parent 3b1ded0
commit 7695bbd
Show file tree

Hide file tree

Showing 12 changed files with 346 additions and 102 deletions.
diff --git a/cloud/blockstore/config/storage.proto b/cloud/blockstore/config/storage.proto
@@ -920,4 +920,26 @@ message TStorageServiceConfig
 
     // Limits unconfirmed (and confirmed but not added to the index) blob count.
     optional uint32 UnconfirmedBlobCountHardLimit = 349;
+
+    // Max bandwidth used for shadow disk fill in MiB/s (actual bandwidth is x2
+    //  due to the need to read and write).
+    optional uint32 MaxShadowDiskFillBandwidth = 350;
+    // Minimum delay between shadow disk acquire attempts when writes to the
+    // source disk are blocked (in ms).
+    optional uint32 MinAcquireShadowDiskRetryDelayWhenBlocked = 351;
+    // Maximum delay between shadow disk acquire attempts when writes to the
+    // source disk are blocked (in ms).
+    optional uint32 MaxAcquireShadowDiskRetryDelayWhenBlocked = 352;
+    // Minimum delay between shadow disk acquire attempts when writes to the
+    // source disk are not blocked (in ms).
+    optional uint32 MinAcquireShadowDiskRetryDelayWhenNonBlocked = 353;
+    // Maximum delay between shadow disk acquire attempts when writes to the
+    // source disk are not blocked (in ms).
+    optional uint32 MaxAcquireShadowDiskRetryDelayWhenNonBlocked = 354;
+    // Timeout for attempts to acquire the shadow disk when writes to the source
+    // disk are blocked (in ms).
+    optional uint32 MaxAcquireShadowDiskTotalTimeoutWhenBlocked = 355;
+    // Timeout for attempts to acquire the shadow disk when writes to the source
+    // disk are not blocked (in ms).
+    optional uint32 MaxAcquireShadowDiskTotalTimeoutWhenNonBlocked = 356;
 }
diff --git a/cloud/blockstore/libs/storage/core/config.cpp b/cloud/blockstore/libs/storage/core/config.cpp
@@ -466,6 +466,15 @@ TDuration MSeconds(ui32 value)
     xxx(CachedAcquireRequestLifetime,              TDuration, Seconds(40)     )\
                                                                                \
     xxx(UnconfirmedBlobCountHardLimit,             ui32,      1000            )\
+                                                                               \
+    xxx(MaxShadowDiskFillBandwidth,                     ui32,      500           )\
+    xxx(MinAcquireShadowDiskRetryDelayWhenBlocked,      TDuration, MSeconds(250) )\
+    xxx(MaxAcquireShadowDiskRetryDelayWhenBlocked,      TDuration, Seconds(1)    )\
+    xxx(MinAcquireShadowDiskRetryDelayWhenNonBlocked,   TDuration, Seconds(1)    )\
+    xxx(MaxAcquireShadowDiskRetryDelayWhenNonBlocked,   TDuration, Seconds(10)   )\
+    xxx(MaxAcquireShadowDiskTotalTimeoutWhenBlocked,    TDuration, Seconds(5)    )\
+    xxx(MaxAcquireShadowDiskTotalTimeoutWhenNonBlocked, TDuration, Seconds(600)  )\
+
 // BLOCKSTORE_STORAGE_CONFIG_RW
 
 #define BLOCKSTORE_STORAGE_CONFIG(xxx)                                         \

diff --git a/cloud/blockstore/libs/storage/core/config.h b/cloud/blockstore/libs/storage/core/config.h
@@ -550,6 +550,14 @@ class TStorageConfig
     TDuration GetCachedAcquireRequestLifetime() const;
 
     ui32 GetUnconfirmedBlobCountHardLimit() const;
+
+    ui32 GetMaxShadowDiskFillBandwidth() const;
+    TDuration GetMinAcquireShadowDiskRetryDelayWhenBlocked() const;
+    TDuration GetMaxAcquireShadowDiskRetryDelayWhenBlocked() const;
+    TDuration GetMinAcquireShadowDiskRetryDelayWhenNonBlocked() const;
+    TDuration GetMaxAcquireShadowDiskRetryDelayWhenNonBlocked() const;
+    TDuration GetMaxAcquireShadowDiskTotalTimeoutWhenBlocked() const;
+    TDuration GetMaxAcquireShadowDiskTotalTimeoutWhenNonBlocked() const;
 };
 
 ui64 GetAllocationUnit(

diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/migration_timeout_calculator.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/migration_timeout_calculator.cpp
@@ -8,20 +8,20 @@ namespace NCloud::NBlockStore::NStorage {
 ///////////////////////////////////////////////////////////////////////////////
 
 TMigrationTimeoutCalculator::TMigrationTimeoutCalculator(
-        TStorageConfigPtr config,
+        ui32 maxMigrationBandwidthMiBs,
+        ui32 expectedDiskAgentSize,
         TNonreplicatedPartitionConfigPtr partitionConfig)
-    : Config(std::move(config))
+    : MaxMigrationBandwidthMiBs(maxMigrationBandwidthMiBs)
+    , ExpectedDiskAgentSize(expectedDiskAgentSize)
     , PartitionConfig(std::move(partitionConfig))
 {}
 
 TDuration TMigrationTimeoutCalculator::CalculateTimeout(
     TBlockRange64 nextProcessingRange) const
 {
-    const ui32 maxMigrationBandwidthMiBs = Config->GetMaxMigrationBandwidth();
-    const ui32 expectedDiskAgentSize = Config->GetExpectedDiskAgentSize();
 
     // migration range is 4_MB
-    const auto migrationFactorPerAgent = maxMigrationBandwidthMiBs / 4;
+    const auto migrationFactorPerAgent = MaxMigrationBandwidthMiBs / 4;
 
     if (PartitionConfig->GetUseSimpleMigrationBandwidthLimiter()) {
         return TDuration::Seconds(1) / migrationFactorPerAgent;
@@ -40,7 +40,7 @@ TDuration TMigrationTimeoutCalculator::CalculateTimeout(
     }
 
     const auto factor =
-        Max(migrationFactorPerAgent * agentDeviceCount / expectedDiskAgentSize,
+        Max(migrationFactorPerAgent * agentDeviceCount / ExpectedDiskAgentSize,
             1U);
 
     return TDuration::Seconds(1) / factor;

diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/migration_timeout_calculator.h b/cloud/blockstore/libs/storage/partition_nonrepl/migration_timeout_calculator.h
@@ -12,16 +12,18 @@ namespace NCloud::NBlockStore::NStorage {
 class TMigrationTimeoutCalculator
 {
 private:
-    const TStorageConfigPtr Config;
+    const ui32 MaxMigrationBandwidthMiBs = 0;
+    const ui32 ExpectedDiskAgentSize = 0;
     TNonreplicatedPartitionConfigPtr PartitionConfig;
 
 public:
     TMigrationTimeoutCalculator(
-        TStorageConfigPtr config,
+        ui32 maxMigrationBandwidthMiBs,
+        ui32 expectedDiskAgentSize,
         TNonreplicatedPartitionConfigPtr partitionConfig);
 
-    [[nodiscard]] TDuration CalculateTimeout(
-        TBlockRange64 nextProcessingRange) const;
+    [[nodiscard]] TDuration
+    CalculateTimeout(TBlockRange64 nextProcessingRange) const;
 };
 
 }   // namespace NCloud::NBlockStore::NStorage
diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/migration_timeout_calculator_ut.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/migration_timeout_calculator_ut.cpp
@@ -42,15 +42,6 @@ TDevices MakeDevices()
     return result;
 }
 
-TStorageConfigPtr MakeStorageConfig(ui32 expectedDiskAgentSize)
-{
-    NProto::TStorageServiceConfig storageConfig;
-    storageConfig.SetMaxMigrationBandwidth(16);
-    storageConfig.SetExpectedDiskAgentSize(expectedDiskAgentSize);
-
-    return std::make_shared<TStorageConfig>(std::move(storageConfig), nullptr);
-}
-
 TNonreplicatedPartitionConfigPtr MakePartitionConfig(
     TDevices devices,
     bool useSimpleMigrationBandwidthLimiter)
@@ -80,7 +71,8 @@ Y_UNIT_TEST_SUITE(TMigrationCalculatorTest)
     Y_UNIT_TEST(ShouldCalculateMigrationTimeout)
     {
         TMigrationTimeoutCalculator timeoutCalculator(
-            MakeStorageConfig(4),
+            16,
+            4,
             MakePartitionConfig(MakeDevices(), false));
 
         // Devices #1, #2, #4 belong to Agent#1, device #3 belong to Agent#2.
@@ -112,7 +104,8 @@ Y_UNIT_TEST_SUITE(TMigrationCalculatorTest)
     Y_UNIT_TEST(ShouldCalculateMigrationTimeoutWithSimpleLimiter)
     {
         TMigrationTimeoutCalculator timeoutCalculator(
-            MakeStorageConfig(100500),
+            16,
+            100500,
             MakePartitionConfig(MakeDevices(), true));
 
         // When UseSimpleMigrationBandwidthLimiter enabled we expect the same

diff --git a/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_migration_actor.cpp b/cloud/blockstore/libs/storage/partition_nonrepl/part_nonrepl_migration_actor.cpp
@@ -40,7 +40,10 @@ TNonreplicatedPartitionMigrationActor::TNonreplicatedPartitionMigrationActor(
     , SrcConfig(std::move(srcConfig))
     , Migrations(std::move(migrations))
     , RdmaClient(std::move(rdmaClient))
-    , TimeoutCalculator(Config, SrcConfig)
+    , TimeoutCalculator(
+          Config->GetMaxMigrationBandwidth(),
+          Config->GetExpectedDiskAgentSize(),
+          SrcConfig)
 {}
 
 void TNonreplicatedPartitionMigrationActor::OnBootstrap(