Baymine opened a new pull request, #66513:
URL: https://github.com/apache/doris/pull/66513
### What problem does this PR solve?
Issue Number: no issue
Problem Summary:
`Allocator::realloc` previously charged the full `new_size` to the memory
tracker up front (`memory_check(new_size); consume_memory(new_size);`) and
only
released `old_size` after the underlying resize completed. This transiently
double-counts the old region: for the window between
`consume_memory(new_size)`
and `release_memory(old_size)` the tracker reflects `old_size + new_size`.
When
a query sits near its `mem_limit` and a container (e.g. a hash table)
doubles at
the boundary, that transient peak trips the limit check and surfaces as a
spurious `MEM_LIMIT_EXCEEDED` for queries that have no spill fallback, even
though the steady-state footprint after the resize is only `new_size`.
The fix tracks the net delta instead of the gross size:
- Malloc / small-mmap `realloc` path: compute `grow = new_size > old_size`
and
`delta = |new_size - old_size|`. On grow, `memory_check(delta)` +
`consume_memory(delta)` before the resize and `release_memory(delta)` on
the
failure path; on shrink, `release_memory(delta)` after success. These paths
overwhelmingly resolve in place (tcmalloc/jemalloc grow the region without
copying), so the net delta matches the real peak. If the allocator
internally
falls back to alloc+copy+free, the tracker is briefly under-counted by
`old_size` for the duration of a single `memcpy`, then self-corrects.
- mremap path: same net-delta pattern. Native Linux `mremap(MREMAP_MAYMOVE)`
rewires page tables in place without duplicating physical pages, so the
peak
equals `new_size` and net-delta accounting is exact. The apple/freebsd
`common/mremap.h` fallback (mmap+memcpy+munmap) peaks at `old+new` and is
briefly under-counted here; production is Linux-only.
- Big-alloc copy branch (`old < threshold < new`): drop the outer
`release_memory(old_size)`. The inner `alloc(new_size)` and `free(buf,
old_size)` already self-track new_size and old_size respectively, so the
outer
release would double-release. The peak here is genuinely `old+new` for the
copy window, which consumers of this path already tolerate.
Net effect: growing a buffer charges only `(new_size - old_size)` to the
current
tracker, eliminating the false positive at the limit boundary while keeping
the
tracker balanced end-to-end.
### Release note
Fix spurious `MEM_LIMIT_EXCEEDED` errors that could occur when a buffer is
reallocated (e.g. a hash table grows) while a query's memory usage is near
its
limit and no spill fallback is available.
### Check List (For Author)
- Test: Unit Test
- Added `AllocatorTrackerTest` (6 cases) in
`be/test/runtime/memory/allocator_test.cpp` covering
grow-charges-delta,
shrink-releases-delta, no-trip-at-limit-boundary, same-size no-op,
grow-then-shrink balance, and the big-alloc copy path. Ran
`run-be-ut.sh --filter='*Allocator*'`: 10/10 tests PASSED, including
the
pre-existing `AllocatorTest.TestNormal`.
- Behavior changed: Yes. `realloc` now charges the net size delta to the
memory
tracker instead of the gross `new_size`, so it no longer transiently
double-counts `old_size` against `mem_limit`.
- Does this need documentation: No
--
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]
