Skip to content

ore/pool: swap-backed, size-classed buffer pool for columnar chunks#37718

Merged
DAlperin merged 7 commits into
mainfrom
dov/ore-pool
Jul 22, 2026
Merged

ore/pool: swap-backed, size-classed buffer pool for columnar chunks#37718
DAlperin merged 7 commits into
mainfrom
dov/ore-pool

Conversation

@DAlperin

@DAlperin DAlperin commented Jul 17, 2026

Copy link
Copy Markdown
Member

Motivation

Implements Layers 1 and 2 of the buffer-managed-state design (#37717) as a new
mz_ore::pool module: the swap-backed, size-classed buffer manager that replaces
the pager's blob spilling.

The pool is dead code in this PR. Nothing constructs it; the wiring arrives in the
next PR of the stack (#37719) and consumers land later. There is zero behavior
change, and every commit builds --locked.

Description

Five commits, each buildable and tested on its own:

  1. The pool. Size-classed anonymous regions, residency-scoped slots, copy-out
    reads under a per-chunk state lock, second-chance eviction banded by depth,
    spill threads for off-worker eviction I/O, warm slot reuse, budget enforcement.
  2. Observed pageout. MADV_PAGEOUT is advisory, so the ledger trusts an
    observation of the mapping rather than the advice's return value, with a
    per-extent retry budget.
  3. Hot-chunk re-admission. read_into_admit returns an evicted probe target
    to a slot from free-budget headroom or by stealing a clean backed victim's slot
    in place. Deadlock-free by try-lock discipline on the victim.
  4. Extent arena. Extents move off the global allocator into pool-owned
    MADV_NOHUGEPAGE regions (jemalloc recycling of paged-out ranges caused
    major-fault storms and swap-slot churn). Retry-capped extents leave the
    enforcement queue onto an unreclaimable gauge so a kernel that declines reclaim
    cannot put a queue walk on every insert. Ledger fixes: budget enforced against
    evictable bytes, warm-slot trims and cooling, MADV_DONTDUMP, THP opt-out on
    sub-hugepage regions, cgroup v1 / nested-v2 memory detection.
  5. Page-table observation and budget-charged steals. The pageout observation
    reads /proc/self/pagemap present bits rather than mincore, which counts the
    clean swap-cache copies of successfully reclaimed pages as resident and would
    retry-cap every extent on healthy async swap. A slot steal settles the ledger
    with the payload difference and a growing steal is charged against the budget
    like any other admission.

Verification

68 unit and concurrency tests, all of which also run under Miri (data-race and
provenance checking, including the spill-thread and admission-steal races). 6 Kani
proof harnesses over the arithmetic the unsafe blocks rest on (size-class
selection, slot-offset bounds, extent-ladder coverage, allocator disjointness).
Linux-target cross-checks of the madvise, pagemap, and cgroup paths. The
verification footprint is roughly one line per line of pool logic.

@DAlperin DAlperin changed the title dov/ore pool ore/pool: swap-backed, size-classed buffer pool for columnar chunks Jul 17, 2026
@DAlperin
DAlperin force-pushed the dov/ore-pool branch 2 times, most recently from 051f944 to 57e61a7 Compare July 20, 2026 21:53
Base automatically changed from dov/buffer-manager-design to main July 21, 2026 19:23
@DAlperin
DAlperin force-pushed the dov/ore-pool branch 2 times, most recently from ba44852 to d8a8bba Compare July 22, 2026 00:21
@DAlperin
DAlperin marked this pull request as ready for review July 22, 2026 14:40
@DAlperin
DAlperin requested a review from antiguru July 22, 2026 14:40

@antiguru antiguru left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exciting! This mostly checks out, although the pool implementation is very dense. I left some comments behind. I think this PR should be fine to merge because it's not wired yet.

}
if scratch == Scratch::Shrink {
buf.clear();
buf.shrink_to_fit();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could give some wiggle room: Shrink to next power of two or so. But let's measure first.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put it as a TODO

Comment on lines +27 to +29
//! * Regions whose class is at least one huge page are aligned to the huge
//! page and advised `MADV_HUGEPAGE`, so populating a large slot costs one
//! fault instead of one per 4 KiB.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should consider mlocking the memory. Needs tuning of limits.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put as a TODO

Comment thread src/ore/src/pool/region.rs Outdated
let base = sys::map(capacity, align)?;
if !huge {
// Opt the whole region out of transparent huge pages before any
// slot is touched. Under `transparent_hugepage=always`, fault-time

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nb, we're running with transparent_hugepage=madvise.

Comment thread src/ore/src/pool/region.rs Outdated
if pagemap.read_exact_at(&mut buf, file_offset).is_err() {
return false;
}
// One little-endian u64 per page; bit 63 is "present in

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: native-endian. Link the kernel ABI doc here.

Comment thread src/ore/src/pool/region.rs Outdated
Comment on lines +674 to +677
buf.chunks_exact(8).all(|entry| {
let entry = u64::from_le_bytes(entry.try_into().expect("eight bytes"));
entry & (1 << 63) == 0
})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use bytemuck and a Vec here to avoid unaligned reads.

Comment thread src/ore/src/pool.rs
struct Spill {
/// Chunks in `WriteInFlight`, awaiting a spill thread.
queue: Mutex<VecDeque<Arc<ChunkMeta>>>,
cv: std::sync::Condvar,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document what cv is good for.

Comment thread src/ore/src/pool.rs
Comment on lines +308 to +321
/// Lock order: a chunk's `state` mutex may be held while taking any of the
/// leaf locks — the eviction `queue`, the `extent_queue`, the spill queue,
/// and the region slot allocators — but never the reverse. The enforcement
/// and backing scans additionally drop the queue guard before trying a
/// chunk's state lock (and only ever `try_lock` it), so no path holds a
/// queue lock while waiting on chunk state. The admitting read's victim
/// steal is the one place a chunk's state lock is held while probing
/// another chunk's, and the victim is only ever `try_lock`ed, so two
/// admitters stealing toward each other skip instead of deadlocking. Reads
/// copy out under the chunk's state lock — the same lock eviction takes —
/// so there is no reader-side count and no reader the evictor must account
/// for.
#[derive(Debug)]
struct PoolInner {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also document what PoolInner represents.

Comment thread src/ore/src/pool.rs
/// The insert-time [`ExtentCodec`]; immutable. Encodes the chunk when it
/// is backed and decodes its extent on reads, so it must outlive any
/// extent it produced, hence `'static`.
codec: &'static dyn ExtentCodec,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment thread src/ore/src/pool.rs Outdated
Comment on lines +561 to +563
/// Relies on abort-on-panic: a panic in `fill` that was caught would
/// leak the slot and its resident-bytes accounting. All hosting
/// binaries abort via `mz_ore::panic::install_enhanced_handler`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we're not using this within the optimizer where we catch panics.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct, we can revisit this if we do

DAlperin added 7 commits July 22, 2026 12:24
Extents move off the global allocator into a pool-owned, size-classed,
NOHUGEPAGE arena with a counted heap fallback that is never paged out.
The extent queue gains amortized pruning, and retry-capped extents leave
it entirely: their bytes move to an unreclaimable gauge that enforcement
and the inline backstop subtract, so a kernel that declines reclaim
cannot put a queue walk on every insert. A read that restores the retry
budget re-counts and re-enqueues the extent. Sub-hugepage slot regions
opt out of THP and every region is excluded from core dumps. Warm-reuse
tail trimming (huge-page granularity on hugepage classes) keeps resident
bytes exact, warm slots cool on budget shrink, and budget enforcement
and admission reserve against evictable bytes only. Debug builds poison
slots before fill. spill_handoff requires a slot. Memory detection
honors cgroup v1 and nested v2 limits.
…dget

Pageout observation reads pagemap present bits instead of mincore. A page
unmapped to a swap entry counts as reclaimed even while its clean copy
lingers in the kernel swap cache, which mincore reports as in core:
observing in-core-ness misclassifies essentially every successful
asynchronous reclaim on an unpressured host, and the retry advice cannot
change the outcome because madvise skips already-unmapped PTEs, so the
compressed tier would retry-cap every extent within milliseconds and
disable itself.

Slot steals settle the ledger inside the steal with the payload
difference, and a steal that grows resident bytes must fit the budget
like any other admission. The retry-cap and stats documentation names
the real transient decline causes, admission and steal-scan contracts
are documented, and admission tests cover the steal race, budget
charging, unequal payloads, device-resident revival, and the non-evicted
no-op paths.
@DAlperin
DAlperin merged commit f2082d0 into main Jul 22, 2026
128 of 129 checks passed
@DAlperin
DAlperin deleted the dov/ore-pool branch July 22, 2026 20:21
DAlperin added a commit that referenced this pull request Jul 23, 2026
…fgs (#37719)

### Motivation

Wires the `mz_ore::pool` buffer manager (#37718) to configuration and
observability. The pool stays consumer-less after this PR; chunk
batchers adopt it
in later PRs of the stack. The design is #37717.

The top merge risk is behavior change under default configs, so the
central
property is that a stock process behaves exactly as before: the pool is
never
constructed and the legacy pager stays fully operational.

### Description

* **Installation is gated.** `apply_pool_config` runs only when a spill
gate
(`enable_column_paged_batcher_spill` or `enable_upsert_paged_spill`,
both default
`false`) is on. A process that never enables spilling never constructs
the pool:
no address-space reservation, no spill threads. The first config tick
with a gate
on installs the pool; installation is one-way (documented), later ticks
retune in
  place.
* **Budgets derive from physical RAM** (cgroup-clamped), never from the
announced
  memory limit, which includes swap on swap-provisioned nodes.
* **Metrics.** 24 `mz_column_pool_*` computed gauges registered with
literal names
and help strings the metrics-catalog generator reads.
`doc/user/data/metrics.yml`
  is regenerated.
* **The legacy pager is untouched.** Its tiered config still applies,
its dyncfgs
stay registered and stay in the Python test-flag lists, and its metrics
stay in
  the catalog. It is deleted at the end of the migration, not here.

### Verification

`cargo check` across the touched crates. `bin/lint-test-flags` and the
metrics-catalog lint both pass (reproduced locally). The regenerated
`metrics.yml`
is byte-identical to generator output. No behavior change under default
configs:
the pool is never installed and the legacy pager path is unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants