Skip to content

[CASSANDRA-21472][4.0] Fix memtable on-heap accounting drift in BTree.update and BTreeRow.merge#4944

Open
netudima wants to merge 1 commit into
apache:cassandra-4.0from
netudima:CASSANDRA-21472-4.0
Open

[CASSANDRA-21472][4.0] Fix memtable on-heap accounting drift in BTree.update and BTreeRow.merge#4944
netudima wants to merge 1 commit into
apache:cassandra-4.0from
netudima:CASSANDRA-21472-4.0

Conversation

@netudima

Copy link
Copy Markdown
Contributor

Several paths report on-heap allocation through UpdateFunction.onAllocatedOnHeap in a way that diverges from the heap actually retained (BTree.sizeOnHeapOf), so the memtable's owned-heap counter drifts. Five under/over-counting causes are fixed so that reported allocation matches sizeOnHeapOf:

  1. BTree.update over-counts on node split/overflow and never counts branch sizeMaps.
    The Updater's running 'allocated' was not a true net delta: leaf drain() cleared the
    source before subtracting it, the redistribute/overflow paths added new nodes without
    releasing the source they replace, and branch sizeMaps were never counted.
    Fix: account each node net - add every newly retained node's shallow heap (array plus
    sizeMap) and subtract it for every replaced source, releasing before the source is
    cleared; and record the root as the top builder's source so the old root is released too.
    Test: BTreeUpdateHeapAccountingTest (randomized small / contiguous-block / overlapping /
    height-4, coverage verified by JaCoCo).

  2. BTreeRow.merge does not release a row's column tree when a row tombstone shadows its
    cells: the retain branch rebuilds it smaller via BTree.transformAndFilter (node accounting
    disabled) but never releases the freed structure. Fix: report sizeOnHeapOf(retained) -
    sizeOnHeapOf(existing) when the filter shrinks the tree, as ColumnData.Reconciler.merge does.

  3. BTreeRow.merge does not account the row's LivenessInfo/Deletion change (e.g. a tombstone
    replacing a live row). Fix: account (reconciled liveness+deletion) - (existing liveness+deletion).

  4. Allocation and release disagree on the branch sizeMap: allocation used sizeOfStructureOnHeap
    (excludes it), release used sizeOnHeapOf (includes it). Fix: remove sizeOfStructureOnHeap and
    use sizeOnHeapOf everywhere.

  5. ColumnData.removeShadowed does not release a shadowed complex (collection) column's own
    structure: it releases the inner cells via recordDeletion.delete but not the column's cell
    tree (which can span multiple nodes) nor, when the column is dropped, its wrapper - both
    counted as owned when written. Fix: report (EMPTY_SIZE + sizeOnHeapOf(tree)) after - before
    (after is 0 when dropped); a no-op on the update side (recordDeletion == noOp), as required
    by CASSANDRA-21469.

Tests for 2-5: PartitionRowAccountingTest.rowTombstoneOverExistingRowDoesNotInflateOwnership and .rowTombstoneOverExistingCollectionDoesNotInflateOwnership require two logically identical partitions reached via different merge paths to own exactly the same on-heap (only with all fixes does it match); SetCellAccountingTest guards that a grow/reset op mix on a set column never drives the owned heap negative.

patch by Dmitry Konstantinov; reviewed by Caleb Rackliffe for CASSANDRA-21472

Several paths report on-heap allocation through UpdateFunction.onAllocatedOnHeap in a
way that diverges from the heap actually retained (BTree.sizeOnHeapOf), so the memtable's
owned-heap counter drifts. Five under/over-counting causes are fixed so that reported
allocation matches sizeOnHeapOf:

1) BTree.update over-counts on node split/overflow and never counts branch sizeMaps.
   The Updater's running 'allocated' was not a true net delta: leaf drain() cleared the
   source before subtracting it, the redistribute/overflow paths added new nodes without
   releasing the source they replace, and branch sizeMaps were never counted.
   Fix: account each node net - add every newly retained node's shallow heap (array plus
   sizeMap) and subtract it for every replaced source, releasing before the source is
   cleared; and record the root as the top builder's source so the old root is released too.
   Test: BTreeUpdateHeapAccountingTest (randomized small / contiguous-block / overlapping /
   height-4, coverage verified by JaCoCo).

2) BTreeRow.merge does not release a row's column tree when a row tombstone shadows its
   cells: the retain branch rebuilds it smaller via BTree.transformAndFilter (node accounting
   disabled) but never releases the freed structure. Fix: report sizeOnHeapOf(retained) -
   sizeOnHeapOf(existing) when the filter shrinks the tree, as ColumnData.Reconciler.merge does.

3) BTreeRow.merge does not account the row's LivenessInfo/Deletion change (e.g. a tombstone
   replacing a live row). Fix: account (reconciled liveness+deletion) - (existing liveness+deletion).

4) Allocation and release disagree on the branch sizeMap: allocation used sizeOfStructureOnHeap
   (excludes it), release used sizeOnHeapOf (includes it). Fix: remove sizeOfStructureOnHeap and
   use sizeOnHeapOf everywhere.

5) ColumnData.removeShadowed does not release a shadowed complex (collection) column's own
   structure: it releases the inner cells via recordDeletion.delete but not the column's cell
   tree (which can span multiple nodes) nor, when the column is dropped, its wrapper - both
   counted as owned when written. Fix: report (EMPTY_SIZE + sizeOnHeapOf(tree)) after - before
   (after is 0 when dropped); a no-op on the update side (recordDeletion == noOp), as required
   by CASSANDRA-21469.

Tests for 2-5: PartitionRowAccountingTest.rowTombstoneOverExistingRowDoesNotInflateOwnership and
.rowTombstoneOverExistingCollectionDoesNotInflateOwnership require two logically identical
partitions reached via different merge paths to own exactly the same on-heap (only with all
fixes does it match); SetCellAccountingTest guards that a grow/reset op mix on a set<text>
column never drives the owned heap negative.

patch by Dmitry Konstantinov; reviewed by Caleb Rackliffe for CASSANDRA-21472
@maedhroz
maedhroz self-requested a review July 16, 2026 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant