netudima opened a new pull request, #4900:
URL: https://github.com/apache/cassandra/pull/4900
Several paths report on-heap allocation to the memtable allocator through
UpdateFunction.onAllocatedOnHeap in a way that diverges from the heap actually
retained (BTree.sizeOnHeapOf), so the memtable's owned-heap counter drifts.
1) BTree.update over-counts heap on node split/overflow, and never counts
branch sizeMaps.
Root cause: the Updater's running 'allocated' total is not a true net
delta. Leaf drain()
cleared the source node before the intended subtraction could read it (so
the source was
never released); the redistribute/overflow paths added freshly allocated
nodes without
releasing the source they replace; and branch sizeMaps were not counted
at all. The
reported value therefore diverged from sizeOnHeapOf, mostly over-counting.
Fix: account each node net - add the shallow heap (backing array plus
sizeMap) of every
newly retained node and subtract it for every replaced source; perform
the release before
the source is cleared; record the root as the top builder's source so the
old root is
released too.
Test: BTreeUpdateHeapAccountingTest asserts the cumulative
onAllocatedOnHeap equals
sizeOnHeapOf(result) across randomized inserts (small / contiguous-block
/ overlapping /
height-4 scenarios), with builder-path coverage verified by JaCoCo.
2) BTreeRow.merge does not release a row's column tree when a row tombstone
shadows its cells.
Root cause: the retain branch rebuilds the existing column tree smaller
via
BTree.transformAndFilter, which runs with BTree node accounting disabled,
but never
released the freed node structure. This under-release inflates the
memtable's owned heap.
ColumnData.Reconciler.merge already accounted this; BTreeRow.merge did
not.
Fix: report onAllocatedOnHeap(sizeOnHeapOf(retained) -
sizeOnHeapOf(existing)) when the
filter shrinks the tree.
Test:
PartitionRowAccountingTest.rowTombstoneOverExistingRowDoesNotInflateOwnership.
3) BTreeRow.merge does not account the row's LivenessInfo/Deletion change.
Root cause: the merge accounts cells and column-tree nodes via the
reconciler but not the
row's own LivenessInfo/Deletion; when they change (e.g. a row tombstone
replaces a live
row) the new objects become memtable-owned and the old ones are released,
unaccounted.
Fix: account onAllocatedOnHeap of (reconciled liveness+deletion) - (existing
liveness+deletion).
Test:
PartitionRowAccountingTest.rowTombstoneOverExistingRowDoesNotInflateOwnership.
4) Allocation and release heap functions disagree on the branch sizeMap.
Root cause: allocation accounting (BTreeRow/Columns unsharedHeapSize*) used
sizeOfStructureOnHeap, which excluded the branch sizeMap, while the
release paths used
sizeOnHeapOf, which includes it, so shrinking a multi-node column tree
over-released by
the sizeMap.
Fix: remove sizeOfStructureOnHeap and use sizeOnHeapOf everywhere, so
allocation and
release both include the sizeMap.
Test:
PartitionRowAccountingTest.rowTombstoneOverExistingRowDoesNotInflateOwnership,
which
requires two logically identical partitions reached via different merge
paths to own
exactly the same on-heap (only with all of 2-4 fixed does the on-heap
ownership match).
5) ColumnData.removeShadowed does not release a shadowed complex
(collection) column's own structure.
Root cause: when a tombstone shadows a whole complex column, removeShadowed
filters its internal
cell tree via BTree.transformAndFilter (node accounting disabled) and
releases the inner cells through
recordDeletion.delete, but never accounts the column's own cell-tree node
structure - which can span
multiple nodes - nor, when the column is dropped entirely, its wrapper.
Both were counted as owned when
the column was written (ComplexColumnData.unsharedHeapSizeExcludingData =
EMPTY_SIZE + sizeOnHeapOf(tree)
+ cells), so this under-release inflates the memtable's owned heap.
ColumnData.Reconciler.merge already accounted the tree delta on its retain
path; removeShadowed did not.
Fix: report onAllocatedOnHeap((EMPTY_SIZE + sizeOnHeapOf(tree)) after -
before), where 'after' is 0 when the column is dropped; on the update side
(recordDeletion == noOp) this is a no-op, so it cannot under-count data the
memtable never owned (CASSANDRA-21469).
Test:
PartitionRowAccountingTest.rowTombstoneOverExistingCollectionDoesNotInflateOwnership
- the collection analogue of (2)-(4): writing then deleting a row whose
set<text> spans multiple column-tree nodes must leave the memtable owning
exactly the same on-heap as never effectively writing it.
SetCellAccountingTest is also added as a guard that a grow/reset op mix on a
set<text> column never drives the memtable's owned heap negative.
patch by Dmitry Konstantinov; reviewed by TBD for CASSANDRA-21472
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
