Skip to content

Commit cf86700

Browse files
kungablinkov
authored andcommitted
Fix compacted pages offload race (#15377)
1 parent 1141f5d commit cf86700

13 files changed

+366
-93
lines changed

ydb/core/tablet_flat/flat_boot_bundle.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ namespace NBoot {
107107
void TryFinalize()
108108
{
109109
if (!LeftReads) {
110-
for (auto req : Loader->Run(false)) {
110+
for (auto req : Loader->Run({.PreloadIndex = true, .PreloadData = false})) {
111111
LeftReads += Logic->LoadPages(this, req);
112112
}
113113
}

ydb/core/tablet_flat/flat_executor.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,7 +1286,7 @@ bool TExecutor::PrepareExternalPart(TPendingPartSwitch &partSwitch, TPendingPart
12861286
}
12871287

12881288
if (auto* stage = bundle.GetStage<TPendingPartSwitch::TLoaderStage>()) {
1289-
if (auto fetch = stage->Loader.Run(PreloadTablesData.contains(partSwitch.TableId))) {
1289+
if (auto fetch = stage->Loader.Run({.PreloadIndex = true, .PreloadData = PreloadTablesData.contains(partSwitch.TableId)})) {
12901290
Y_ABORT_UNLESS(fetch.size() == 1, "Cannot handle loads from more than one page collection");
12911291

12921292
for (auto req : fetch) {
@@ -2978,9 +2978,9 @@ void TExecutor::Handle(NSharedCache::TEvResult::TPtr &ev) {
29782978
void TExecutor::Handle(NSharedCache::TEvUpdated::TPtr &ev) {
29792979
const auto *msg = ev->Get();
29802980

2981-
for (auto &kv : msg->Actions) {
2981+
for (auto &kv : msg->DroppedPages) {
29822982
if (auto *info = PrivatePageCache->Info(kv.first)) {
2983-
for (ui32 pageId : kv.second.Dropped) {
2983+
for (ui32 pageId : kv.second) {
29842984
PrivatePageCache->DropSharedBody(info, pageId);
29852985
}
29862986
}

ydb/core/tablet_flat/flat_executor_ut.cpp

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6288,13 +6288,22 @@ Y_UNIT_TEST_SUITE(TFlatTableExecutor_StickyPages) {
62886288
env->Send(MakeSharedPageCacheId(), TActorId{}, new NMemory::TEvConsumerLimit(0));
62896289
}
62906290

6291+
void SetupEnvironment(TMyEnvBase &env, std::optional<bool> bTreeIndex = {}) {
6292+
env->SetLogPriority(NKikimrServices::TABLET_SAUSAGECACHE, NActors::NLog::PRI_TRACE);
6293+
env->SetLogPriority(NKikimrServices::TABLET_EXECUTOR, NActors::NLog::PRI_TRACE);
6294+
6295+
if (bTreeIndex.has_value()) {
6296+
auto &appData = env->GetAppData();
6297+
appData.FeatureFlags.SetEnableLocalDBBtreeIndex(bTreeIndex.value());
6298+
appData.FeatureFlags.SetEnableLocalDBFlatIndex(!bTreeIndex.value());
6299+
}
6300+
}
6301+
62916302
Y_UNIT_TEST(TestNonSticky_FlatIndex) {
62926303
TMyEnvBase env;
62936304
TRowsModel rows;
62946305

6295-
auto &appData = env->GetAppData();
6296-
appData.FeatureFlags.SetEnableLocalDBBtreeIndex(false);
6297-
appData.FeatureFlags.SetEnableLocalDBFlatIndex(true);
6306+
SetupEnvironment(env, false);
62986307

62996308
env.FireDummyTablet(ui32(NFake::TDummy::EFlg::Comp));
63006309
ZeroSharedCache(env);
@@ -6329,9 +6338,7 @@ Y_UNIT_TEST_SUITE(TFlatTableExecutor_StickyPages) {
63296338
TMyEnvBase env;
63306339
TRowsModel rows;
63316340

6332-
auto &appData = env->GetAppData();
6333-
appData.FeatureFlags.SetEnableLocalDBBtreeIndex(true);
6334-
appData.FeatureFlags.SetEnableLocalDBFlatIndex(false);
6341+
SetupEnvironment(env, true);
63356342

63366343
env.FireDummyTablet(ui32(NFake::TDummy::EFlg::Comp));
63376344
ZeroSharedCache(env);
@@ -6366,6 +6373,8 @@ Y_UNIT_TEST_SUITE(TFlatTableExecutor_StickyPages) {
63666373
TMyEnvBase env;
63676374
TRowsModel rows;
63686375

6376+
SetupEnvironment(env);
6377+
63696378
env.FireDummyTablet(ui32(NFake::TDummy::EFlg::Comp));
63706379
ZeroSharedCache(env);
63716380

@@ -6398,9 +6407,7 @@ Y_UNIT_TEST_SUITE(TFlatTableExecutor_StickyPages) {
63986407
TMyEnvBase env;
63996408
TRowsModel rows;
64006409

6401-
auto &appData = env->GetAppData();
6402-
appData.FeatureFlags.SetEnableLocalDBBtreeIndex(false);
6403-
appData.FeatureFlags.SetEnableLocalDBFlatIndex(true);
6410+
SetupEnvironment(env, false);
64046411

64056412
env.FireDummyTablet(ui32(NFake::TDummy::EFlg::Comp));
64066413
ZeroSharedCache(env);
@@ -6435,9 +6442,7 @@ Y_UNIT_TEST_SUITE(TFlatTableExecutor_StickyPages) {
64356442
TMyEnvBase env;
64366443
TRowsModel rows;
64376444

6438-
auto &appData = env->GetAppData();
6439-
appData.FeatureFlags.SetEnableLocalDBBtreeIndex(true);
6440-
appData.FeatureFlags.SetEnableLocalDBFlatIndex(false);
6445+
SetupEnvironment(env, true);
64416446

64426447
env.FireDummyTablet(ui32(NFake::TDummy::EFlg::Comp));
64436448
ZeroSharedCache(env);
@@ -6472,6 +6477,8 @@ Y_UNIT_TEST_SUITE(TFlatTableExecutor_StickyPages) {
64726477
TMyEnvBase env;
64736478
TRowsModel rows;
64746479

6480+
SetupEnvironment(env);
6481+
64756482
env.FireDummyTablet(ui32(NFake::TDummy::EFlg::Comp));
64766483
ZeroSharedCache(env);
64776484

@@ -6504,9 +6511,7 @@ Y_UNIT_TEST_SUITE(TFlatTableExecutor_StickyPages) {
65046511
TMyEnvBase env;
65056512
TRowsModel rows;
65066513

6507-
auto &appData = env->GetAppData();
6508-
appData.FeatureFlags.SetEnableLocalDBBtreeIndex(false);
6509-
appData.FeatureFlags.SetEnableLocalDBFlatIndex(true);
6514+
SetupEnvironment(env, false);
65106515

65116516
env.FireDummyTablet(ui32(NFake::TDummy::EFlg::Comp));
65126517
ZeroSharedCache(env);
@@ -6543,9 +6548,7 @@ Y_UNIT_TEST_SUITE(TFlatTableExecutor_StickyPages) {
65436548
TMyEnvBase env;
65446549
TRowsModel rows;
65456550

6546-
auto &appData = env->GetAppData();
6547-
appData.FeatureFlags.SetEnableLocalDBBtreeIndex(true);
6548-
appData.FeatureFlags.SetEnableLocalDBFlatIndex(false);
6551+
SetupEnvironment(env, true);
65496552

65506553
env.FireDummyTablet(ui32(NFake::TDummy::EFlg::Comp));
65516554
ZeroSharedCache(env);
@@ -6582,6 +6585,8 @@ Y_UNIT_TEST_SUITE(TFlatTableExecutor_StickyPages) {
65826585
TMyEnvBase env;
65836586
TRowsModel rows;
65846587

6588+
SetupEnvironment(env);
6589+
65856590
env.FireDummyTablet(ui32(NFake::TDummy::EFlg::Comp));
65866591
ZeroSharedCache(env);
65876592

@@ -6615,6 +6620,8 @@ Y_UNIT_TEST_SUITE(TFlatTableExecutor_StickyPages) {
66156620
TMyEnvBase env;
66166621
TRowsModel rows;
66176622

6623+
SetupEnvironment(env);
6624+
66186625
env.FireDummyTablet(ui32(NFake::TDummy::EFlg::Comp));
66196626
ZeroSharedCache(env);
66206627

@@ -6650,6 +6657,8 @@ Y_UNIT_TEST_SUITE(TFlatTableExecutor_StickyPages) {
66506657
TMyEnvBase env;
66516658
TRowsModel rows;
66526659

6660+
SetupEnvironment(env);
6661+
66536662
env.FireDummyTablet(ui32(NFake::TDummy::EFlg::Comp));
66546663
ZeroSharedCache(env);
66556664

ydb/core/tablet_flat/flat_ops_compact.h

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,21 @@ namespace NTabletFlatExecutor {
368368
{ },
369369
std::move(result.Overlay));
370370

371-
auto fetch = loader.Run(false);
372-
373-
Y_ABORT_UNLESS(!fetch, "Just compacted part needs to load some pages");
371+
// do not preload index as it may be already offloaded
372+
auto fetch = loader.Run({.PreloadIndex = false, .PreloadData = false});
373+
374+
if (Y_UNLIKELY(fetch)) {
375+
TStringBuilder error;
376+
error << "Just compacted part needs to load pages";
377+
for (auto collection : fetch) {
378+
error << " " << collection->PageCollection->Label().ToString() << ": [ ";
379+
for (auto pageId : collection->Pages) {
380+
error << pageId << " " << (NTable::NPage::EPage)collection->PageCollection->Page(pageId).Type << " ";
381+
}
382+
error << "]";
383+
}
384+
Y_ABORT_S(error);
385+
}
374386

375387
auto& res = prod->Results.emplace_back();
376388
res.Part = loader.Result();

ydb/core/tablet_flat/flat_part_loader.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ void TLoader::StageParseMeta() noexcept
149149
}
150150
}
151151

152-
TAutoPtr<NPageCollection::TFetch> TLoader::StageCreatePartView() noexcept
152+
TAutoPtr<NPageCollection::TFetch> TLoader::StageCreatePartView(bool preloadIndex) noexcept
153153
{
154154
Y_ABORT_UNLESS(!PartView, "PartView already initialized in CreatePartView stage");
155155
Y_ABORT_UNLESS(Packs && Packs.front());
@@ -161,12 +161,14 @@ TAutoPtr<NPageCollection::TFetch> TLoader::StageCreatePartView() noexcept
161161
};
162162

163163
if (BTreeGroupIndexes) {
164-
// Note: preload root nodes only because we don't want to have multiple restarts here
165-
for (const auto& meta : BTreeGroupIndexes) {
166-
if (meta.LevelCount) getPage(meta.GetPageId());
167-
}
168-
for (const auto& meta : BTreeHistoricIndexes) {
169-
if (meta.LevelCount) getPage(meta.GetPageId());
164+
if (preloadIndex) {
165+
// Note: preload root nodes only because we don't want to have multiple restarts here
166+
for (const auto& meta : BTreeGroupIndexes) {
167+
if (meta.LevelCount) getPage(meta.GetPageId());
168+
}
169+
for (const auto& meta : BTreeHistoricIndexes) {
170+
if (meta.LevelCount) getPage(meta.GetPageId());
171+
}
170172
}
171173
} else if (FlatGroupIndexes) {
172174
for (auto indexPageId : FlatGroupIndexes) {

ydb/core/tablet_flat/flat_part_loader.h

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,16 @@ namespace NTable {
113113
THashSet<TPageId> NeedPages;
114114
};
115115

116+
struct TRunOptions {
117+
// Marks that optional index pages should be loaded
118+
//
119+
// Effects only b-tree index as flat index is kept as sticky
120+
bool PreloadIndex = true;
121+
122+
// Marks that all data pages from the main group should be loaded
123+
bool PreloadData = false;
124+
};
125+
116126
TLoader(TPartComponents ou)
117127
: TLoader(TPartStore::Construct(std::move(ou.PageCollectionComponents)),
118128
std::move(ou.Legacy),
@@ -128,7 +138,7 @@ namespace NTable {
128138
TEpoch epoch = NTable::TEpoch::Max());
129139
~TLoader();
130140

131-
TVector<TAutoPtr<NPageCollection::TFetch>> Run(bool preloadData)
141+
TVector<TAutoPtr<NPageCollection::TFetch>> Run(TRunOptions options)
132142
{
133143
while (Stage < EStage::Result) {
134144
TAutoPtr<NPageCollection::TFetch> fetch;
@@ -138,7 +148,7 @@ namespace NTable {
138148
StageParseMeta();
139149
break;
140150
case EStage::PartView:
141-
fetch = StageCreatePartView();
151+
fetch = StageCreatePartView(options.PreloadIndex);
142152
break;
143153
case EStage::Slice:
144154
fetch = StageSliceBounds();
@@ -147,7 +157,7 @@ namespace NTable {
147157
StageDeltas();
148158
break;
149159
case EStage::PreloadData:
150-
if (preloadData) {
160+
if (options.PreloadData) {
151161
fetch = StagePreloadData();
152162
}
153163
break;
@@ -241,7 +251,7 @@ namespace NTable {
241251
}
242252

243253
void StageParseMeta() noexcept;
244-
TAutoPtr<NPageCollection::TFetch> StageCreatePartView() noexcept;
254+
TAutoPtr<NPageCollection::TFetch> StageCreatePartView(bool preloadIndex) noexcept;
245255
TAutoPtr<NPageCollection::TFetch> StageSliceBounds() noexcept;
246256
void StageDeltas() noexcept;
247257
TAutoPtr<NPageCollection::TFetch> StagePreloadData() noexcept;

ydb/core/tablet_flat/flat_sausagecache.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ void TPrivatePageCache::RegisterPageCollection(TIntrusivePtr<TInfo> info) {
5858
Stats.TotalPinnedBody += page->Size;
5959

6060
TryUnload(page);
61+
// notify shared cache that we have a page handle
62+
ToTouchShared[page->Info->Id].insert(page->Id);
6163
Y_DEBUG_ABORT_UNLESS(!page->IsUnnecessary());
6264
}
6365

@@ -285,6 +287,7 @@ const TSharedData* TPrivatePageCache::Lookup(TPageId pageId, TInfo *info) {
285287
}
286288

287289
if (page->Empty()) {
290+
Y_DEBUG_ABORT_UNLESS(info->GetPageType(page->Id) != EPage::FlatIndex, "Flat index pages should have been sticked and preloaded");
288291
ToLoad.PushBack(page);
289292
Stats.CurrentCacheMisses++;
290293
}

ydb/core/tablet_flat/flat_sausagecache.h

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,19 @@ class TPrivatePageCache {
8181
return Info->IsSticky(Id);
8282
}
8383

84-
void Fill(TSharedPageRef shared) {
85-
SharedBody = std::move(shared);
84+
void Fill(TSharedPageRef sharedBody) {
85+
SharedBody = std::move(sharedBody);
8686
LoadState = LoadStateLoaded;
8787
PinnedBody = TPinnedPageRef(SharedBody).GetData();
8888
}
8989

90+
void ProvideSharedBody(TSharedPageRef sharedBody) {
91+
SharedBody = std::move(sharedBody);
92+
SharedBody.UnUse();
93+
LoadState = LoadStateNo;
94+
PinnedBody = { };
95+
}
96+
9097
const TSharedData* GetPinnedBody() const noexcept {
9198
return LoadState == LoadStateLoaded ? &PinnedBody : nullptr;
9299
}
@@ -118,11 +125,11 @@ class TPrivatePageCache {
118125
}
119126

120127
// Note: this method is only called during a page collection creation
121-
void Fill(TPageId pageId, TSharedPageRef page, bool sticky) noexcept {
128+
void Fill(TPageId pageId, TSharedPageRef sharedBody, bool sticky) noexcept {
122129
if (sticky) {
123-
AddSticky(pageId, page);
130+
AddSticky(pageId, sharedBody);
124131
}
125-
EnsurePage(pageId)->Fill(std::move(page));
132+
EnsurePage(pageId)->ProvideSharedBody(std::move(sharedBody));
126133
}
127134

128135
void AddSticky(TPageId pageId, TSharedPageRef page) noexcept {

ydb/core/tablet_flat/flat_scan_actor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ namespace NOps {
194194
}
195195

196196
void RunLoader() {
197-
for (auto req : Loader->Run(false)) {
197+
for (auto req : Loader->Run({.PreloadIndex = false, .PreloadData = false})) {
198198
Send(Owner, new TEvPrivate::TEvLoadPages(std::move(req)));
199199
++ReadsLeft;
200200
}

ydb/core/tablet_flat/shared_cache_events.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,7 @@ namespace NKikimr::NSharedCache {
135135
};
136136

137137
struct TEvUpdated : public TEventLocal<TEvUpdated, EvUpdated> {
138-
struct TActions {
139-
THashSet<ui32> Dropped;
140-
};
141-
142-
THashMap<TLogoBlobID, TActions> Actions;
138+
THashMap<TLogoBlobID, THashSet<TPageId>> DroppedPages;
143139
};
144140
}
145141

0 commit comments

Comments
 (0)