Skip to content

Commit

Permalink
KIKIMR-19139 Simplify TKeysLoader to simplify TPartIndexIt API
Browse files Browse the repository at this point in the history
  • Loading branch information
@kunga
kunga committed Sep 20, 2023
1 parent be14815 commit bac30c7
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 61 deletions.
5 changes: 0 additions & 5 deletions ydb/core/tablet_flat/flat_part_index_iter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,6 @@ class TPartIndexIt {
return Iter.GetRecord();
}

const TRecord * TryGetLastRecord() {
Y_VERIFY(Index);
return Index->GetLastKeyRecord();
}

private:
EReady DataOrGone() {
return Iter ? EReady::Data : EReady::Gone;
Expand Down
54 changes: 8 additions & 46 deletions ydb/core/tablet_flat/flat_part_keys.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,32 +151,17 @@ namespace NTable {
return true;
}

auto hasLast = Index.SeekLast();
if (hasLast == EReady::Page) {
auto ready = Index.Seek(rowId);
if (ready == EReady::Page) {
return false;
}
Y_VERIFY(hasLast != EReady::Gone, "Unexpected failure to find the last index record");

if (const auto* lastKey = Index.TryGetLastRecord()) {
if (lastKey->GetRowId() == rowId) {
LoadIndexKey(*lastKey);
return true;
} else if (lastKey->GetRowId() < rowId) {
// Row is out of range for this part
RowId = Max<TRowId>();
Key = { };
return true;
}
}

if (!SeekIndex(rowId)) {
return false;
}
if (Index.GetRowId() == rowId) {
LoadIndexKey(*Index.GetRecord());
} else if (ready == EReady::Gone) {
// Row is out of range for this part
RowId = Max<TRowId>();
Key = { };
return true;
}
Y_VERIFY(Index.GetRowId() < rowId, "SeekIndex invariant failure");

Y_VERIFY(Index.GetRowId() <= rowId, "SeekIndex invariant failure");
if (!LoadPage(Index.GetPageId())) {
return false;
}
Expand All @@ -201,11 +186,6 @@ namespace NTable {
}
Y_VERIFY(hasLast != EReady::Gone, "Unexpected failure to find the last index record");

if (const auto* lastKey = Index.TryGetLastRecord()) {
LoadIndexKey(*lastKey);
return true;
}

if (!LoadPage(Index.GetPageId())) {
return false;
}
Expand All @@ -215,13 +195,6 @@ namespace NTable {
return true;
}

bool SeekIndex(TRowId rowId) noexcept
{
auto ready = Index.Seek(rowId);
Y_VERIFY(ready != EReady::Gone, "SeekIndex called with an out of bounds row");
return ready == EReady::Data;
}

bool LoadPage(TPageId pageId) noexcept
{
Y_VERIFY(pageId != Max<TPageId>(), "Unexpected seek to an invalid page id");
Expand Down Expand Up @@ -249,17 +222,6 @@ namespace NTable {
}
}

void LoadIndexKey(const NPage::TIndex::TRecord& record) noexcept
{
if (RowId != record.GetRowId()) {
Key.clear();
for (const auto& info : Part->Scheme->Groups[0].ColsKeyIdx) {
Key.push_back(record.Cell(info));
}
RowId = record.GetRowId();
}
}

private:
const TPart* Part;
IPages* Env;
Expand Down
37 changes: 27 additions & 10 deletions ydb/core/tablet_flat/ut/ut_slice_loader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,13 @@ namespace {
}
}

void VerifyKey(const NTest::TMass& mass, TSerializedCellVec& actual, TRowId rowId) {
auto tool = NTest::TRowTool(*mass.Model->Scheme);
auto expected = tool.KeyCells(mass.Saved[rowId]);
UNIT_ASSERT_VALUES_EQUAL_C(TSerializedCellVec::Serialize(expected), TSerializedCellVec::Serialize(actual.GetCells()),
"row " << rowId << " mismatch");
}

TCheckResult RunLoaderTest(TIntrusiveConstPtr<NTest::TPartStore> part, TIntrusiveConstPtr<TScreen> screen)
{
TCheckResult result;
Expand Down Expand Up @@ -155,6 +162,19 @@ namespace {
UNIT_ASSERT_C(result.Run->size() == scrSize,
"Restored slice bounds have " << result.Run->size() <<
" slices, expected to have " << scrSize);

auto& mass = Mass0();
for (size_t i = 0; i < scrSize; i++) {
auto &sliceItem = *(result.Run->begin() + i);
auto screenItem = screen ? *(screen->begin() + i) : TScreen::THole(0, mass.Saved.Size());
UNIT_ASSERT_VALUES_EQUAL(sliceItem.FirstRowId, screenItem.Begin);
UNIT_ASSERT_VALUES_EQUAL(sliceItem.LastRowId, Min(screenItem.End, mass.Saved.Size() - 1));
UNIT_ASSERT_VALUES_EQUAL(sliceItem.FirstInclusive, true);
UNIT_ASSERT_VALUES_EQUAL(sliceItem.LastInclusive, screenItem.End >= mass.Saved.Size());
VerifyKey(mass, sliceItem.FirstKey, sliceItem.FirstRowId);
VerifyKey(mass, sliceItem.LastKey, sliceItem.LastRowId);
}

VerifyRunOrder(result.Run, *Eggs0().Scheme->Keys);

return result;
Expand All @@ -166,7 +186,7 @@ Y_UNIT_TEST_SUITE(TPartSliceLoader) {

Y_UNIT_TEST(RestoreMissingSlice) {
auto result = RunLoaderTest(Part0(), nullptr);
UNIT_ASSERT_C(result.Pages == 1, // index page
UNIT_ASSERT_C(result.Pages == 3, // index + first + last
"Restoring slice bounds needed " << result.Pages << " extra pages");
}

Expand All @@ -178,12 +198,9 @@ Y_UNIT_TEST_SUITE(TPartSliceLoader) {
TIntrusiveConstPtr<TScreen> screen = new TScreen(std::move(holes));
auto result = RunLoaderTest(Part0(), screen);

size_t expected = 1; // index page
if (startOff != 0) expected++;
if (endOff < -1) expected++;
UNIT_ASSERT_C(result.Pages == expected, // index page
"Restoring slice [" << startOff << ", " << Part0()->Index.GetEndRowId() + endOff << "] bounds needed " <<
result.Pages << " extra pages but expected " << expected);
UNIT_ASSERT_VALUES_EQUAL_C(result.Pages, 3, // index + first + last
"Restoring slice [" << startOff << ", " << Part0()->Index.GetEndRowId() + endOff << "] bounds needed "
<< result.Pages << " extra pages");
}
}
}
Expand All @@ -208,7 +225,7 @@ Y_UNIT_TEST_SUITE(TPartSliceLoader) {
screen = new TScreen(std::move(holes));
}
auto result = RunLoaderTest(Part0(), screen);
UNIT_ASSERT_C(result.Pages == 1, // index page
UNIT_ASSERT_VALUES_EQUAL_C(result.Pages, 1 + Part0()->Index->End().End(), // index + all data pages
"Restoring slice bounds needed " << result.Pages << " extra pages");
}

Expand All @@ -233,7 +250,7 @@ Y_UNIT_TEST_SUITE(TPartSliceLoader) {
screen = new TScreen(std::move(holes));
}
auto result = RunLoaderTest(Part0(), screen);
UNIT_ASSERT_C(result.Pages == 1, // index page
UNIT_ASSERT_VALUES_EQUAL_C(result.Pages, 1 + Part0()->Index->End().End(), // index + all data pages
"Restoring slice bounds needed " << result.Pages << " extra pages");
}

Expand Down Expand Up @@ -261,7 +278,7 @@ Y_UNIT_TEST_SUITE(TPartSliceLoader) {
screen = new TScreen(std::move(holes));
}
auto result = RunLoaderTest(Part0(), screen);
UNIT_ASSERT_C(result.Pages == screen->Size() + 1, // with index page
UNIT_ASSERT_VALUES_EQUAL_C(result.Pages, screen->Size() + 1, // index + data pages
"Restoring slice bounds needed " << result.Pages <<
" extra pages, expected " << screen->Size());
}
Expand Down

0 comments on commit bac30c7

Please sign in to comment.