Improve code by adding TEntityId KIKIMR-14580

ref:8a48d3509ca8185c82fa86f82ef944853ad0f206

3 files changed, 92 insertions, 43 deletions

diff --git a/ydb/core/mind/bscontroller/group_layout_checker.cpp b/ydb/core/mind/bscontroller/group_layout_checker.cpp
index 3e4418ce731..cf947e16741 100644
--- a/ydb/core/mind/bscontroller/group_layout_checker.cpp
+++ b/ydb/core/mind/bscontroller/group_layout_checker.cpp
@@ -45,3 +45,5 @@ namespace NKikimr::NBsController {
     }
 
 } // NKikimr::NBsController
+
+Y_DECLARE_OUT_SPEC(, NKikimr::NBsController::NLayoutChecker::TEntityId, stream, value) { value.Output(stream); }
diff --git a/ydb/core/mind/bscontroller/group_layout_checker.h b/ydb/core/mind/bscontroller/group_layout_checker.h
index 5a317f59ddb..2319d83515c 100644
--- a/ydb/core/mind/bscontroller/group_layout_checker.h
+++ b/ydb/core/mind/bscontroller/group_layout_checker.h
@@ -6,39 +6,76 @@
 
 namespace NKikimr::NBsController {
 
-    struct TLayoutCheckResult {
-        std::vector<TVDiskIdShort> Candidates;
+    namespace NLayoutChecker {
 
-        explicit operator bool() const { // checks whether fail model is correct
-            return Candidates.empty();
-        }
-    };
+        struct TEntityId {
+            ui32 Value = ::Max<ui32>();
 
-    TLayoutCheckResult CheckGroupLayout(const TGroupGeometryInfo& geom, const THashMap<TVDiskIdShort, std::pair<TNodeLocation, TPDiskId>>& layout);
+        public:
+            bool operator ==(const TEntityId& other) const { return Value == other.Value; }
+            bool operator !=(const TEntityId& other) const { return Value != other.Value; }
+            bool operator < (const TEntityId& other) const { return Value <  other.Value; }
+            bool operator <=(const TEntityId& other) const { return Value <= other.Value; }
+            bool operator > (const TEntityId& other) const { return Value >  other.Value; }
+            bool operator >=(const TEntityId& other) const { return Value >= other.Value; }
+
+            size_t Index() const {
+                return Value;
+            }
+
+            size_t Hash() const {
+                return THash<ui32>()(Value);
+            }
+
+            static constexpr TEntityId Min() { return {.Value = 0}; };
+            static constexpr TEntityId Max() { return {.Value = ::Max<ui32>()}; };
+
+            TString ToString() const { return TStringBuilder() << Value; }
+            void Output(IOutputStream& s) const { s << Value; }
+
+        private:
+            friend class TDomainMapper;
+
+            static TEntityId SequentialValue(size_t index) {
+                return TEntityId{static_cast<ui32>(index)};
+            }
+        };
+
+    } // NLayoutChecker
+
+} // NKikimr::NBsController
+
+template<>
+struct THash<NKikimr::NBsController::NLayoutChecker::TEntityId> {
+    template<typename T>
+    size_t operator()(const T& id) const { return id.Hash(); }
+};
+
+namespace NKikimr::NBsController {
 
     namespace NLayoutChecker {
 
         class TDomainMapper {
-            std::unordered_map<TString, ui32> FailDomainId;
+            std::unordered_map<TString, TEntityId> FailDomainId;
 
         public:
-            ui32 operator ()(TString item) {
-                return FailDomainId.emplace(std::move(item), FailDomainId.size()).first->second;
+            TEntityId operator ()(TString item) {
+                return FailDomainId.emplace(std::move(item), TEntityId::SequentialValue(FailDomainId.size())).first->second;
             }
 
-            ui32 GetIdCount() const {
+            size_t GetIdCount() const {
                 return FailDomainId.size();
             }
         };
 
         struct TPDiskLayoutPosition {
-            ui32 RealmGroup = 0;
-            ui32 Realm = 0;
-            ui32 Domain = 0;
+            TEntityId RealmGroup;
+            TEntityId Realm;
+            TEntityId Domain;
 
             TPDiskLayoutPosition() = default;
 
-            TPDiskLayoutPosition(ui32 realmGroup, ui32 realm, ui32 domain)
+            TPDiskLayoutPosition(TEntityId realmGroup, TEntityId realm, TEntityId domain)
                 : RealmGroup(realmGroup)
                 , Realm(realm)
                 , Domain(domain)
@@ -121,15 +158,15 @@ namespace NKikimr::NBsController {
             const ui32 NumFailDomainsPerFailRealm;
 
             ui32 NumDisks = 0;
-            THashMap<ui32, ui32> NumDisksPerRealmGroup;
+            THashMap<TEntityId, ui32> NumDisksPerRealmGroup;
 
             TStackVec<ui32, 4> NumDisksInRealm;
-            TStackVec<THashMap<ui32, ui32>, 4> NumDisksPerRealm;
-            THashMap<ui32, ui32> NumDisksPerRealmTotal;
+            TStackVec<THashMap<TEntityId, ui32>, 4> NumDisksPerRealm;
+            THashMap<TEntityId, ui32> NumDisksPerRealmTotal;
 
             TStackVec<ui32, 32> NumDisksInDomain;
-            TStackVec<THashMap<ui32, ui32>, 32> NumDisksPerDomain;
-            THashMap<ui32, ui32> NumDisksPerDomainTotal;
+            TStackVec<THashMap<TEntityId, ui32>, 32> NumDisksPerDomain;
+            THashMap<TEntityId, ui32> NumDisksPerDomainTotal;
 
             TGroupLayout(ui32 numFailRealms, ui32 numFailDomainsPerFailRealm)
                 : NumFailDomainsPerFailRealm(numFailDomainsPerFailRealm)
@@ -174,4 +211,14 @@ namespace NKikimr::NBsController {
 
     } // NLayoutChecker
 
+    struct TLayoutCheckResult {
+        std::vector<TVDiskIdShort> Candidates;
+
+        explicit operator bool() const { // checks whether fail model is correct
+            return Candidates.empty();
+        }
+    };
+
+    TLayoutCheckResult CheckGroupLayout(const TGroupGeometryInfo& geom, const THashMap<TVDiskIdShort, std::pair<TNodeLocation, TPDiskId>>& layout);
+
 } // NKikimr::NBsController
diff --git a/ydb/core/mind/bscontroller/group_mapper.cpp b/ydb/core/mind/bscontroller/group_mapper.cpp
index 347a136a8a7..967aeafaa15 100644
--- a/ydb/core/mind/bscontroller/group_mapper.cpp
+++ b/ydb/core/mind/bscontroller/group_mapper.cpp
@@ -4,9 +4,9 @@
 
 namespace NKikimr::NBsController {
 
-    class TGroupMapper::TImpl : TNonCopyable {
-        using TPDiskLayoutPosition = NLayoutChecker::TPDiskLayoutPosition;
+    using namespace NLayoutChecker;
 
+    class TGroupMapper::TImpl : TNonCopyable {
         struct TPDiskInfo : TPDiskRecord {
             TPDiskLayoutPosition Position;
             bool Matching;
@@ -74,8 +74,8 @@ namespace NKikimr::NBsController {
             const bool RequireOperational;
             TForbiddenPDisks ForbiddenDisks;
             THashMap<ui32, unsigned> LocalityFactor;
-            NLayoutChecker::TGroupLayout GroupLayout;
-            std::optional<NLayoutChecker::TScore> BestScore;
+            TGroupLayout GroupLayout;
+            std::optional<TScore> BestScore;
 
             TAllocator(TImpl& self, const TGroupGeometryInfo& geom, i64 requiredSpace, bool requireOperational,
                     TForbiddenPDisks forbiddenDisks, const THashMap<TVDiskIdShort, TPDiskId>& replacedDisks)
@@ -198,7 +198,7 @@ namespace NKikimr::NBsController {
                         prev = position;
 
                         res.emplace_back(position, pdisk);
-                        ++numMatchingDisksInDomain[position.Domain];
+                        ++numMatchingDisksInDomain[position.Domain.Index()];
                     }
                 }
                 for (; realmGroupBegin < res.size(); ++realmGroupBegin) {
@@ -211,7 +211,7 @@ namespace NKikimr::NBsController {
                     res[domainBegin].second->SkipToNextDomain = res.size() - domainBegin;
                 }
                 for (const auto& [position, pdisk] : res) {
-                    pdisk->NumDomainMatchingDisks = numMatchingDisksInDomain[position.Domain];
+                    pdisk->NumDomainMatchingDisks = numMatchingDisksInDomain[position.Domain.Index()];
                 }
 
                 return std::move(res);
@@ -343,7 +343,7 @@ namespace NKikimr::NBsController {
                 using TNestedEntity = TAllocateWholeDomain;
 
                 static std::pair<TPDiskLayoutPosition, TPDiskLayoutPosition> MakeRange(const TPDiskLayoutPosition& x) {
-                    return {{x.RealmGroup, x.Realm, 0}, {x.RealmGroup, x.Realm, Max<ui32>()}};
+                    return {{x.RealmGroup, x.Realm, TEntityId::Min()}, {x.RealmGroup, x.Realm, TEntityId::Max()}};
                 }
             };
 
@@ -353,7 +353,7 @@ namespace NKikimr::NBsController {
                 using TNestedEntity = TAllocateWholeRealm;
 
                 static std::pair<TPDiskLayoutPosition, TPDiskLayoutPosition> MakeRange(const TPDiskLayoutPosition& x) {
-                    return {{x.RealmGroup, 0, 0}, {x.RealmGroup, Max<ui32>(), Max<ui32>()}};
+                    return {{x.RealmGroup, TEntityId::Min(), TEntityId::Min()}, {x.RealmGroup, TEntityId::Max(), TEntityId::Max()}};
                 }
             };
 
@@ -365,7 +365,7 @@ namespace NKikimr::NBsController {
                 const TDiskRange originalRange(range);
                 const size_t undoPosition = undo.GetPosition();
                 TPDiskLayoutPosition *prefix = nullptr;
-                ui32 currentEntityId = Max<ui32>();
+                TEntityId currentEntityId = TEntityId::Max();
                 for (ui32 index = 0, num = this->*T::EntityCount; index < num; ) {
                     // allocate nested entity
                     prefix = AllocateWholeEntity(typename T::TNestedEntity(), group, undo,
@@ -380,22 +380,22 @@ namespace NKikimr::NBsController {
                         ++index;
                     } else if (index) {
                         // disable just checked entity (to prevent its selection again)
-                        Y_VERIFY(currentEntityId != Max<ui32>());
-                        forbiddenEntities.Set(currentEntityId);
+                        Y_VERIFY(currentEntityId != TEntityId::Max());
+                        forbiddenEntities.Set(currentEntityId.Index());
                         // try another entity at this level
                         Revert(undo, group, undoPosition);
                         // revert original wide range and start from the beginning
                         range = originalRange;
                         index = 0;
-                        currentEntityId = Max<ui32>();
+                        currentEntityId = TEntityId::Max();
                     } else {
                         // no chance to allocate new entity, exit
                         return nullptr;
                     }
                 }
                 // disable filled entity from further selection
-                Y_VERIFY(prefix && currentEntityId != Max<ui32>());
-                forbiddenEntities.Set(currentEntityId);
+                Y_VERIFY(prefix && currentEntityId != TEntityId::Max());
+                forbiddenEntities.Set(currentEntityId.Index());
                 return prefix;
             }
 
@@ -418,14 +418,14 @@ namespace NKikimr::NBsController {
                 }
             }
 
-            NLayoutChecker::TScore CalculateBestScoreWithCache(const TGroup& group) {
+            TScore CalculateBestScoreWithCache(const TGroup& group) {
                 if (!BestScore) {
                     // find the worst disk from a position of layout correctness and use it as a milestone for other
                     // disks -- they can't be misplaced worse
-                    NLayoutChecker::TScore bestScore;
+                    TScore bestScore;
                     for (ui32 i = 0; i < GroupSize; ++i) {
                         if (TPDiskInfo *pdisk = group[i]; pdisk && !pdisk->Decommitted) {
-                            NLayoutChecker::TScore score = GroupLayout.GetCandidateScore(pdisk->Position, RealmIdx[i],
+                            TScore score = GroupLayout.GetCandidateScore(pdisk->Position, RealmIdx[i],
                                 DomainIdx[i]);
                             if (bestScore.BetterThan(score)) {
                                 bestScore = score;
@@ -440,7 +440,7 @@ namespace NKikimr::NBsController {
             template<typename TCallback>
             void FindMatchingDiskBasedOnScore(TCallback&& cb, const TGroup& group, ui32 failRealmIdx, ui32 failDomainIdx,
                     TDiskRange range, TDynBitMap& forbiddenEntities) {
-                NLayoutChecker::TScore bestScore = CalculateBestScoreWithCache(group);
+                TScore bestScore = CalculateBestScoreWithCache(group);
 
                 std::vector<TPDiskInfo*> candidates;
 
@@ -449,18 +449,18 @@ namespace NKikimr::NBsController {
 
                     if (!pdisk->Matching) {
                         continue;
-                    } else if (forbiddenEntities[position.RealmGroup]) {
+                    } else if (forbiddenEntities[position.RealmGroup.Index()]) {
                         range.first += Min<ui32>(std::distance(range.first, range.second), pdisk->SkipToNextRealmGroup - 1);
                         continue;
-                    } else if (forbiddenEntities[position.Realm]) {
+                    } else if (forbiddenEntities[position.Realm.Index()]) {
                         range.first += Min<ui32>(std::distance(range.first, range.second), pdisk->SkipToNextRealm - 1);
                         continue;
-                    } else if (forbiddenEntities[position.Domain]) {
+                    } else if (forbiddenEntities[position.Domain.Index()]) {
                         range.first += Min<ui32>(std::distance(range.first, range.second), pdisk->SkipToNextDomain - 1);
                         continue;
                     }
 
-                    NLayoutChecker::TScore score = GroupLayout.GetCandidateScore(position, failRealmIdx, failDomainIdx);
+                    TScore score = GroupLayout.GetCandidateScore(position, failRealmIdx, failDomainIdx);
                     if (score.BetterThan(bestScore)) {
                         candidates.clear();
                         candidates.push_back(pdisk);
@@ -529,7 +529,7 @@ namespace NKikimr::NBsController {
     private:
         const TGroupGeometryInfo Geom;
         const bool Randomize;
-        NLayoutChecker::TDomainMapper DomainMapper;
+        TDomainMapper DomainMapper;
         TPDisks PDisks;
         TPDiskByPosition PDiskByPosition;
         bool Dirty = false;