dont use removed portions before remove from local_db - prevent race … (

ydb-platform#12383)
zverevgeny · Jan 5, 2025 · 3a7f516 · 3a7f516
1 parent 82ba822
commit 3a7f516
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 22 deletions.
diff --git a/ydb/core/tx/columnshard/columnshard_impl.cpp b/ydb/core/tx/columnshard/columnshard_impl.cpp
@@ -1334,7 +1334,9 @@ class TPortionConstructorV2 {
     }
 
     NOlap::TPortionDataAccessor BuildAccessor() {
-        AFL_VERIFY(PortionInfo && Records && Indexes);
+        AFL_VERIFY(PortionInfo);
+        AFL_VERIFY(Records)("portion_id", PortionInfo->GetPortionId())("path_id", PortionInfo->GetPathId());
+        AFL_VERIFY(Indexes)("portion_id", PortionInfo->GetPortionId())("path_id", PortionInfo->GetPathId());
         std::vector<NOlap::TColumnChunkLoadContextV1> records = Records->BuildRecordsV1();
         return NOlap::TPortionAccessorConstructor::BuildForLoading(std::move(PortionInfo), std::move(records), std::move(*Indexes));
     }
@@ -1418,17 +1420,13 @@ class TTxAskPortionChunks: public TTransactionBase<TColumnShard> {
                 auto p = i.second.back();
                 TPortionConstructorV2 constructor(p);
                 {
-                    auto rowset = db.Table<NColumnShard::Schema::IndexColumnsV2>().Prefix(p->GetPathId(), p->GetPortionId()).Select();
+                    auto rowset = db.Table<NColumnShard::Schema::IndexColumnsV2>().Key(p->GetPathId(), p->GetPortionId()).Select();
                     if (!rowset.IsReady()) {
                         return false;
                     }
-                    while (!rowset.EndOfSet()) {
-                        NOlap::TColumnChunkLoadContextV2 info(rowset);
-                        constructor.SetRecords(std::move(info));
-                        if (!rowset.Next()) {
-                            return false;
-                        }
-                    }
+                    AFL_VERIFY(!rowset.EndOfSet())("path_id", p->GetPathId())("portion_id", p->GetPortionId())("debug", p->DebugString(true));
+                    NOlap::TColumnChunkLoadContextV2 info(rowset);
+                    constructor.SetRecords(std::move(info));
                 }
                 std::vector<NOlap::TIndexChunkLoadContext> indexes;
                 if (p->GetSchema(Self->GetIndexAs<NOlap::TColumnEngineForLogs>().GetVersionedIndex())->GetIndexesCount()) {

diff --git a/ydb/core/tx/columnshard/engines/reader/sys_view/abstract/filler.cpp b/ydb/core/tx/columnshard/engines/reader/sys_view/abstract/filler.cpp
@@ -23,7 +23,7 @@ NKikimr::TConclusionStatus TMetadataFromStore::DoFillMetadata(const NColumnShard
         auto pathInfos = logsIndex->GetTables(fromPathId, toPathId);
         for (auto&& pathInfo : pathInfos) {
             if (pathIds.emplace(pathInfo->GetPathId()).second) {
-                metadata->IndexGranules.emplace_back(BuildGranuleView(*pathInfo, metadata->IsDescSorted()));
+                metadata->IndexGranules.emplace_back(BuildGranuleView(*pathInfo, metadata->IsDescSorted(), metadata->GetRequestSnapshot()));
             }
         }
     }
@@ -52,7 +52,7 @@ NKikimr::TConclusionStatus TMetadataFromTable::DoFillMetadata(const NColumnShard
             if (!pathInfo) {
                 continue;
             }
-            metadata->IndexGranules.emplace_back(BuildGranuleView(*pathInfo, metadata->IsDescSorted()));
+            metadata->IndexGranules.emplace_back(BuildGranuleView(*pathInfo, metadata->IsDescSorted(), metadata->GetRequestSnapshot()));
             break;
         }
     }

diff --git a/ydb/core/tx/columnshard/engines/reader/sys_view/abstract/filler.h b/ydb/core/tx/columnshard/engines/reader/sys_view/abstract/filler.h
@@ -9,8 +9,8 @@ class IMetadataFiller {
 private:
     virtual TConclusionStatus DoFillMetadata(const NColumnShard::TColumnShard* shard, const std::shared_ptr<TReadMetadataBase>& metadata, const TReadDescription& read) const = 0;
 
-    virtual NAbstract::TGranuleMetaView DoBuildGranuleView(const TGranuleMeta& granule, const bool reverse) const {
-        return NAbstract::TGranuleMetaView(granule, reverse);
+    virtual NAbstract::TGranuleMetaView DoBuildGranuleView(const TGranuleMeta& granule, const bool reverse, const TSnapshot& reqSnapshot) const {
+        return NAbstract::TGranuleMetaView(granule, reverse, reqSnapshot);
     }
 public:
     virtual ~IMetadataFiller() = default;
@@ -19,8 +19,8 @@ class IMetadataFiller {
         return DoFillMetadata(shard, metadata, read);
     }
 
-    NAbstract::TGranuleMetaView BuildGranuleView(const TGranuleMeta& granule, const bool reverse) const {
-        return DoBuildGranuleView(granule, reverse);
+    NAbstract::TGranuleMetaView BuildGranuleView(const TGranuleMeta& granule, const bool reverse, const TSnapshot& reqSnapshot) const {
+        return DoBuildGranuleView(granule, reverse, reqSnapshot);
     }
 
 };

diff --git a/ydb/core/tx/columnshard/engines/reader/sys_view/abstract/granule_view.h b/ydb/core/tx/columnshard/engines/reader/sys_view/abstract/granule_view.h
@@ -11,10 +11,13 @@ class TGranuleMetaView {
     YDB_READONLY_DEF(TPortions, Portions);
     YDB_READONLY_DEF(std::vector<NStorageOptimizer::TTaskDescription>, OptimizerTasks);
 public:
-    TGranuleMetaView(const TGranuleMeta& granule, const bool reverse)
+    TGranuleMetaView(const TGranuleMeta& granule, const bool reverse, const TSnapshot& reqSnapshot)
         : PathId(granule.GetPathId())
     {
         for (auto&& i : granule.GetPortions()) {
+            if (i.second->IsRemovedFor(reqSnapshot)) {
+                continue;
+            }
             Portions.emplace_back(i.second);
         }
 

diff --git a/ydb/core/tx/columnshard/engines/reader/sys_view/chunks/chunks.cpp b/ydb/core/tx/columnshard/engines/reader/sys_view/chunks/chunks.cpp
@@ -128,7 +128,7 @@ std::vector<std::pair<TString, NKikimr::NScheme::TTypeInfo>> TReadStatsMetadata:
     return GetColumns(TStatsIterator::StatsSchema, TStatsIterator::StatsSchema.KeyColumns);
 }
 
-std::shared_ptr<NKikimr::NOlap::NReader::NSysView::NAbstract::TReadStatsMetadata> TConstructor::BuildMetadata(
+std::shared_ptr<NAbstract::TReadStatsMetadata> TConstructor::BuildMetadata(
     const NColumnShard::TColumnShard* self, const TReadDescription& read) const {
     auto* index = self->GetIndexOptional();
     return std::make_shared<TReadStatsMetadata>(index ? index->CopyVersionedIndexPtr() : nullptr, self->TabletID(),

diff --git a/ydb/core/tx/columnshard/engines/reader/sys_view/chunks/chunks.h b/ydb/core/tx/columnshard/engines/reader/sys_view/chunks/chunks.h
@@ -27,7 +27,7 @@ class TReadStatsMetadata: public NAbstract::TReadStatsMetadata {
 public:
     using TBase::TBase;
 
-    virtual std::unique_ptr<TScanIteratorBase> StartScan(const std::shared_ptr<TReadContext>& /*readContext*/) const override;
+    virtual std::unique_ptr<TScanIteratorBase> StartScan(const std::shared_ptr<TReadContext>& readContext) const override;
     virtual std::vector<std::pair<TString, NScheme::TTypeInfo>> GetKeyYqlSchema() const override;
 };
 

diff --git a/ydb/core/tx/columnshard/engines/reader/sys_view/optimizer/optimizer.h b/ydb/core/tx/columnshard/engines/reader/sys_view/optimizer/optimizer.h
@@ -41,8 +41,8 @@ class TStatsIterator : public NAbstract::TStatsIterator<NKikimr::NSysView::Schem
 
 class TMetadataFromStore: public NAbstract::TMetadataFromStore {
 protected:
-    virtual NAbstract::TGranuleMetaView DoBuildGranuleView(const TGranuleMeta& granule, const bool reverse) const override {
-        NAbstract::TGranuleMetaView result(granule, reverse);
+    virtual NAbstract::TGranuleMetaView DoBuildGranuleView(const TGranuleMeta& granule, const bool reverse, const TSnapshot& reqSnapshot) const override {
+        NAbstract::TGranuleMetaView result(granule, reverse, reqSnapshot);
         result.FillOptimizerTasks(granule, reverse);
         return result;
     }
@@ -52,8 +52,9 @@ class TMetadataFromStore: public NAbstract::TMetadataFromStore {
 
 class TMetadataFromTable: public NAbstract::TMetadataFromTable {
 protected:
-    virtual NAbstract::TGranuleMetaView DoBuildGranuleView(const TGranuleMeta& granule, const bool reverse) const override {
-        NAbstract::TGranuleMetaView result(granule, reverse);
+    virtual NAbstract::TGranuleMetaView DoBuildGranuleView(
+        const TGranuleMeta& granule, const bool reverse, const TSnapshot& reqSnapshot) const override {
+        NAbstract::TGranuleMetaView result(granule, reverse, reqSnapshot);
         result.FillOptimizerTasks(granule, reverse);
         return result;
     }