-
Type:
Technical Debt
-
Resolution: Unresolved
-
Priority:
Minor - P4
-
None
-
Affects Version/s: None
-
Component/s: Checkpoints
-
None
-
Storage Engines - Persistence
-
400.92
-
SE Persistence backlog
-
None
Motivation
Follow-up to the WT-12315 epic. After that epic, _ckpt_process was reduced from CCN 36 to CCN 6, and no function in src/block/block_ckpt.c exceeds the lizard CCN > 15 warning threshold. The closest remaining function is _ckpt_delete_and_merge at CCN 15 — right at the threshold. Extracting the per-deletion loop body brings it well clear and improves readability.
Current state
__ckpt_delete_and_merge contains two WT_CKPT_FOREACH loops:
- First loop: for each WT_CKPT_DELETE ckpt, free its extent-list blocks, merge its alloc/discard lists into the next real checkpoint, and flag the target with WT_CKPT_UPDATE if it needs a cookie rewrite.
- Second loop: for each WT_CKPT_UPDATE-flagged ckpt, call __ckpt_update to rewrite the cookie.
The first loop body packs seven sequential phases and is the only reason the function's CCN is 15.
Proposed refactor
Extract the body of the first loop into a static helper _ckpt_delete_one. The two loops stay where they are — they form one logical unit (the flag set by the first is consumed by the second) and belong together at this level of abstraction. ckpt_process should continue to call _ckpt_delete_and_merge as a single step.
Proposed diff sketch
static int __ckpt_delete_one(WT_SESSION_IMPL *session, WT_BLOCK *block, WT_CKPT *ckptbase, WT_BLOCK_CKPT *ci, WT_CKPT *ckpt) { WT_BLOCK_CKPT *a, *b; WT_CKPT *next_ckpt; const char *next_name; WT_ASSERT_SPINLOCK_OWNED(session, &block->live_lock); /* "From" checkpoint; skip if it applies to a previous object. */ a = ckpt->bpriv; if (a->root_objectid != block->objectid) return (0); if (WT_VERBOSE_LEVEL_ISSET(session, WT_VERB_CHECKPOINT, WT_VERBOSE_DEBUG_2)) __wti_ckpt_verbose(session, block, "delete", ckpt->name, ckpt->raw.data, ckpt->raw.size); /* Find the next real checkpoint -- the merge target. */ for (next_ckpt = ckpt + 1;; ++next_ckpt) if (!F_ISSET(next_ckpt, WT_CKPT_FAKE)) break; b = F_ISSET(next_ckpt, WT_CKPT_ADD) ? &block->live : next_ckpt->bpriv; next_name = next_ckpt->name != NULL ? next_ckpt->name : "live"; /* Free the "from" root page into its own discard list. */ if (a->root_offset != WT_BLOCK_INVALID_OFFSET) WT_RET_MSG_CHK(session, __wti_block_insert_ext(session, block, &a->discard, a->root_offset, a->root_size), "inserting root page into discard list for deleted checkpoint %s", ckpt->name); /* Free the blocks holding the "from" extent lists. */ WT_RET_MSG_CHK(session, __ckpt_extlist_fblocks(session, block, &a->alloc), "freeing alloc extent list blocks for deleted checkpoint %s", ckpt->name); WT_RET_MSG_CHK(session, __ckpt_extlist_fblocks(session, block, &a->avail), "freeing avail extent list blocks for deleted checkpoint %s", ckpt->name); WT_RET_MSG_CHK(session, __ckpt_extlist_fblocks(session, block, &a->discard), "freeing discard extent list blocks for deleted checkpoint %s", ckpt->name); /* Roll the "from" alloc/discard lists into the "to" checkpoint. */ if (a->alloc.entries != 0) WT_RET_MSG_CHK(session, __wti_block_extlist_merge(session, block, &a->alloc, &b->alloc), "merging alloc extent list from deleted checkpoint %s", ckpt->name); if (a->discard.entries != 0) WT_RET_MSG_CHK(session, __wti_block_extlist_merge(session, block, &a->discard, &b->discard), "merging discard extent list from deleted checkpoint %s", ckpt->name); /* If the target is also being deleted, leave it for the next iteration to absorb. */ if (F_ISSET(next_ckpt, WT_CKPT_DELETE)) return (0); /* Move overlapping ranges in the target into the live avail list for re-use. */ WT_RET_MSG_CHK(session, __wti_block_extlist_overlap(session, block, b), "finding reusable blocks in checkpoint %s", next_name); if (F_ISSET(next_ckpt, WT_CKPT_ADD)) { WT_RET_MSG_CHK(session, __ckpt_live_blkmods(session, ckptbase, ci, block, false), "clearing live block modifications after merging into %s", next_name); return (0); } /* Target is a real on-disk checkpoint: free its old extent-list blocks and flag for rewrite. */ WT_RET_MSG_CHK(session, __ckpt_extlist_fblocks(session, block, &b->alloc), "freeing alloc extent list blocks for updated checkpoint %s", next_name); WT_RET_MSG_CHK(session, __ckpt_extlist_fblocks(session, block, &b->discard), "freeing discard extent list blocks for updated checkpoint %s", next_name); F_SET(next_ckpt, WT_CKPT_UPDATE); return (0); }
After extraction, __ckpt_delete_and_merge becomes:
static int __ckpt_delete_and_merge(WT_SESSION_IMPL *session, WT_BLOCK *block, WT_CKPT *ckptbase, WT_BLOCK_CKPT *ci) { WT_CKPT *ckpt; WT_ASSERT_SPINLOCK_OWNED(session, &block->live_lock); WT_CKPT_FOREACH (ckptbase, ckpt) if (!F_ISSET(ckpt, WT_CKPT_FAKE) && F_ISSET(ckpt, WT_CKPT_DELETE)) WT_RET(__ckpt_delete_one(session, block, ckptbase, ci, ckpt)); WT_CKPT_FOREACH (ckptbase, ckpt) if (F_ISSET(ckpt, WT_CKPT_UPDATE)) WT_RET_MSG_CHK(session, __ckpt_update(session, block, ckptbase, ckpt, ckpt->bpriv), "updating checkpoint %s after deletion merge", ckpt->name); return (0); }
The four continue statements in the original loop become return (0) in the helper – same flow, no behavioural change.
Expected complexity after the change
| Function | CCN before | CCN after |
|---|---|---|
| __ckpt_delete_and_merge | 15 | ~4 |
| __ckpt_delete_one | n/a | ~11 |
Both are under the CCN > 15 warning threshold. Total CCN unchanged; the win is readability and headroom.
Definition of done
- __ckpt_delete_one extracted as described.
- __ckpt_delete_and_merge reduced to the two foreach loops.
- No behavioural change; pure code motion.
- Lizard confirms both functions are below CCN 15.
- ninja -C build compiles clean; existing block-manager tests pass.
- is related to
-
WT-12315 Reduce the code complexity of __ckpt_process
-
- Closed
-