Skip to content

Commit bac30c7

Browse files
kungakunga
committed
KIKIMR-19139 Simplify TKeysLoader to simplify TPartIndexIt API
1 parent be14815 commit bac30c7

File tree

3 files changed

+35
-61
lines changed

3 files changed

+35
-61
lines changed

‎ydb/core/tablet_flat/flat_part_index_iter.h

-5
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,6 @@ class TPartIndexIt {
129129
return Iter.GetRecord();
130130
}
131131

132-
const TRecord * TryGetLastRecord() {
133-
Y_VERIFY(Index);
134-
return Index->GetLastKeyRecord();
135-
}
136-
137132
private:
138133
EReady DataOrGone() {
139134
return Iter ? EReady::Data : EReady::Gone;

‎ydb/core/tablet_flat/flat_part_keys.h

+8-46
Original file line numberDiff line numberDiff line change
@@ -151,32 +151,17 @@ namespace NTable {
151151
return true;
152152
}
153153

154-
auto hasLast = Index.SeekLast();
155-
if (hasLast == EReady::Page) {
154+
auto ready = Index.Seek(rowId);
155+
if (ready == EReady::Page) {
156156
return false;
157-
}
158-
Y_VERIFY(hasLast != EReady::Gone, "Unexpected failure to find the last index record");
159-
160-
if (const auto* lastKey = Index.TryGetLastRecord()) {
161-
if (lastKey->GetRowId() == rowId) {
162-
LoadIndexKey(*lastKey);
163-
return true;
164-
} else if (lastKey->GetRowId() < rowId) {
165-
// Row is out of range for this part
166-
RowId = Max<TRowId>();
167-
Key = { };
168-
return true;
169-
}
170-
}
171-
172-
if (!SeekIndex(rowId)) {
173-
return false;
174-
}
175-
if (Index.GetRowId() == rowId) {
176-
LoadIndexKey(*Index.GetRecord());
157+
} else if (ready == EReady::Gone) {
158+
// Row is out of range for this part
159+
RowId = Max<TRowId>();
160+
Key = { };
177161
return true;
178162
}
179-
Y_VERIFY(Index.GetRowId() < rowId, "SeekIndex invariant failure");
163+
164+
Y_VERIFY(Index.GetRowId() <= rowId, "SeekIndex invariant failure");
180165
if (!LoadPage(Index.GetPageId())) {
181166
return false;
182167
}
@@ -201,11 +186,6 @@ namespace NTable {
201186
}
202187
Y_VERIFY(hasLast != EReady::Gone, "Unexpected failure to find the last index record");
203188

204-
if (const auto* lastKey = Index.TryGetLastRecord()) {
205-
LoadIndexKey(*lastKey);
206-
return true;
207-
}
208-
209189
if (!LoadPage(Index.GetPageId())) {
210190
return false;
211191
}
@@ -215,13 +195,6 @@ namespace NTable {
215195
return true;
216196
}
217197

218-
bool SeekIndex(TRowId rowId) noexcept
219-
{
220-
auto ready = Index.Seek(rowId);
221-
Y_VERIFY(ready != EReady::Gone, "SeekIndex called with an out of bounds row");
222-
return ready == EReady::Data;
223-
}
224-
225198
bool LoadPage(TPageId pageId) noexcept
226199
{
227200
Y_VERIFY(pageId != Max<TPageId>(), "Unexpected seek to an invalid page id");
@@ -249,17 +222,6 @@ namespace NTable {
249222
}
250223
}
251224

252-
void LoadIndexKey(const NPage::TIndex::TRecord& record) noexcept
253-
{
254-
if (RowId != record.GetRowId()) {
255-
Key.clear();
256-
for (const auto& info : Part->Scheme->Groups[0].ColsKeyIdx) {
257-
Key.push_back(record.Cell(info));
258-
}
259-
RowId = record.GetRowId();
260-
}
261-
}
262-
263225
private:
264226
const TPart* Part;
265227
IPages* Env;

‎ydb/core/tablet_flat/ut/ut_slice_loader.cpp

+27-10
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,13 @@ namespace {
123123
}
124124
}
125125

126+
void VerifyKey(const NTest::TMass& mass, TSerializedCellVec& actual, TRowId rowId) {
127+
auto tool = NTest::TRowTool(*mass.Model->Scheme);
128+
auto expected = tool.KeyCells(mass.Saved[rowId]);
129+
UNIT_ASSERT_VALUES_EQUAL_C(TSerializedCellVec::Serialize(expected), TSerializedCellVec::Serialize(actual.GetCells()),
130+
"row " << rowId << " mismatch");
131+
}
132+
126133
TCheckResult RunLoaderTest(TIntrusiveConstPtr<NTest::TPartStore> part, TIntrusiveConstPtr<TScreen> screen)
127134
{
128135
TCheckResult result;
@@ -155,6 +162,19 @@ namespace {
155162
UNIT_ASSERT_C(result.Run->size() == scrSize,
156163
"Restored slice bounds have " << result.Run->size() <<
157164
" slices, expected to have " << scrSize);
165+
166+
auto& mass = Mass0();
167+
for (size_t i = 0; i < scrSize; i++) {
168+
auto &sliceItem = *(result.Run->begin() + i);
169+
auto screenItem = screen ? *(screen->begin() + i) : TScreen::THole(0, mass.Saved.Size());
170+
UNIT_ASSERT_VALUES_EQUAL(sliceItem.FirstRowId, screenItem.Begin);
171+
UNIT_ASSERT_VALUES_EQUAL(sliceItem.LastRowId, Min(screenItem.End, mass.Saved.Size() - 1));
172+
UNIT_ASSERT_VALUES_EQUAL(sliceItem.FirstInclusive, true);
173+
UNIT_ASSERT_VALUES_EQUAL(sliceItem.LastInclusive, screenItem.End >= mass.Saved.Size());
174+
VerifyKey(mass, sliceItem.FirstKey, sliceItem.FirstRowId);
175+
VerifyKey(mass, sliceItem.LastKey, sliceItem.LastRowId);
176+
}
177+
158178
VerifyRunOrder(result.Run, *Eggs0().Scheme->Keys);
159179

160180
return result;
@@ -166,7 +186,7 @@ Y_UNIT_TEST_SUITE(TPartSliceLoader) {
166186

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

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

181-
size_t expected = 1; // index page
182-
if (startOff != 0) expected++;
183-
if (endOff < -1) expected++;
184-
UNIT_ASSERT_C(result.Pages == expected, // index page
185-
"Restoring slice [" << startOff << ", " << Part0()->Index.GetEndRowId() + endOff << "] bounds needed " <<
186-
result.Pages << " extra pages but expected " << expected);
201+
UNIT_ASSERT_VALUES_EQUAL_C(result.Pages, 3, // index + first + last
202+
"Restoring slice [" << startOff << ", " << Part0()->Index.GetEndRowId() + endOff << "] bounds needed "
203+
<< result.Pages << " extra pages");
187204
}
188205
}
189206
}
@@ -208,7 +225,7 @@ Y_UNIT_TEST_SUITE(TPartSliceLoader) {
208225
screen = new TScreen(std::move(holes));
209226
}
210227
auto result = RunLoaderTest(Part0(), screen);
211-
UNIT_ASSERT_C(result.Pages == 1, // index page
228+
UNIT_ASSERT_VALUES_EQUAL_C(result.Pages, 1 + Part0()->Index->End().End(), // index + all data pages
212229
"Restoring slice bounds needed " << result.Pages << " extra pages");
213230
}
214231

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

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

0 commit comments

Comments
 (0)