correction (ydb-platform#3958)

GrigoriyPA · Apr 21, 2024 · 713e5bb · 713e5bb
1 parent 23734ba
commit 713e5bb
Show file tree

Hide file tree

Showing 10 changed files with 66 additions and 62 deletions.
diff --git a/ydb/core/tx/schemeshard/olap/manager/manager.cpp b/ydb/core/tx/schemeshard/olap/manager/manager.cpp
@@ -7,7 +7,7 @@ void TTablesStorage::OnAddObject(const TPathId& pathId, TColumnTableInfo::TPtr o
     if (!!tieringId) {
         PathsByTieringId[tieringId].emplace(pathId);
     }
-    for (auto&& s : object->ColumnShards) {
+    for (auto&& s : object->GetColumnShards()) {
         TablesByShard[s].AddId(pathId);
     }
 }
@@ -24,7 +24,7 @@ void TTablesStorage::OnRemoveObject(const TPathId& pathId, TColumnTableInfo::TPt
             PathsByTieringId.erase(it);
         }
     }
-    for (auto&& s : object->ColumnShards) {
+    for (auto&& s : object->GetColumnShards()) {
         TablesByShard[s].RemoveId(pathId);
     }
 }
@@ -112,21 +112,6 @@ void TTablesStorage::TTableExtractedGuard::UseAlterDataVerified() {
     Object = alterInfo;
 }
 
-bool TTablesStorage::TTableCreateOperator::InitShardingTablets(const TColumnTablesLayout& currentLayout, const ui32 shardsCount, TOlapStoreInfo::ILayoutPolicy::TPtr layoutPolicy, bool& isNewGroup) const {
-    if (!layoutPolicy->Layout(currentLayout, shardsCount, Object->ColumnShards, isNewGroup)) {
-        ALS_ERROR(NKikimrServices::FLAT_TX_SCHEMESHARD) << "cannot layout new table with " << shardsCount << " shards";
-        return false;
-    }
-    Object->Sharding.SetVersion(1);
-
-    Object->Sharding.MutableColumnShards()->Clear();
-    Object->Sharding.MutableColumnShards()->Reserve(Object->ColumnShards.size());
-    for (ui64 columnShard : Object->ColumnShards) {
-        Object->Sharding.AddColumnShards(columnShard);
-    }
-    return true;
-}
-
 std::unordered_set<TPathId> TTablesStorage::GetAllPathIds() const {
     std::unordered_set<TPathId> result;
     for (const auto& [pathId, _] : Tables) {

diff --git a/ydb/core/tx/schemeshard/olap/manager/manager.h b/ydb/core/tx/schemeshard/olap/manager/manager.h
@@ -48,7 +48,6 @@ class TTablesStorage {
         using TBase = TTableReadGuard;
     public:
         using TBase::TBase;
-        bool InitShardingTablets(const TColumnTablesLayout& currentLayout, const ui32 shardsCount, TOlapStoreInfo::ILayoutPolicy::TPtr layoutPolicy, bool& isNewGroup) const;
     };
 
     class TTableCreatedGuard: public TTableCreateOperator, TMoveOnly {

diff --git a/ydb/core/tx/schemeshard/olap/operations/alter_table.cpp b/ydb/core/tx/schemeshard/olap/operations/alter_table.cpp
@@ -577,7 +577,7 @@ class TAlterColumnTable: public TSubOperation {
         txState.State = TTxState::ConfigureParts;
 
         // TODO: we need to know all shards where this table is currently active
-        for (ui64 columnShardId : tableInfo->ColumnShards) {
+        for (ui64 columnShardId : tableInfo->GetColumnShards()) {
             auto tabletId = TTabletId(columnShardId);
             auto shardIdx = context.SS->TabletIdToShardIdx.at(tabletId);
 

diff --git a/ydb/core/tx/schemeshard/olap/operations/create_table.cpp b/ydb/core/tx/schemeshard/olap/operations/create_table.cpp
@@ -270,7 +270,7 @@ class TConfigureParts: public TSubOperationState {
 
         txState->ClearShardsInProgress();
 
-        Y_ABORT_UNLESS(tableInfo->ColumnShards.empty() || tableInfo->OwnedColumnShards.empty());
+        Y_ABORT_UNLESS(tableInfo->GetColumnShards().empty() || tableInfo->OwnedColumnShards.empty());
 
         TString columnShardTxBody;
         auto seqNo = context.SS->StartRound(*txState);
@@ -280,7 +280,7 @@ class TConfigureParts: public TSubOperationState {
         {
             NKikimrTxColumnShard::TCreateTable* create{};
             if (tableInfo->IsStandalone()) {
-                Y_ABORT_UNLESS(tableInfo->ColumnShards.empty());
+                Y_ABORT_UNLESS(tableInfo->GetColumnShards().empty());
                 Y_ABORT_UNLESS(tableInfo->Description.HasSchema());
 
                 auto* init = tx.MutableInitShard();
@@ -392,11 +392,7 @@ class TPropose: public TSubOperationState {
 
         auto table = context.SS->ColumnTables.TakeAlterVerified(pathId);
         if (table->IsStandalone()) {
-            Y_ABORT_UNLESS(table->ColumnShards.empty());
-            auto currentLayout = TColumnTablesLayout::BuildTrivial(TColumnTablesLayout::ShardIdxToTabletId(table->OwnedColumnShards, *context.SS));
-            auto layoutPolicy = std::make_shared<TOlapStoreInfo::TMinimalTablesCountLayout>();
-            bool isNewGroup;
-            Y_ABORT_UNLESS(table.InitShardingTablets(currentLayout, table->OwnedColumnShards.size(), layoutPolicy, isNewGroup));
+            table->SetColumnShards(TColumnTablesLayout::ShardIdxToTabletId(table->OwnedColumnShards, *context.SS));
         }
 
         context.SS->PersistColumnTableAlterRemove(db, pathId);
@@ -584,7 +580,7 @@ class TCreateColumnTable: public TSubOperation {
         const auto acceptExisted = !Transaction.GetFailOnExist();
         const TString& parentPathStr = Transaction.GetWorkingDir();
 
-        // Copy CreateColumnTable for changes. Update defaut sharding if not set.
+        // Copy CreateColumnTable for changes. Update default sharding if not set.
         auto createDescription = Transaction.GetCreateColumnTable();
         if (!createDescription.HasColumnShardCount()) {
             createDescription.SetColumnShardCount(TTableConstructorBase::DEFAULT_SHARDS_COUNT);
@@ -705,12 +701,12 @@ class TCreateColumnTable: public TSubOperation {
             tableInfo = tableConstructor.BuildTableInfo(errors);
             if (tableInfo) {
                 auto layoutPolicy = storeInfo->GetTablesLayoutPolicy();
-                auto currentLayout = context.SS->ColumnTables.GetTablesLayout(
-                    TColumnTablesLayout::ShardIdxToTabletId(storeInfo->GetColumnShards(), *context.SS));
-                TTablesStorage::TTableCreateOperator createOperator(tableInfo);
-                if (!createOperator.InitShardingTablets(currentLayout, shardsCount, layoutPolicy, needUpdateObject)) {
-                    result->SetError(NKikimrScheme::StatusPreconditionFailed,
-                        "cannot layout table by shards");
+                auto currentLayout = context.SS->ColumnTables.GetTablesLayout(TColumnTablesLayout::ShardIdxToTabletId(storeInfo->GetColumnShards(), *context.SS));
+                auto layoutConclusion = layoutPolicy->Layout(currentLayout, shardsCount);
+                needUpdateObject = layoutConclusion->GetIsNewGroup();
+                tableInfo->SetColumnShards(std::move(layoutConclusion->MutableTabletIds()));
+                if (layoutConclusion.IsFail()) {
+                    result->SetError(NKikimrScheme::StatusPreconditionFailed, layoutConclusion.GetErrorMessage());
                     return result;
                 }
             }
@@ -746,9 +742,9 @@ class TCreateColumnTable: public TSubOperation {
             auto olapStorePath = parentPath.FindOlapStore();
 
             txState.State = TTxState::ConfigureParts;
-            txState.Shards.reserve(tableInfo->ColumnShards.size());
+            txState.Shards.reserve(tableInfo->GetColumnShards().size());
 
-            for (ui64 columnShardId : tableInfo->ColumnShards) {
+            for (ui64 columnShardId : tableInfo->GetColumnShards()) {
                 auto tabletId = TTabletId(columnShardId);
                 auto shardIdx = context.SS->TabletIdToShardIdx.at(tabletId);
                 TShardInfo& shardInfo = context.SS->ShardInfos.at(shardIdx);

diff --git a/ydb/core/tx/schemeshard/olap/operations/drop_table.cpp b/ydb/core/tx/schemeshard/olap/operations/drop_table.cpp
@@ -452,7 +452,7 @@ class TDropColumnTable: public TSubOperation {
             context.SS->PersistLastTxId(db, storePath.Base());
 
             // TODO: we need to know all shards where this table has ever been created
-            for (ui64 columnShardId : tableInfo->ColumnShards) {
+            for (ui64 columnShardId : tableInfo->GetColumnShards()) {
                 auto tabletId = TTabletId(columnShardId);
                 auto shardIdx = context.SS->TabletIdToShardIdx.at(tabletId);
 

diff --git a/ydb/core/tx/schemeshard/olap/store/store.cpp b/ydb/core/tx/schemeshard/olap/store/store.cpp
@@ -2,33 +2,30 @@
 
 namespace NKikimr::NSchemeShard {
 
-bool TOlapStoreInfo::ILayoutPolicy::Layout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount, std::vector<ui64>& result, bool& isNewGroup) const {
-    if (!DoLayout(currentLayout, shardsCount, result, isNewGroup)) {
-        return false;
+TConclusion<TOlapStoreInfo::TLayoutInfo> TOlapStoreInfo::ILayoutPolicy::Layout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount) const {
+    auto result = DoLayout(currentLayout, shardsCount);
+    if (result.IsFail()) {
+        return result;
     }
-    Y_ABORT_UNLESS(result.size() == shardsCount);
-    return true;
+    AFL_VERIFY(result->GetTabletIds().size() == shardsCount);
+    return result;
 }
 
-bool TOlapStoreInfo::TIdentityGroupsLayout::DoLayout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount, std::vector<ui64>& result, bool& isNewGroup) const {
-    isNewGroup = false;
+TConclusion<TOlapStoreInfo::TLayoutInfo> TOlapStoreInfo::TIdentityGroupsLayout::DoLayout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount) const {
     for (auto&& i : currentLayout.GetGroups()) {
         if (i.GetTableIds().Size() == 0 && i.GetShardIds().Size() >= shardsCount) {
-            result = i.GetShardIds().GetIdsVector(shardsCount);
-            isNewGroup = true;
-            return true;
+            return TOlapStoreInfo::TLayoutInfo(i.GetShardIds().GetIdsVector(shardsCount), true);
         }
         if (i.GetShardIds().Size() != shardsCount) {
             continue;
         }
-        result = i.GetShardIds().GetIdsVector();
-        return true;
+        return TOlapStoreInfo::TLayoutInfo(i.GetShardIds().GetIdsVector(), false);
     }
-    return false;
+    return TConclusionStatus::Fail("cannot find appropriate group for " + ::ToString(shardsCount) + " shards");
 }
 
-bool TOlapStoreInfo::TMinimalTablesCountLayout::DoLayout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount, std::vector<ui64>& result, bool& isNewGroup) const {
-    isNewGroup = true;
+TConclusion<TOlapStoreInfo::TLayoutInfo> TOlapStoreInfo::TMinimalTablesCountLayout::DoLayout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount) const {
+    bool isNewGroup = true;
     std::vector<ui64> resultLocal;
     for (auto&& i : currentLayout.GetGroups()) {
         if (i.GetTableIds().Size() > 0) {
@@ -37,12 +34,11 @@ bool TOlapStoreInfo::TMinimalTablesCountLayout::DoLayout(const TColumnTablesLayo
         for (auto&& s : i.GetShardIds()) {
             resultLocal.emplace_back(s);
             if (resultLocal.size() == shardsCount) {
-                std::swap(result, resultLocal);
-                return true;
+                return TOlapStoreInfo::TLayoutInfo(std::move(resultLocal), isNewGroup);
             }
         }
     }
-    return false;
+    return TConclusionStatus::Fail("cannot find appropriate group for " + ::ToString(shardsCount) + " shards");
 }
 
 TOlapStoreInfo::TOlapStoreInfo(

diff --git a/ydb/core/tx/schemeshard/olap/store/store.h b/ydb/core/tx/schemeshard/olap/store/store.h
@@ -19,23 +19,36 @@ struct TOlapStoreInfo {
 public:
     using TPtr = std::shared_ptr<TOlapStoreInfo>;
 
+    class TLayoutInfo {
+    private:
+        YDB_ACCESSOR_DEF(std::vector<ui64>, TabletIds);
+        YDB_READONLY(bool, IsNewGroup, false);
+    public:
+        TLayoutInfo(std::vector<ui64>&& ids, const bool isNewGroup)
+            : TabletIds(std::move(ids))
+            , IsNewGroup(isNewGroup)
+        {
+
+        }
+    };
+
     class ILayoutPolicy {
     protected:
-        virtual bool DoLayout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount, std::vector<ui64>& result, bool& isNewGroup) const = 0;
+        virtual TConclusion<TLayoutInfo> DoLayout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount) const = 0;
     public:
         using TPtr = std::shared_ptr<ILayoutPolicy>;
         virtual ~ILayoutPolicy() = default;
-        bool Layout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount, std::vector<ui64>& result, bool& isNewGroup) const;
+        TConclusion<TLayoutInfo> Layout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount) const;
     };
 
     class TMinimalTablesCountLayout: public ILayoutPolicy {
     protected:
-        virtual bool DoLayout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount, std::vector<ui64>& result, bool& isNewGroup) const override;
+        virtual TConclusion<TLayoutInfo> DoLayout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount) const override;
     };
 
     class TIdentityGroupsLayout: public ILayoutPolicy {
     protected:
-        virtual bool DoLayout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount, std::vector<ui64>& result, bool& isNewGroup) const override;
+        virtual TConclusion<TLayoutInfo> DoLayout(const TColumnTablesLayout& currentLayout, const ui32 shardsCount) const override;
     };
 
     TPtr AlterData;

diff --git a/ydb/core/tx/schemeshard/olap/table/table.h b/ydb/core/tx/schemeshard/olap/table/table.h
@@ -6,20 +6,35 @@
 namespace NKikimr::NSchemeShard {
 
 struct TColumnTableInfo {
+private:
+    YDB_READONLY_DEF(std::vector<ui64>, ColumnShards);
+public:
     using TPtr = std::shared_ptr<TColumnTableInfo>;
 
     ui64 AlterVersion = 0;
     TPtr AlterData;
 
+    void SetColumnShards(std::vector<ui64>&& columnShards) {
+        AFL_VERIFY(ColumnShards.empty());
+        ColumnShards = std::move(columnShards);
+
+        Sharding.SetVersion(1);
+
+        Sharding.MutableColumnShards()->Clear();
+        Sharding.MutableColumnShards()->Reserve(ColumnShards.size());
+        for (ui64 columnShard : ColumnShards) {
+            Sharding.AddColumnShards(columnShard);
+        }
+    }
+
     NKikimrSchemeOp::TColumnTableDescription Description;
     NKikimrSchemeOp::TColumnTableSharding Sharding;
     TMaybe<NKikimrSchemeOp::TColumnStoreSharding> StandaloneSharding;
     TMaybe<NKikimrSchemeOp::TAlterColumnTable> AlterBody;
 
     TMaybe<TPathId> OlapStorePathId; // PathId of the table store
 
-    TVector<ui64> ColumnShards; // Current list of column shards
-    TVector<TShardIdx> OwnedColumnShards;
+    std::vector<TShardIdx> OwnedColumnShards;
     TAggregatedStats Stats;
 
     TColumnTableInfo() = default;

diff --git a/ydb/core/tx/schemeshard/schemeshard_impl.cpp b/ydb/core/tx/schemeshard/schemeshard_impl.cpp
@@ -6682,7 +6682,7 @@ bool TSchemeShard::FillUniformPartitioning(TVector<TString>& rangeEnds, ui32 key
     return true;
 }
 
-void TSchemeShard::SetPartitioning(TPathId pathId, const TVector<TShardIdx>& partitioning) {
+void TSchemeShard::SetPartitioning(TPathId pathId, const std::vector<TShardIdx>& partitioning) {
     if (AppData()->FeatureFlags.GetEnableSystemViews()) {
         TVector<std::pair<ui64, ui64>> shardIndices;
         shardIndices.reserve(partitioning.size());

diff --git a/ydb/core/tx/schemeshard/schemeshard_impl.h b/ydb/core/tx/schemeshard/schemeshard_impl.h
@@ -597,7 +597,7 @@ class TSchemeShard
 
     void DoShardsDeletion(const THashSet<TShardIdx>& shardIdx, const TActorContext& ctx);
 
-    void SetPartitioning(TPathId pathId, const TVector<TShardIdx>& partitioning);
+    void SetPartitioning(TPathId pathId, const std::vector<TShardIdx>& partitioning);
     void SetPartitioning(TPathId pathId, TOlapStoreInfo::TPtr storeInfo);
     void SetPartitioning(TPathId pathId, TColumnTableInfo::TPtr tableInfo);
     void SetPartitioning(TPathId pathId, TTableInfo::TPtr tableInfo, TVector<TTableShardInfo>&& newPartitioning);