From 3a7f516d12275a709135f56b5c5263594c3d9461 Mon Sep 17 00:00:00 2001 From: ivanmorozov333 Date: Mon, 9 Dec 2024 08:51:06 +0300 Subject: [PATCH] =?UTF-8?q?dont=20use=20removed=20portions=20before=20remo?= =?UTF-8?q?ve=20from=20local=5Fdb=20-=20prevent=20race=20=E2=80=A6=20(#123?= =?UTF-8?q?83)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ydb/core/tx/columnshard/columnshard_impl.cpp | 16 +++++++--------- .../engines/reader/sys_view/abstract/filler.cpp | 4 ++-- .../engines/reader/sys_view/abstract/filler.h | 8 ++++---- .../reader/sys_view/abstract/granule_view.h | 5 ++++- .../engines/reader/sys_view/chunks/chunks.cpp | 2 +- .../engines/reader/sys_view/chunks/chunks.h | 2 +- .../reader/sys_view/optimizer/optimizer.h | 9 +++++---- 7 files changed, 24 insertions(+), 22 deletions(-) diff --git a/ydb/core/tx/columnshard/columnshard_impl.cpp b/ydb/core/tx/columnshard/columnshard_impl.cpp index 5e1244cf1bf1..3cc0a1464d10 100644 --- 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 records = Records->BuildRecordsV1(); return NOlap::TPortionAccessorConstructor::BuildForLoading(std::move(PortionInfo), std::move(records), std::move(*Indexes)); } @@ -1418,17 +1420,13 @@ class TTxAskPortionChunks: public TTransactionBase { auto p = i.second.back(); TPortionConstructorV2 constructor(p); { - auto rowset = db.Table().Prefix(p->GetPathId(), p->GetPortionId()).Select(); + auto rowset = db.Table().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 indexes; if (p->GetSchema(Self->GetIndexAs().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 index 2a23b12c3fae..cd65ff991335 100644 --- 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 index 5b1199496819..8caecc01f5b7 100644 --- 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& 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 index 5f18d8b9ece2..97144d686c96 100644 --- 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, 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 index 55811c2b65df..5634f37fcbfa 100644 --- 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> TReadStatsMetadata: return GetColumns(TStatsIterator::StatsSchema, TStatsIterator::StatsSchema.KeyColumns); } -std::shared_ptr TConstructor::BuildMetadata( +std::shared_ptr TConstructor::BuildMetadata( const NColumnShard::TColumnShard* self, const TReadDescription& read) const { auto* index = self->GetIndexOptional(); return std::make_shared(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 index 1808888ff6be..7e9d0b2552aa 100644 --- 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 StartScan(const std::shared_ptr& /*readContext*/) const override; + virtual std::unique_ptr StartScan(const std::shared_ptr& readContext) const override; virtual std::vector> 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 index 6c69bece0fd0..d31545fe619c 100644 --- 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