From 7a99d70ad33b7feba0199624e32dbfb9b6a04e36 Mon Sep 17 00:00:00 2001
From: Andrei Strelkovskii <thinkingwhat@yandex.ru>
Date: Fri, 26 Jan 2024 15:21:59 +0300
Subject: [PATCH] NBSNEBIUS-86: fixed stupid bug: FilteredFreshDeviceIds
 filtration should 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
---
 .../libs/storage/volume/volume_state.cpp      | 34 +++++++++++------
 .../libs/storage/volume/volume_state.h        |  2 +
 .../libs/storage/volume/volume_state_ut.cpp   | 37 +++++++++++++++++++
 3 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/cloud/blockstore/libs/storage/volume/volume_state.cpp b/cloud/blockstore/libs/storage/volume/volume_state.cpp
index 0097f10ec6c..7e94c526dfa 100644
--- 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
index 07b9f11a716..8c748814428 100644
--- a/cloud/blockstore/libs/storage/volume/volume_state.h
+++ b/cloud/blockstore/libs/storage/volume/volume_state.h
@@ -619,6 +619,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
index b61d3b30a3b..8b804ab7c91 100644
--- 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