Skip to content

Commit 139cd2d

Browse files
committed
MB-47604: Have CompactionContext update rollbackPurgeSeqno
For NexusKVStore we need to know the highest purged seqno before we can enable implicit compaction or concurrent flush and compaction. To get this we could add and track a new seqno but this is only required for NexusKVStore. It makes more sense to instead call some function on CompactionContext for each purged item, and defer to subclasses for the required action. This also allows us to tidy up the purging behaviours which may/may not update seqnos based on the type of item purged and use common code for all KVStores. Change-Id: I8e289529ac4a4bfb2677b9cf77145d21ece9a4a5 Reviewed-on: http://review.couchbase.org/c/kv_engine/+/163092 Reviewed-by: Richard de Mellow <[email protected]> Tested-by: Build Bot <[email protected]>
1 parent d621820 commit 139cd2d

File tree

3 files changed

+46
-7
lines changed

3 files changed

+46
-7
lines changed

engines/ep/src/kvstore/couch-kvstore/couch-kvstore.cc

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,7 @@ static int time_purge_hook(Db& d,
892892
}
893893
}
894894
// No need to make 'disk-size' changes here as the collection has gone.
895+
ctx.purgedItem(PurgedItemType::LogicalDelete, info->db_seq);
895896
return COUCHSTORE_COMPACT_DROP_ITEM;
896897
} else if (docKey.isInSystemCollection()) {
897898
ctx.eraserContext->processSystemEvent(
@@ -922,13 +923,10 @@ static int time_purge_hook(Db& d,
922923
}
923924

924925
if (purgeItem) {
925-
// Maybe update purge seqno
926-
if (ctx.rollbackPurgeSeqno < info->db_seq) {
927-
ctx.rollbackPurgeSeqno = info->db_seq;
928-
}
929926
// Update stats and return
930927
ctx.stats.tombstonesPurged++;
931928
maybeAccountForPurgedCollectionData();
929+
ctx.purgedItem(PurgedItemType::Tombstone, info->db_seq);
932930
return COUCHSTORE_COMPACT_DROP_ITEM;
933931
}
934932
}
@@ -947,6 +945,7 @@ static int time_purge_hook(Db& d,
947945
// as the stat doc will have already been deleted (for
948946
// couchstore)
949947
maybeAccountForPurgedCollectionData();
948+
ctx.purgedItem(PurgedItemType::Prepare, info->db_seq);
950949
return COUCHSTORE_COMPACT_DROP_ITEM;
951950
}
952951

engines/ep/src/kvstore/kvstore.h

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,18 @@ struct CompactionConfig {
129129
bool retain_erroneous_tombstones = false;
130130
};
131131

132+
/**
133+
* Type of item purged
134+
*/
135+
enum class PurgedItemType {
136+
// Tombstone - deleted document
137+
Tombstone = 0,
138+
// Logical deletion - item belonging to dropped collection
139+
LogicalDelete,
140+
// Prepare - complete prepare (seqno lower than PCS)
141+
Prepare,
142+
};
143+
132144
struct CompactionContext {
133145
CompactionContext(Vbid vbid,
134146
const CompactionConfig& config,
@@ -139,6 +151,34 @@ struct CompactionContext {
139151
rollbackPurgeSeqno(purgeSeq),
140152
timeToExpireFrom(timeToExpireFrom) {
141153
}
154+
155+
/**
156+
* Process a purged item
157+
*
158+
* @param type The type of the item purged
159+
* @param seqno The seqno of the item purged
160+
*/
161+
void purgedItem(PurgedItemType type, uint64_t seqno) {
162+
switch (type) {
163+
case PurgedItemType::Tombstone:
164+
// Only tombstones need to move the rollback purge seqno
165+
updateRollbackPurgeSeqno(seqno);
166+
break;
167+
case PurgedItemType::LogicalDelete:
168+
case PurgedItemType::Prepare:
169+
break;
170+
}
171+
}
172+
173+
/**
174+
* Update the rollback purge seqno
175+
*
176+
* @param seqno The seqno of the item purged
177+
*/
178+
void updateRollbackPurgeSeqno(uint64_t seqno) {
179+
rollbackPurgeSeqno = std::max(rollbackPurgeSeqno, seqno);
180+
}
181+
142182
Vbid vbid;
143183

144184
/// The configuration for this compaction.

engines/ep/src/kvstore/magma-kvstore/magma-kvstore.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,7 @@ bool MagmaKVStore::compactionCore(MagmaKVStore::MagmaCompactionCB& cbCtx,
336336
vbid,
337337
userSanitizedItemStr);
338338
}
339+
cbCtx.ctx->purgedItem(PurgedItemType::LogicalDelete, seqno);
339340
return true;
340341
}
341342
}
@@ -376,14 +377,12 @@ bool MagmaKVStore::compactionCore(MagmaKVStore::MagmaCompactionCB& cbCtx,
376377

377378
if (drop) {
378379
cbCtx.ctx->stats.tombstonesPurged++;
379-
if (cbCtx.ctx->rollbackPurgeSeqno < seqno) {
380-
cbCtx.ctx->rollbackPurgeSeqno = seqno;
381-
}
382380
auto& dbStats = cbCtx.magmaDbStats;
383381
if (dbStats.purgeSeqno < seqno) {
384382
dbStats.purgeSeqno = seqno;
385383
}
386384
maybeUpdatePurgeSeqno();
385+
cbCtx.ctx->purgedItem(PurgedItemType::Tombstone, seqno);
387386
return true;
388387
}
389388
}
@@ -403,6 +402,7 @@ bool MagmaKVStore::compactionCore(MagmaKVStore::MagmaCompactionCB& cbCtx,
403402
vbid,
404403
userSanitizedItemStr);
405404
}
405+
cbCtx.ctx->purgedItem(PurgedItemType::Prepare, seqno);
406406
return true;
407407
}
408408
}

0 commit comments

Comments
 (0)