Fix bug in distconf drive enumeration code (#4873)

Author: alexvru

ydb/core/blobstorage/nodewarden/distconf_fsm.cpp

@@ -299,13 +299,12 @@ namespace NKikimr::NStorage {
         switch (task.Request.GetRequestCase()) {
             case TEvScatter::kCollectConfigs: {
                 std::vector<TString> drives;
-                EnumerateConfigDrives(*StorageConfig, 0, [&](const auto& /*node*/, const auto& drive) {
+                auto callback = [&](const auto& /*node*/, const auto& drive) {
                     drives.push_back(drive.GetPath());
-                });
+                };
+                EnumerateConfigDrives(*StorageConfig, SelfId().NodeId(), callback);
                 if (ProposedStorageConfig) {
-                    EnumerateConfigDrives(*ProposedStorageConfig, 0, [&](const auto& /*node*/, const auto& drive) {
-                        drives.push_back(drive.GetPath());
-                    });
+                    EnumerateConfigDrives(*ProposedStorageConfig, SelfId().NodeId(), callback);
                 }
                 std::sort(drives.begin(), drives.end());
                 drives.erase(std::unique(drives.begin(), drives.end()), drives.end());
@@ -342,8 +341,6 @@ namespace NKikimr::NStorage {
                     }
 
                     PersistConfig([this, cookie](TEvPrivate::TEvStorageConfigStored& msg) {
-                        STLOG(PRI_DEBUG, BS_NODE, NWDC45, "ProposeStorageConfig TEvStorageConfigStored", (Cookie, cookie));
-
                         Y_ABORT_UNLESS(ProposedStorageConfigCookieUsage);
                         Y_ABORT_UNLESS(cookie == ProposedStorageConfigCookie);
                         --ProposedStorageConfigCookieUsage;
@@ -362,6 +359,11 @@ namespace NKikimr::NStorage {
                                     }
                                 }
                             }
+                            STLOG(PRI_DEBUG, BS_NODE, NWDC48, "ProposeStorageConfig TEvStorageConfigStored",
+                                (Cookie, cookie), (Status, *status));
+                        } else {
+                            STLOG(PRI_DEBUG, BS_NODE, NWDC45, "ProposeStorageConfig TEvStorageConfigStored no scatter task",
+                                (Cookie, cookie));
                         }
 
                         FinishAsyncOperation(cookie);

ydb/core/blobstorage/nodewarden/distconf_persistent_storage.cpp

@@ -86,15 +86,12 @@ namespace NKikimr::NStorage {
         }
 
         std::vector<TString> drives;
+        auto processDrive = [&](const auto& /*node*/, const auto& drive) { drives.push_back(drive.GetPath()); };
         if (item.Record.HasCommittedStorageConfig()) {
-            EnumerateConfigDrives(item.Record.GetCommittedStorageConfig(), 0, [&](const auto& /*node*/, const auto& drive) {
-                drives.push_back(drive.GetPath());
-            });
+            EnumerateConfigDrives(item.Record.GetCommittedStorageConfig(), SelfId().NodeId(), processDrive);
         }
         if (item.Record.HasProposedStorageConfig()) {
-            EnumerateConfigDrives(item.Record.GetProposedStorageConfig(), 0, [&](const auto& /*node*/, const auto& drive) {
-                drives.push_back(drive.GetPath());
-            });
+            EnumerateConfigDrives(item.Record.GetProposedStorageConfig(), SelfId().NodeId(), processDrive);
         }
         std::sort(drives.begin(), drives.end());
         drives.erase(std::unique(drives.begin(), drives.end()), drives.end());
@@ -224,19 +221,19 @@ namespace NKikimr::NStorage {
 
 namespace NKikimr {
 
+    static const TString VaultLockFile = "/Berkanavt/kikimr/state/storage.lock";
     static const TString VaultPath = "/Berkanavt/kikimr/state/storage.txt";
+    static TMutex VaultMutex;
 
-    static bool ReadVault(TFile& file, NKikimrBlobStorage::TStorageFileContent& vault) {
-        TString buffer;
-        try {
-            buffer = TFileInput(file).ReadAll();
-        } catch (...) {
-            return false;
-        }
-        if (!buffer) {
+    static bool ReadVault(NKikimrBlobStorage::TStorageFileContent& vault) {
+        if (TFileHandle fh(VaultPath, OpenExisting | RdOnly); !fh.IsOpen()) {
+            return true;
+        } else if (TString buffer = TString::Uninitialized(fh.GetLength())) {
+            return fh.Read(buffer.Detach(), buffer.size()) == (i32)buffer.size() &&
+                google::protobuf::util::JsonStringToMessage(buffer, &vault).ok();
+        } else {
             return true;
         }
-        return google::protobuf::util::JsonStringToMessage(buffer, &vault).ok();
     }
 
     static bool WriteVault(NKikimrBlobStorage::TStorageFileContent& vault) {
@@ -245,30 +242,29 @@ namespace NKikimr {
         Y_ABORT_UNLESS(success);
 
         const TString tempPath = VaultPath + ".tmp";
-        TFileHandle fh1(tempPath, OpenAlways);
-        if (!fh1.IsOpen() || fh1.Write(buffer.data(), buffer.size()) != (i32)buffer.size()) {
+        if (TFileHandle fh(tempPath, CreateAlways | WrOnly); !fh.IsOpen()) {
             return false;
-        }
-
-        if (!NFs::Rename(tempPath, VaultPath)) {
+        } else if (fh.Flock(LOCK_EX | LOCK_NB)) {
+            Y_DEBUG_ABORT("unexpected lock race");
+            return false;
+        } else if (fh.Write(buffer.data(), buffer.size()) != (i32)buffer.size()) {
+            return false;
+        } else if (!NFs::Rename(tempPath, VaultPath)) {
             return false;
         }
-
         return true;
     }
 
     NPDisk::EPDiskMetadataOutcome ReadPDiskMetadata(const TString& path, const NPDisk::TMainKey& key, TRcBuf& metadata,
             std::optional<ui64> *pdiskGuid) {
-        TFileHandle fh(VaultPath, OpenExisting | RdWr);
-        if (!fh.IsOpen()) {
-            return NPDisk::EPDiskMetadataOutcome::NO_METADATA;
-        } else if (fh.Flock(LOCK_SH)) {
+        TGuard<TMutex> guard(VaultMutex);
+        TFileHandle lock(VaultLockFile, OpenAlways);
+        if (!lock.IsOpen() || flock(lock, LOCK_EX) == -1) {
             return NPDisk::EPDiskMetadataOutcome::ERROR;
         }
-        TFile file(fh.Release());
 
         NKikimrBlobStorage::TStorageFileContent vault;
-        if (!ReadVault(file, vault)) {
+        if (!ReadVault(vault)) {
             return NPDisk::EPDiskMetadataOutcome::ERROR;
         }
 
@@ -323,14 +319,14 @@ namespace NKikimr {
 
     NPDisk::EPDiskMetadataOutcome WritePDiskMetadata(const TString& path, const NPDisk::TMainKey& key, TRcBuf&& metadata,
             std::optional<ui64> *pdiskGuid) {
-        TFileHandle fh(VaultPath, OpenAlways);
-        if (!fh.IsOpen() || fh.Flock(LOCK_EX)) {
+        TGuard<TMutex> guard(VaultMutex);
+        TFileHandle lock(VaultLockFile, OpenAlways);
+        if (!lock.IsOpen() || flock(lock, LOCK_EX) == -1) {
             return NPDisk::EPDiskMetadataOutcome::ERROR;
         }
-        TFile file(fh.Release());
 
         NKikimrBlobStorage::TStorageFileContent vault;
-        if (!ReadVault(file, vault)) {
+        if (!ReadVault(vault)) {
             return NPDisk::EPDiskMetadataOutcome::ERROR;
         }