Skip to content

Commit fe94d95

Browse files
badrishcCopilot
andcommitted
fix: use value-only heap in OnDispose(Deleted) to prevent key-overflow double-subtract
OnDispose(Deleted) now uses GetValueHeapMemorySize() instead of CalculateHeapMemorySize(). This prevents double-subtracting key overflow bytes for immutable-region deletes where: 1. OnDispose(Deleted) fires on the source (subtracts full heap including key) 2. ClearHeapFields(clearKey=false) keeps key alive for chain traversal 3. Source is sealed (not tombstoned) — OnEvict visits it later 4. CalculateHeapMemorySize returns key overflow → subtracted again With the fix: - OnDispose(Deleted) subtracts value-only heap (value overflow + value object) - For mutable deletes: tombstoned → OnEvict skips → key overflow is a bounded phantom freed by GC when the page is freed (same as pre-IRecordTriggers) - For immutable deletes: sealed → OnEvict visits → subtracts key overflow once - Net: no double-decrement, bounded key-overflow phantom on mutable deletes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f96ee7e commit fe94d95

1 file changed

Lines changed: 8 additions & 5 deletions

File tree

libs/server/Storage/Functions/GarnetRecordTriggers.cs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,14 @@ public void OnDispose(ref LogRecord logRecord, DisposeReason reason)
5656

5757
if (reason == DisposeReason.Deleted)
5858
{
59-
// Decrement the full heap contribution of the dying record (overflow key/value bytes
60-
// and/or value-object heap). Tombstone is NOT yet set when this fires — Tsavorite sets
61-
// it AFTER OnDispose returns — so CalculateHeapMemorySize returns the correct non-zero
62-
// value. ClearHeapFields then physically frees the heap fields from the ObjectIdMap.
63-
var size = MemoryUtils.CalculateHeapMemorySize(in logRecord);
59+
// Decrement only the VALUE-side heap (value overflow bytes or value-object heap).
60+
// Key overflow is NOT subtracted here because ClearHeapFields(clearKey=false) keeps the
61+
// key alive for hash-chain traversal. Key overflow is handled separately:
62+
// - Mutable delete: record is tombstoned after OnDispose → OnEvict skips it → key overflow
63+
// is a bounded phantom (freed by GC when the page is freed, same as pre-IRecordTriggers).
64+
// - Immutable delete: source is sealed (not tombstoned) → OnEvict visits it and subtracts
65+
// the remaining key overflow via CalculateHeapMemorySize.
66+
var size = logRecord.GetValueHeapMemorySize();
6467
if (size != 0)
6568
cacheSizeTracker.AddHeapSize(-size);
6669
}

0 commit comments

Comments
 (0)