Skip to content

DynamicOffHeapSizingMemoryTarget.borrow/repay ignore the wrapped target's return value, drifting the off-heap ledger #12424

Description

@LuciferYang

Describe the bug

DynamicOffHeapSizingMemoryTarget books the requested size into its counters and discards the wrapped target's return value:

public long borrow(long size) {
  ...
  USED_OFF_HEAP_BYTES.addAndGet(size);
  recorder.inc(size);
  target.borrow(size);   // return value discarded
  return size;            // unconditional
}

public long repay(long size) {
  USED_OFF_HEAP_BYTES.addAndGet(-size);
  recorder.inc(-size);
  target.repay(size);    // return value discarded
  return size;            // unconditional
}

The wrapped target is a Spark-facing consumer (TreeMemoryConsumer / TreeMemoryConsumer.Node) whose borrow(size) is allowed to grant less than size — Spark's MemoryConsumer.acquireMemory can return anywhere from 0 to size. DynamicOffHeapSizingMemoryTarget credits the full requested size into USED_OFF_HEAP_BYTES and its per-instance recorder, and returns size to the caller as if the reservation fully succeeded. The upstream ThrowOnOomMemoryTarget therefore also treats the reservation as successful and hands the returned amount to native code, while Spark's execution pool actually reserved less.

repay has the mirrored bug: TreeMemoryConsumer.Node.repay uses Math.min(usedBytes(), size) to bound what it can free, but DynamicOffHeapSizingMemoryTarget.repay decrements the full size regardless of what was actually freed.

There is a related failure mode when the wrapped target throws: the pre-fix code increments the counters before calling target.borrow, so an exception (from Spark's memory manager, a Preconditions.checkState inside ensureFreeCapacity, or task-teardown races) permanently leaks the requested bytes into the static counter for the lifetime of the executor JVM.

Expected behavior

Book only what the wrapped target actually grants / frees, and only after the wrapped call returns normally:

public long borrow(long size) {
  ...
  final long granted = target.borrow(size);
  USED_OFF_HEAP_BYTES.addAndGet(granted);
  recorder.inc(granted);
  return granted;
}

public long repay(long size) {
  if (size == 0) return 0;
  final long freed = target.repay(size);
  USED_OFF_HEAP_BYTES.addAndGet(-freed);
  recorder.inc(-freed);
  return freed;
}

Found while reviewing the L2 memory subsystem in gluten-core. Patch and tests ready — will send a PR.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions