issue-2404: correct issues

ydb-platform · Jan 22, 2025 · d67fc79 · d67fc79
1 parent e76f785
commit d67fc79
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 24 deletions.
diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state.cpp
@@ -5031,18 +5031,18 @@ void TDiskRegistryState::ApplyAgentStateChange(
         }
 
         if (agent.GetState() == NProto::AGENT_STATE_WARNING) {
-            if (!NeedToStartMigration(disk, deviceId)) {
-                // migration already started or finished
-                continue;
-            }
-
             if (Find(disk.Devices, deviceId) == disk.Devices.end()) {
                 ReportDiskRegistryWrongMigratedDeviceOwnership(
                     TStringBuilder() << "ApplyAgentStateChange: device "
                                      << deviceId << " not found");
                 continue;
             }
 
+            if (!NeedToStartMigration(disk, deviceId)) {
+                // migration already started or finished
+                continue;
+            }
+
             AddMigration(disk, diskId, deviceId);
         } else {
             if (agent.GetState() == NProto::AGENT_STATE_UNAVAILABLE
@@ -7574,12 +7574,21 @@ bool TDiskRegistryState::NeedToStartMigration(
     const TDiskState& disk,
     const TString& deviceUUID)
 {
-    return !(
-        disk.MigrationSource2Target.contains(deviceUUID) ||
-        FindIfPtr(
+    if (disk.MigrationSource2Target.contains(deviceUUID)) {
+        // migration already started
+        return false;
+    }
+
+    if (FindIfPtr(
             disk.FinishedMigrations,
-            [&](const auto& el)
-            { return el.DeviceId == deviceUUID && !el.IsCanceled; }));
+            [&](const auto& m)
+            { return m.DeviceId == deviceUUID && !m.IsCanceled; }))
+    {
+        // there is a finished migration for the device
+        return false;
+    }
+
+    return true;
 }
 
 }   // namespace NCloud::NBlockStore::NStorage
diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_cms.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_cms.cpp
@@ -20,6 +20,7 @@ namespace NCloud::NBlockStore::NStorage {
 using namespace NDiskRegistryStateTest;
 
 ////////////////////////////////////////////////////////////////////////////////
+
 Y_UNIT_TEST_SUITE(TDiskRegistryStateCMSTest)
 {
     Y_UNIT_TEST(ShouldAddNewDevice)
@@ -1124,6 +1125,7 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateCMSTest)
                     0,
                     agentConfig->GetUnknownDevices().size());
             });
+
     }
 
 }

diff --git a/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_migration.cpp b/cloud/blockstore/libs/storage/disk_registry/disk_registry_state_ut_migration.cpp
@@ -61,7 +61,7 @@ TResultOrError<TDiskRegistryState::TAllocateDiskResult> AllocateDisk(
     return result;
 }
 
-TVector<NProto::TAgentConfig> GetSeveralAgents()
+TVector<NProto::TAgentConfig> CreateSeveralAgents()
 {
     return {
         AgentConfig(
@@ -78,7 +78,7 @@ TVector<NProto::TAgentConfig> GetSeveralAgents()
             })};
 }
 
-TDiskRegistryState GetTestState(const TVector<NProto::TAgentConfig>& agents)
+TDiskRegistryState CreateTestState(const TVector<NProto::TAgentConfig>& agents)
 {
     return TDiskRegistryStateBuilder()
         .WithKnownAgents(agents)
@@ -1170,9 +1170,9 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMigrationTest)
         TTestExecutor executor;
         executor.WriteTx([&](TDiskRegistryDatabase db) { db.InitSchema(); });
 
-        const TVector agents = GetSeveralAgents();
+        const TVector agents = CreateSeveralAgents();
 
-        TDiskRegistryState state = GetTestState(agents);
+        TDiskRegistryState state = CreateTestState(agents);
 
         UNIT_ASSERT_VALUES_EQUAL(0, state.BuildMigrationList().size());
         UNIT_ASSERT(state.IsMigrationListEmpty());
@@ -1225,7 +1225,6 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMigrationTest)
             [&](TDiskRegistryDatabase db)
             {
                 auto [result, error] = AllocateDisk(db, state, "disk-1");
-                ;
                 UNIT_ASSERT_SUCCESS(error);
 
                 UNIT_ASSERT_VALUES_EQUAL(2, result.Devices.size());
@@ -1299,9 +1298,9 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMigrationTest)
         TTestExecutor executor;
         executor.WriteTx([&](TDiskRegistryDatabase db) { db.InitSchema(); });
 
-        const TVector agents = GetSeveralAgents();
+        const TVector agents = CreateSeveralAgents();
 
-        TDiskRegistryState state = GetTestState(agents);
+        TDiskRegistryState state = CreateTestState(agents);
 
         UNIT_ASSERT_VALUES_EQUAL(0, state.BuildMigrationList().size());
         UNIT_ASSERT(state.IsMigrationListEmpty());
@@ -1348,7 +1347,6 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMigrationTest)
             [&](TDiskRegistryDatabase db)
             {
                 auto [result, error] = AllocateDisk(db, state, "disk-1");
-                ;
                 UNIT_ASSERT_SUCCESS(error);
 
                 UNIT_ASSERT_VALUES_EQUAL(2, result.Devices.size());
@@ -1420,9 +1418,9 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMigrationTest)
         TTestExecutor executor;
         executor.WriteTx([&](TDiskRegistryDatabase db) { db.InitSchema(); });
 
-        const TVector agents = GetSeveralAgents();
+        const TVector agents = CreateSeveralAgents();
 
-        TDiskRegistryState state = GetTestState(agents);
+        TDiskRegistryState state = CreateTestState(agents);
 
         UNIT_ASSERT_VALUES_EQUAL(0, state.BuildMigrationList().size());
         UNIT_ASSERT(state.IsMigrationListEmpty());
@@ -1475,7 +1473,6 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMigrationTest)
             [&](TDiskRegistryDatabase db)
             {
                 auto [result, error] = AllocateDisk(db, state, "disk-1");
-                ;
                 UNIT_ASSERT_SUCCESS(error);
 
                 UNIT_ASSERT_VALUES_EQUAL(2, result.Devices.size());
@@ -1544,9 +1541,9 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMigrationTest)
         TTestExecutor executor;
         executor.WriteTx([&](TDiskRegistryDatabase db) { db.InitSchema(); });
 
-        const TVector agents = GetSeveralAgents();
+        const TVector agents = CreateSeveralAgents();
 
-        TDiskRegistryState state = GetTestState(agents);
+        TDiskRegistryState state = CreateTestState(agents);
 
         UNIT_ASSERT_VALUES_EQUAL(0, state.BuildMigrationList().size());
         UNIT_ASSERT(state.IsMigrationListEmpty());
@@ -1599,7 +1596,6 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMigrationTest)
             [&](TDiskRegistryDatabase db)
             {
                 auto [result, error] = AllocateDisk(db, state, "disk-1");
-                ;
                 UNIT_ASSERT_SUCCESS(error);
 
                 UNIT_ASSERT_VALUES_EQUAL(2, result.Devices.size());