Skip to content

Commit 1ddedbb

Browse files
paolococchipaolococchi
committed
MB-58961: Rename CursorRegResult::seqno into nextSeqno
That makes the member's semantic explicit. Ie, the seqno that we get there isn't the seqno of the queued_item pointed by the newly registered cursor, that is actually the seqno of the next item available in checkpoints for cursor. Change-Id: Ia8b7e71ff8fd98b2032325b5338210b6316784d8 Reviewed-on: https://review.couchbase.org/c/kv_engine/+/199150 Tested-by: Paolo Cocchi <[email protected]> Well-Formed: Restriction Checker Reviewed-by: Vesko Karaganev <[email protected]>
1 parent 2693220 commit 1ddedbb

File tree

4 files changed

+30
-27
lines changed

4 files changed

+30
-27
lines changed

‎engines/ep/src/checkpoint_manager.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ CursorRegResult CheckpointManager::registerCursorBySeqno(
314314
std::shared_ptr<CheckpointCursor> newCursor;
315315

316316
CursorRegResult result;
317-
result.seqno = std::numeric_limits<uint64_t>::max();
317+
result.nextSeqno = std::numeric_limits<uint64_t>::max();
318318
result.tryBackfill = false;
319319

320320
auto ckptIt = checkpointList.begin();
@@ -330,7 +330,7 @@ CursorRegResult CheckpointManager::registerCursorBySeqno(
330330
// meaningless if all mutations have been expelled.
331331
newCursor = std::make_shared<CheckpointCursor>(
332332
name, ckptIt, (*ckptIt)->begin(), droppable, 0);
333-
result.seqno = lastBySeqno + 1;
333+
result.nextSeqno = lastBySeqno + 1;
334334
result.cursor.setCursor(newCursor);
335335

336336
// Trigger backfill only if there's a gap between lastProcessedSeqno and
@@ -368,7 +368,7 @@ CursorRegResult CheckpointManager::registerCursorBySeqno(
368368
// checkpoint, position cursor at the checkpoint begin.
369369
newCursor = std::make_shared<CheckpointCursor>(
370370
name, ckptIt, (*ckptIt)->begin(), droppable, 0);
371-
result.seqno = st;
371+
result.nextSeqno = st;
372372
result.cursor.setCursor(newCursor);
373373
// Set tryBackfill:true only if lastProcessedSeqno to st is non
374374
// contiguous. Note this is likely the fix for MB-53616/MB-58302
@@ -409,12 +409,12 @@ CursorRegResult CheckpointManager::registerCursorBySeqno(
409409
while (true) {
410410
auto next = std::next(pos);
411411
if (next == (*ckptIt)->end()) {
412-
result.seqno = uint64_t((*pos)->getBySeqno() + 1);
412+
result.nextSeqno = uint64_t((*pos)->getBySeqno() + 1);
413413
break;
414414
}
415415
const auto nextSeqno = uint64_t((*next)->getBySeqno());
416416
if (lastProcessedSeqno < nextSeqno) {
417-
result.seqno = nextSeqno;
417+
result.nextSeqno = nextSeqno;
418418
break;
419419
}
420420
++pos;
@@ -428,7 +428,7 @@ CursorRegResult CheckpointManager::registerCursorBySeqno(
428428
}
429429
}
430430

431-
if (result.seqno == std::numeric_limits<uint64_t>::max()) {
431+
if (result.nextSeqno == std::numeric_limits<uint64_t>::max()) {
432432
/*
433433
* We should never get here since this would mean that the sequence
434434
* number we are looking for is higher than anything currently

‎engines/ep/src/cursor.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ struct CursorRegResult {
9191
// True if the new cursor won't provide all mutations requested by the user
9292
bool tryBackfill;
9393
// The first seqno found in CM that the new cursor will pick at move
94-
uint64_t seqno;
94+
uint64_t nextSeqno;
9595
// The registered cursor
9696
Cursor cursor;
9797
};

‎engines/ep/src/dcp/active_stream.cc‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ void ActiveStream::registerCursor(CheckpointManager& chkptmgr,
258258
if (result.tryBackfill) {
259259
pendingBackfill = true;
260260
}
261-
curChkSeqno = result.seqno;
261+
curChkSeqno = result.nextSeqno;
262262
cursor = result.cursor;
263263

264264
// Note: We have just registered a cursor and then unlocked the CM at
@@ -273,12 +273,13 @@ void ActiveStream::registerCursor(CheckpointManager& chkptmgr,
273273
}
274274
log(spdlog::level::level_enum::info,
275275
"{} ActiveStream::registerCursor name \"{}\", wantedSeqno:{}, "
276-
"result{{tryBackfill:{}, seqno:{}, op:{}}}, pendingBackfill:{}",
276+
"result{{tryBackfill:{}, nextSeqno:{}, op:{}}}, "
277+
"pendingBackfill:{}",
277278
logPrefix,
278279
name_,
279280
lastProcessedSeqno,
280281
result.tryBackfill,
281-
result.seqno,
282+
result.nextSeqno,
282283
op ? ::to_string(*op) : "N/A",
283284
pendingBackfill);
284285
} catch (std::exception& error) {
@@ -1788,16 +1789,17 @@ void ActiveStream::scheduleBackfill_UNLOCKED(DcpProducer& producer,
17881789
}
17891790
log(spdlog::level::level_enum::info,
17901791
"{} ActiveStream::scheduleBackfill_UNLOCKED register cursor with "
1791-
"name \"{}\" wantedSeqno:{}, result{{tryBackfill:{}, seqno:{}}}, "
1792+
"name \"{}\" wantedSeqno:{}, result{{tryBackfill:{}, "
1793+
"nextSeqno:{}}}, "
17921794
"op:{}",
17931795
logPrefix,
17941796
name_,
17951797
lastReadSeqno.load(),
17961798
registerResult.tryBackfill,
1797-
registerResult.seqno,
1799+
registerResult.nextSeqno,
17981800
op ? ::to_string(*op) : "N/A");
17991801

1800-
curChkSeqno = registerResult.seqno;
1802+
curChkSeqno = registerResult.nextSeqno;
18011803
tryBackfill = registerResult.tryBackfill;
18021804
cursor = registerResult.cursor;
18031805

@@ -2083,12 +2085,12 @@ void ActiveStream::notifyEmptyBackfill_UNLOCKED(uint64_t lastSeenSeqno) {
20832085
log(spdlog::level::level_enum::info,
20842086
"{} ActiveStream::notifyEmptyBackfill "
20852087
"Re-registering dropped cursor with name \"{}\", "
2086-
"backfill:{}, seqno:{}",
2088+
"backfill:{}, nextSeqno:{}",
20872089
logPrefix,
20882090
name_,
20892091
result.tryBackfill,
2090-
result.seqno);
2091-
curChkSeqno = result.seqno;
2092+
result.nextSeqno);
2093+
curChkSeqno = result.nextSeqno;
20922094
cursor = result.cursor;
20932095
} catch (std::exception& error) {
20942096
log(spdlog::level::level_enum::warn,

‎engines/ep/tests/module_tests/checkpoint_test.cc‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,8 @@ TEST_P(CheckpointTest, MB25056_backfill_not_required) {
804804
// Request to register the cursor with a seqno that has been de-duped away
805805
CursorRegResult result = manager->registerCursorBySeqno(
806806
dcp_cursor.c_str(), 1005, CheckpointCursor::Droppable::Yes);
807-
EXPECT_EQ(1011, result.seqno) << "Returned seqno is not expected value.";
807+
EXPECT_EQ(1011, result.nextSeqno)
808+
<< "Returned seqno is not expected value.";
808809
EXPECT_FALSE(result.tryBackfill) << "Backfill is unexpectedly required.";
809810
}
810811

@@ -1010,7 +1011,7 @@ TEST_P(CheckpointTest, MB_58321) {
10101011
auto cursor = manager->registerCursorBySeqno(
10111012
"mb58321", 1001, CheckpointCursor::Droppable::Yes);
10121013

1013-
EXPECT_EQ(1002, cursor.seqno);
1014+
EXPECT_EQ(1002, cursor.nextSeqno);
10141015
EXPECT_FALSE(cursor.tryBackfill);
10151016

10161017
std::vector<queued_item> items;
@@ -2086,7 +2087,7 @@ void SingleThreadedCheckpointTest::
20862087
"cursor", startSeqno, CheckpointCursor::Droppable::Yes);
20872088

20882089
const auto cursor = res.cursor.lock();
2089-
EXPECT_EQ(res.seqno, 3);
2090+
EXPECT_EQ(res.nextSeqno, 3);
20902091
EXPECT_EQ(3, (*cursor->getCheckpoint())->getId());
20912092
EXPECT_EQ(queue_op::empty, (*cursor->getPos())->getOperation());
20922093
EXPECT_EQ(3, (*cursor->getPos())->getBySeqno());
@@ -2392,21 +2393,21 @@ void CheckpointTest::testExpelCheckpointItems() {
23922393
std::string dcp_cursor1(DCP_CURSOR_PREFIX + std::to_string(1));
23932394
CursorRegResult regResult = manager->registerCursorBySeqno(
23942395
dcp_cursor1.c_str(), 1001, CheckpointCursor::Droppable::Yes);
2395-
EXPECT_EQ(1003, regResult.seqno);
2396+
EXPECT_EQ(1003, regResult.nextSeqno);
23962397
EXPECT_TRUE(regResult.tryBackfill);
23972398

23982399
// Try to register a DCP cursor from 1002 - another expelled item
23992400
std::string dcp_cursor2(DCP_CURSOR_PREFIX + std::to_string(2));
24002401
regResult = manager->registerCursorBySeqno(
24012402
dcp_cursor2.c_str(), 1002, CheckpointCursor::Droppable::Yes);
2402-
EXPECT_EQ(1003, regResult.seqno);
2403+
EXPECT_EQ(1003, regResult.nextSeqno);
24032404
EXPECT_FALSE(regResult.tryBackfill);
24042405

24052406
// Try to register a DCP cursor from 1003 - the first item still in chk
24062407
std::string dcp_cursor3(DCP_CURSOR_PREFIX + std::to_string(3));
24072408
regResult = manager->registerCursorBySeqno(
24082409
dcp_cursor3.c_str(), 1003, CheckpointCursor::Droppable::Yes);
2409-
EXPECT_EQ(1004, regResult.seqno);
2410+
EXPECT_EQ(1004, regResult.nextSeqno);
24102411
EXPECT_FALSE(regResult.tryBackfill);
24112412
}
24122413

@@ -3629,7 +3630,7 @@ TEST_P(CheckpointTest, MB_47516) {
36293630
auto cursor = manager->registerCursorBySeqno(
36303631
"MB_47516", 0, CheckpointCursor::Droppable::Yes);
36313632
EXPECT_TRUE(cursor.tryBackfill);
3632-
EXPECT_EQ(1002, cursor.seqno);
3633+
EXPECT_EQ(1002, cursor.nextSeqno);
36333634
}
36343635

36353636
// In the case of open/closed checkpoints cursors placed at the high-seqno
@@ -3650,10 +3651,10 @@ TEST_P(CheckpointTest, MB_47551) {
36503651
"MB-47551", seqno, CheckpointCursor::Droppable::Yes);
36513652
if (seqno == 1001) {
36523653
EXPECT_FALSE(cursor.tryBackfill) << seqno;
3653-
EXPECT_EQ(1002, cursor.seqno) << seqno;
3654+
EXPECT_EQ(1002, cursor.nextSeqno) << seqno;
36543655
} else {
36553656
EXPECT_TRUE(cursor.tryBackfill) << seqno;
3656-
EXPECT_EQ(1001, cursor.seqno) << seqno;
3657+
EXPECT_EQ(1001, cursor.nextSeqno) << seqno;
36573658
}
36583659

36593660
EXPECT_EQ(1, (*cursor.cursor.lock()->getCheckpoint())->getId());
@@ -3666,7 +3667,7 @@ TEST_P(CheckpointTest, MB_47551) {
36663667
// And we expect to be in the open checkpoint, so we don't hold the closed
36673668
// one. Possibly don't need backfill=true, but DCP streams handle this case
36683669
EXPECT_FALSE(cursor2.tryBackfill);
3669-
EXPECT_EQ(1003, cursor2.seqno);
3670+
EXPECT_EQ(1003, cursor2.nextSeqno);
36703671
EXPECT_EQ(2, (*cursor2.cursor.lock()->getCheckpoint())->getId());
36713672
}
36723673

@@ -3698,7 +3699,7 @@ TEST_P(EagerCheckpointDisposalTest, reRegisterCheckpointCursor) {
36983699
"cursor", 1002, CheckpointCursor::Droppable::Yes);
36993700

37003701
EXPECT_FALSE(cursor.tryBackfill);
3701-
EXPECT_EQ(1003, cursor.seqno);
3702+
EXPECT_EQ(1003, cursor.nextSeqno);
37023703
EXPECT_EQ(3, (*cursor.cursor.lock()->getCheckpoint())->getId());
37033704
}
37043705

0 commit comments

Comments
 (0)