Fix tests KIKIMR-18774

6 files changed, 39 insertions, 52 deletions

diff --git a/ydb/core/cms/cms_ut_common.cpp b/ydb/core/cms/cms_ut_common.cpp
index 96421cc7a5..5bfea71ce2 100644
--- a/ydb/core/cms/cms_ut_common.cpp
+++ b/ydb/core/cms/cms_ut_common.cpp
@@ -1,7 +1,7 @@
 #include "cms_impl.h"
 #include "cms_ut_common.h"
-#include "ut_helpers.h"
 #include "sentinel.h"
+#include "ut_helpers.h"
 
 #include <ydb/core/base/tabletid.h>
 #include <ydb/core/blobstorage/crypto/default.h>
@@ -56,7 +56,7 @@ void TFakeNodeWhiteboardService::Handle(TEvBlobStorage::TEvControllerConfigReque
 {
     TGuard<TMutex> guard(Mutex);
     auto &rec = ev->Get()->Record;
-    auto *resp = new TEvBlobStorage::TEvControllerConfigResponse;
+    auto resp = MakeHolder<TEvBlobStorage::TEvControllerConfigResponse>();
     if (rec.GetRequest().CommandSize() && rec.GetRequest().GetCommand(0).HasQueryBaseConfig()) {
         resp->Record.CopyFrom(Config);
     } else if (rec.GetRequest().CommandSize() && rec.GetRequest().GetCommand(0).HasReadDriveStatus()) {
@@ -69,27 +69,27 @@ void TFakeNodeWhiteboardService::Handle(TEvBlobStorage::TEvControllerConfigReque
         driveStatus.MutableHostKey()->SetIcPort(drive.GetHostKey().GetIcPort());
         driveStatus.SetPath(drive.GetPath());
         driveStatus.SetStatus(NKikimrBlobStorage::ACTIVE);
-    } else if (rec.GetRequest().CommandSize() && rec.GetRequest().GetCommand(0).HasUpdateDriveStatus()) { // assume that all commands are UpdateDriveStatus
+    } else if (rec.GetRequest().CommandSize() && rec.GetRequest().GetCommand(0).HasUpdateDriveStatus()) {
+        // assume that all commands are UpdateDriveStatus
         if (NoisyBSCPipe && ++NoisyBSCPipeCounter % 3) {
             ctx.Send(ev->Sender, new TEvSentinel::TEvBSCPipeDisconnected, 0);
-            delete resp;
             return;
         }
         bool success = true;
         for (ui32 i = 0; i < rec.GetRequest().CommandSize(); ++i) {
-            auto cmd = rec.GetRequest().GetCommand(i).GetUpdateDriveStatus();
-            auto id = NCms::TPDiskID(cmd.GetHostKey().GetNodeId(), cmd.GetPDiskId());
-            if (auto& pattern = BSControllerResponsePatterns[id]; !pattern.empty() && !pattern[0]) {
-                success = false;
+            const auto &cmd = rec.GetRequest().GetCommand(i).GetUpdateDriveStatus();
+            const auto id = NCms::TPDiskID(cmd.GetHostKey().GetNodeId(), cmd.GetPDiskId());
+            if (auto& pattern = BSControllerResponsePatterns[id]; !pattern.empty()) {
+                success = success && pattern[0];
+                resp->Record.MutableResponse()->AddStatus()->SetSuccess(pattern[0]);
                 pattern.erase(pattern.begin());
-                resp->Record.MutableResponse()->AddStatus()->SetSuccess(false);
             } else {
                 resp->Record.MutableResponse()->AddStatus()->SetSuccess(true);
             }
         }
         resp->Record.MutableResponse()->SetSuccess(success);
     }
-    ctx.Send(ev->Sender, resp, 0, ev->Cookie);
+    ctx.Send(ev->Sender, std::move(resp), 0, ev->Cookie);
 }
 
 void TFakeNodeWhiteboardService::Handle(TEvWhiteboard::TEvTabletStateRequest::TPtr &ev,
@@ -1177,10 +1177,10 @@ NKikimrCms::TGetLogTailResponse TCmsTestEnv::GetLogTail(ui32 type,
     return rec;
 }
 
-void TCmsTestEnv::AddBSCFailures(const NCms::TPDiskID& id, TVector<bool> failuresPattern) {
+void TCmsTestEnv::AddBSCFailures(const NCms::TPDiskID& id, TVector<bool> &&failuresPattern) {
     TGuard<TMutex> guard(TFakeNodeWhiteboardService::Mutex);
     auto& vec = TFakeNodeWhiteboardService::BSControllerResponsePatterns[id];
-    vec.insert(vec.end(), failuresPattern.begin(), failuresPattern.end());
+    std::move(failuresPattern.begin(), failuresPattern.end(), std::back_inserter(vec));
 }
 
 void TCmsTestEnv::EnableNoisyBSCPipe() {
diff --git a/ydb/core/cms/cms_ut_common.h b/ydb/core/cms/cms_ut_common.h
index 9d4d81b9e5..88513c84f1 100644
--- a/ydb/core/cms/cms_ut_common.h
+++ b/ydb/core/cms/cms_ut_common.h
@@ -363,7 +363,7 @@ public:
         ui32 limit = 100,
         ui32 offset = 0);
 
-    void AddBSCFailures(const NCms::TPDiskID &id, TVector<bool> failuresPattern);
+    void AddBSCFailures(const NCms::TPDiskID &id, TVector<bool> &&failuresPattern);
 
     void EnableNoisyBSCPipe();
 
diff --git a/ydb/core/cms/sentinel.cpp b/ydb/core/cms/sentinel.cpp
index f4e779befd..e265df65f8 100644
--- a/ydb/core/cms/sentinel.cpp
+++ b/ydb/core/cms/sentinel.cpp
@@ -4,6 +4,7 @@
 
 #include <ydb/core/base/appdata.h>
 #include <ydb/core/base/counters.h>
+#include <ydb/core/base/tablet_pipe.h>
 #include <ydb/core/blobstorage/base/blobstorage_events.h>
 #include <ydb/core/node_whiteboard/node_whiteboard.h>
 #include <ydb/library/services/services.pb.h>
@@ -287,6 +288,18 @@ TClusterMap::TPDiskIDSet TGuardian::GetAllowedPDisks(const TClusterMap& all, TSt
     return result;
 }
 
+/// Misc
+
+IActor* CreateBSControllerPipe(TCmsStatePtr cmsState) {
+    auto domains = AppData()->DomainsInfo;
+    const ui32 domainUid = domains->GetDomainUidByTabletId(cmsState->CmsTabletId);
+    const ui64 bscId = MakeBSControllerID(domains->GetDefaultStateStorageGroup(domainUid));
+
+    NTabletPipe::TClientConfig config;
+    config.RetryPolicy = NTabletPipe::TClientRetryPolicy::WithRetries();
+    return NTabletPipe::CreateClient(cmsState->CmsActorId, bscId, config);
+}
+
 /// Actors
 
 template <typename TDerived>
@@ -375,7 +388,7 @@ class TConfigUpdater: public TUpdaterBase<TEvSentinel::TEvConfigUpdated, TConfig
             << ": attempt# " << SentinelState->ConfigUpdaterState.BSCAttempt);
 
         if (!CmsState->BSControllerPipe) {
-            CmsState->BSControllerPipe = this->Register(CreateBSCClientActor(CmsState));
+            CmsState->BSControllerPipe = this->Register(CreateBSControllerPipe(CmsState));
         }
 
         auto request = MakeHolder<TEvBlobStorage::TEvControllerConfigRequest>();
@@ -948,7 +961,7 @@ class TSentinel: public TActorBootstrapped<TSentinel> {
         }
 
         if (!CmsState->BSControllerPipe) {
-            CmsState->BSControllerPipe = Register(CreateBSCClientActor(CmsState));
+            CmsState->BSControllerPipe = Register(CreateBSControllerPipe(CmsState));
         }
 
         LOG_D("Change pdisk status"
@@ -1064,13 +1077,18 @@ class TSentinel: public TActorBootstrapped<TSentinel> {
         const auto& response = ev->Get()->Record.GetResponse();
 
         LOG_D("Handle TEvBlobStorage::TEvControllerConfigResponse"
-            << ": response# " << response.ShortDebugString());
+            << ": response# " << response.ShortDebugString()
+            << ", cookie# " << ev->Cookie);
 
         if (ev->Cookie != SentinelState->ChangeRequestId) {
+            LOG_W("Ignore TEvBlobStorage::TEvControllerConfigResponse"
+                << ": cookie# " << ev->Cookie
+                << ", expected# " << SentinelState->ChangeRequestId);
             return;
         }
 
         if (SentinelState->ChangeRequests.empty()) {
+            LOG_W("Ignore TEvBlobStorage::TEvControllerConfigResponse: empty queue");
             return;
         }
 
@@ -1212,16 +1230,6 @@ private:
 
 }; // TSentinel
 
-IActor* CreateBSCClientActor(const TCmsStatePtr& cmsState) {
-    auto domains = AppData()->DomainsInfo;
-    const ui32 domainUid = domains->GetDomainUidByTabletId(cmsState->CmsTabletId);
-    const ui64 bscId = MakeBSControllerID(domains->GetDefaultStateStorageGroup(domainUid));
-
-    NTabletPipe::TClientConfig config;
-    config.RetryPolicy = NTabletPipe::TClientRetryPolicy::WithRetries();
-    return NTabletPipe::CreateClient(cmsState->CmsActorId, bscId, config);
-}
-
 } // NSentinel
 
 IActor* CreateSentinel(TCmsStatePtr state) {
diff --git a/ydb/core/cms/sentinel.h b/ydb/core/cms/sentinel.h
index 9fa1e4e034..8aeac3cc53 100644
--- a/ydb/core/cms/sentinel.h
+++ b/ydb/core/cms/sentinel.h
@@ -14,10 +14,7 @@ struct TEvSentinel {
         EvUpdateState,
         EvStateUpdated,
 
-        EvStatusChanged,
-
         EvTimeout,
-
         EvBSCPipeDisconnected,
 
         EvEnd,
@@ -32,18 +29,6 @@ struct TEvSentinel {
     struct TEvStateUpdated: public TEventLocal<TEvStateUpdated, EvStateUpdated> {};
 
     struct TEvTimeout: public TEventLocal<TEvTimeout, EvTimeout> {};
-
-    struct TEvStatusChanged: public TEventLocal<TEvStatusChanged, EvStatusChanged> {
-        TPDiskID Id;
-        bool Success;
-
-        explicit TEvStatusChanged(const TPDiskID& id, bool success)
-            : Id(id)
-            , Success(success)
-        {
-        }
-    };
-
     struct TEvBSCPipeDisconnected: public TEventLocal<TEvBSCPipeDisconnected, EvBSCPipeDisconnected> {};
 
 }; // TEvSentinel
diff --git a/ydb/core/cms/sentinel_impl.h b/ydb/core/cms/sentinel_impl.h
index d8b034dd86..cf246f9564 100644
--- a/ydb/core/cms/sentinel_impl.h
+++ b/ydb/core/cms/sentinel_impl.h
@@ -173,6 +173,4 @@ private:
 
 }; // TGuardian
 
-IActor* CreateBSCClientActor(const TCmsStatePtr& cmsState);
-
 } // namespace NKikimr::NCms::NSentinel
diff --git a/ydb/core/cms/sentinel_ut.cpp b/ydb/core/cms/sentinel_ut.cpp
index 05410b5a90..6b6f6ff338 100644
--- a/ydb/core/cms/sentinel_ut.cpp
+++ b/ydb/core/cms/sentinel_ut.cpp
@@ -11,8 +11,7 @@
 #include <util/random/random.h>
 #include <util/string/builder.h>
 
-namespace NKikimr {
-namespace NCmsTest {
+namespace NKikimr::NCmsTest {
 
 static constexpr ui32 DefaultStateLimit = 5;
 static constexpr ui32 DefaultErrorStateLimit = 60;
@@ -38,7 +37,6 @@ constexpr NCms::EPDiskState FaultyStates[] = {
 };
 
 Y_UNIT_TEST_SUITE(TSentinelBaseTests) {
-
     using namespace NCms;
     using namespace NCms::NSentinel;
     using TPDiskID = NCms::TPDiskID;
@@ -313,11 +311,9 @@ Y_UNIT_TEST_SUITE(TSentinelBaseTests) {
         }
     }
 
-
 } // TSentinelBaseTests
 
 Y_UNIT_TEST_SUITE(TSentinelTests) {
-
     using namespace NCms;
     using namespace NCms::NSentinel;
     using TPDiskID = NCms::TPDiskID;
@@ -525,7 +521,6 @@ Y_UNIT_TEST_SUITE(TSentinelTests) {
 
     private:
         TCmsStatePtr State;
-        std::atomic<bool> NosiyBlobstoragePipe = false;
         TActorId Sentinel;
 
     }; // TTestEnv
@@ -617,12 +612,12 @@ Y_UNIT_TEST_SUITE(TSentinelTests) {
 
     Y_UNIT_TEST(BSControllerUnresponsive) {
         TTestEnv env(8, 4);
-
         env.EnableNoisyBSCPipe();
 
         const TPDiskID id1 = env.RandomPDiskID();
         const TPDiskID id2 = env.RandomPDiskID();
         const TPDiskID id3 = env.RandomPDiskID();
+
         for (size_t i = 0; i < sizeof(ErrorStates) / sizeof(ErrorStates[0]); ++i) {
             env.AddBSCFailures(id1, {false, true});
             env.AddBSCFailures(id2, {false, false, false, false, false, false});
@@ -640,6 +635,7 @@ Y_UNIT_TEST_SUITE(TSentinelTests) {
         const TPDiskID id1 = env.RandomPDiskID();
         const TPDiskID id2 = env.RandomPDiskID();
         const TPDiskID id3 = env.RandomPDiskID();
+
         for (size_t i = 0; i < sizeof(ErrorStates) / sizeof(ErrorStates[0]); ++i) {
             env.AddBSCFailures(id1, {true, false, false, true, false, false});
             // will fail for all requests assuming there is only 5 retries
@@ -652,7 +648,7 @@ Y_UNIT_TEST_SUITE(TSentinelTests) {
             env.SetPDiskState({id1, id2, id3}, NKikimrBlobStorage::TPDiskState::Normal, EPDiskStatus::ACTIVE);
         }
     }
+
 } // TSentinelTests
 
-} // NCmsTest
-} // NKikimr
+}