Issue 2404: mv WrongMigratedDeviceOwnership critical event to AddMigr…

…ation Function (#2955) * issue-2404: mv critical event to add migration function * issue-2404: correct test * issue-2404: add comments * issue-2404: fix test * issue-2404: rm bool flag * issue-2404: refactor * issue-2404: rm friend class
ydb-platform · Feb 3, 2025 · 4d37f30 · 4d37f30
1 parent 253b505
commit 4d37f30
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 10 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
@@ -5023,14 +5023,17 @@ void TDiskRegistryState::ApplyAgentStateChange(
         }
 
         if (agent.GetState() == NProto::AGENT_STATE_WARNING) {
-            if (Find(disk.Devices, deviceId) == disk.Devices.end()) {
-                ReportDiskRegistryWrongMigratedDeviceOwnership(
-                    TStringBuilder() << "ApplyAgentStateChange: device "
-                                     << deviceId << " not found");
-                continue;
-            }
-
             if (MigrationCanBeStarted(disk, deviceId)) {
+                if (!FindPtr(disk.Devices, deviceId)) {
+                    ReportDiskRegistryWrongMigratedDeviceOwnership(Sprintf(
+                        "ApplyAgentStateChange: device[DeviceUUID = %s] not "
+                        "found in disk[DiskId "
+                        "= %s]",
+                        deviceId.c_str(),
+                        diskId.c_str()));
+                    continue;
+                }
+
                 AddMigration(disk, diskId, deviceId);
             }
         } else {

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
@@ -388,9 +388,7 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMigrationTest)
         }
 
         UNIT_ASSERT(state.IsMigrationListEmpty());
-        // Now bug is fixed, but, if it reproduce in future, we must report
-        // event.
-        UNIT_ASSERT_VALUES_EQUAL(1, configCounter->Val());
+        UNIT_ASSERT_VALUES_EQUAL(0, configCounter->Val());
     }
 
     Y_UNIT_TEST(ShouldEraseMigrationsForDeletedDisk)
@@ -1177,6 +1175,14 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMigrationTest)
         UNIT_ASSERT_VALUES_EQUAL(0, state.BuildMigrationList().size());
         UNIT_ASSERT(state.IsMigrationListEmpty());
 
+        NMonitoring::TDynamicCountersPtr counters =
+            new NMonitoring::TDynamicCounters();
+        InitCriticalEventsCounter(counters);
+        auto critCounter = counters->GetCounter(
+            "AppCriticalEvents/DiskRegistryWrongMigratedDeviceOwnership",
+            true);
+        UNIT_ASSERT_VALUES_EQUAL(0, critCounter->Val());
+
         executor.WriteTx(
             [&](TDiskRegistryDatabase db)
             {
@@ -1291,6 +1297,10 @@ Y_UNIT_TEST_SUITE(TDiskRegistryStateMigrationTest)
 
         auto migrationsAfterSecondRequest = state.BuildMigrationList();
         UNIT_ASSERT_VALUES_EQUAL(0, migrationsAfterSecondRequest.size());
+        critCounter = counters->GetCounter(
+            "AppCriticalEvents/DiskRegistryWrongMigratedDeviceOwnership",
+            true);
+        UNIT_ASSERT_VALUES_EQUAL(0, critCounter->Val());
     }
 
     Y_UNIT_TEST(ShouldStartCanceledMigrationAgent)