NBSNEBIUS-86: fixed stupid bug: FilteredFreshDeviceIds filtration sho…

…uld not drop all FreshDeviceIds for volumes created after 2023-08-30 (#250) * NBSNEBIUS-86: fixed stupid bug: FilteredFreshDeviceIds filtration should not drop all FreshDeviceIds for volumes created after 2023-08-30 * NBSNEBIUS-86: moved FreshDeviceIds filtration to a separate func
ydb-platform · Jan 26, 2024 · d148a4c · d148a4c
1 parent c35fabe
commit d148a4c
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 12 deletions.
diff --git a/cloud/blockstore/libs/storage/volume/volume_state.cpp b/cloud/blockstore/libs/storage/volume/volume_state.cpp
@@ -244,29 +244,39 @@ void TVolumeState::Reset()
 
     // this filtration is needed due to a bug that caused some disks to have
     // garbage in FreshDeviceIds list
-    FilteredFreshDeviceIds.clear();
+    FilteredFreshDeviceIds = MakeFilteredDeviceIds();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
+THashSet<TString> TVolumeState::MakeFilteredDeviceIds() const
+{
+    const TInstant oldDate = TInstant::ParseIso8601("2023-08-30");
+    const auto& ids = Meta.GetFreshDeviceIds();
+    if (GetCreationTs() > oldDate) {
+        return {ids.begin(), ids.end()};
+    }
 
+    THashSet<TString> filtered;
     auto addFreshDevices = [&] (const auto& devices) {
         for (const auto& device: devices) {
             const bool found = Find(
-                Meta.GetFreshDeviceIds().begin(),
-                Meta.GetFreshDeviceIds().end(),
-                device.GetDeviceUUID()) != Meta.GetFreshDeviceIds().end();
+                ids.begin(),
+                ids.end(),
+                device.GetDeviceUUID()) != ids.end();
 
             if (found) {
-                FilteredFreshDeviceIds.insert(device.GetDeviceUUID());
+                filtered.insert(device.GetDeviceUUID());
             }
         }
     };
 
-    // XXX the aforementioned bug affects only "old" disks, see NBS-4383
-    const TInstant oldDate = TInstant::ParseIso8601("2023-08-30");
-    if (GetCreationTs() <= oldDate) {
-        addFreshDevices(Meta.GetDevices());
-        for (const auto& r: Meta.GetReplicas()) {
-            addFreshDevices(r.GetDevices());
-        }
+    addFreshDevices(Meta.GetDevices());
+    for (const auto& r: Meta.GetReplicas()) {
+        addFreshDevices(r.GetDevices());
     }
+
+    return filtered;
 }
 
 ////////////////////////////////////////////////////////////////////////////////

diff --git a/cloud/blockstore/libs/storage/volume/volume_state.h b/cloud/blockstore/libs/storage/volume/volume_state.h
@@ -620,6 +620,8 @@ class TVolumeState
         ui64 proposedFillGeneration);
 
     THistoryLogKey AllocateHistoryLogKey(TInstant timestamp);
+
+    THashSet<TString> MakeFilteredDeviceIds() const;
 };
 
 }   // namespace NCloud::NBlockStore::NStorage
diff --git a/cloud/blockstore/libs/storage/volume/volume_state_ut.cpp b/cloud/blockstore/libs/storage/volume/volume_state_ut.cpp
@@ -1658,6 +1658,43 @@ Y_UNIT_TEST_SUITE(TVolumeStateTest)
                 volumeState.GetFilteredFreshDevices().begin(),
                 volumeState.GetFilteredFreshDevices().end()));
     }
+
+    Y_UNIT_TEST(ShouldNotFilterFreshDeviceIdsForNewDisks)
+    {
+        // testing that we do not filter out garbage fresh device ids for the
+        // disks created after NBS-4383 got fixed
+        auto volumeState = CreateVolumeState();
+        auto meta = volumeState.GetMeta();
+        const TInstant newDate = TInstant::ParseIso8601("2023-08-31");
+        meta.MutableVolumeConfig()->SetCreationTs(newDate.MicroSeconds());
+        meta.AddDevices()->SetDeviceUUID("d1");
+        meta.AddDevices()->SetDeviceUUID("d2");
+        auto& r1 = *meta.AddReplicas();
+        r1.AddDevices()->SetDeviceUUID("d3");
+        r1.AddDevices()->SetDeviceUUID("d4");
+        auto& r2 = *meta.AddReplicas();
+        r2.AddDevices()->SetDeviceUUID("d5");
+        r2.AddDevices()->SetDeviceUUID("d6");
+        meta.AddFreshDeviceIds("d2");
+        meta.AddFreshDeviceIds("d3");
+        meta.AddFreshDeviceIds("d5");
+        meta.AddFreshDeviceIds("garbage1");
+        meta.AddFreshDeviceIds("garbage2");
+        meta.AddFreshDeviceIds("garbage3");
+        volumeState.ResetMeta(meta);
+
+        ASSERT_VECTOR_CONTENTS_EQUAL(
+            TVector<TString>({
+                "d2",
+                "d3",
+                "d5",
+                "garbage1",
+                "garbage2",
+                "garbage3"}),
+            TVector<TString>(
+                volumeState.GetFilteredFreshDevices().begin(),
+                volumeState.GetFilteredFreshDevices().end()));
+    }
 }
 
 }   // namespace NCloud::NBlockStore::NStorage