LuciferYang opened a new issue, #12424:
URL: https://github.com/apache/gluten/issues/12424
### Describe the bug
`DynamicOffHeapSizingMemoryTarget` books the requested size into its
counters and discards the wrapped target's return value:
```java
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:
```java
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.
--
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]
