feat(vector): tiering M1 — one memory-accounting spine every ceiling reads

[pilotspacex-byte]

Vector tiering M1: one memory-accounting spine every ceiling reads

Stacked on #252 (base = feat/vector-memory-aware-offload) — shows only this delta. First milestone of the tiering-v2 architecture (decision record: tmp/TIERING-ARCHITECTURE.md, D3 accounting spine + D9 DiskANN quarantine).

Problem

A three-agent architecture review of the HOT/WARM/COLD tier found the memory ceilings don't share an accounting spine:

• --maxmemory was 100% blind to vector memory — a pure-vector workload (the documented primary disk-offload case) could drive RSS to OOM while eviction reported "under budget" and never freed a byte.
• The elastic budget misclassified vector-heavy/KV-light shards as idle donors, lending away headroom while their true footprint was over base — and the pressure cascade compared a vector-inclusive used-term against a budget inflated by that donation.
• --vec-warm-mmap-budget applied per-shard, so an N-shard instance silently allowed N× the configured WARM memory (unlike --maxmemory, which divides).
• IVF and DiskANN-cold tiers reported 0 resident bytes (no accessor) — untracked RAM invisible to the pressure trigger, MEMORY DOCTOR, and Prometheus.
• INFO reclamation_mmap_warm_bytes was a permanent 0 (read a never-incremented twin static) while the real MmapBudget counter was write-only, read by nothing.

Fix (five commits, red/green TDD each)

1. A2 — INFO reclamation_mmap_warm_bytes re-pointed at the live recl_atomics counter; new reclamation_mmap_budget_evictions_total; dead static deleted.
2. A1 — resident_bytes() for IvfSegment (centroids + posting lists), DiskAnnSegment (PQ codes + codebook; NVMe graph excluded by design), ProductQuantizer; summed into the holder roll-up. D9 quarantine: config docs disambiguate COLD-stub vs COLD-ann; dead knobs marked [reserved: M3/M5].
3. A3 — run_eviction gates on the shard aggregate (Σ all dbs' KV + published vector bytes, one Relaxed load per 100ms tick) and evicts across dbs only until the aggregate is back under budget.
4. A5 — --vec-warm-mmap-budget is now an instance-total divided across shards (floors at 1 byte/shard; "0" still disables). Behavior change for multi-shard deployments (CHANGELOG callout).
5. A4 — elastic donor/hot classification sums KV + vector per shard; a vector-heavy shard borrows instead of donating (the counterweight to A3).

Adversarial review outcome (perf-reviewer + security-reviewer)

• CRITICAL (caught + fixed, commit 2da34b92): A3's initial per-db formulation charged the shard-wide vector term to every logical db — under-detecting with KV spread across dbs (nothing evicted while aggregate RSS overran) and over-evicting (16× multi-tenant drain). Now aggregate-gated with a pinning test (eviction stops at the aggregate target; sibling db untouched).
• HIGH (documented follow-up): the on-write eviction gate remains KV-only — under noeviction a vector-heavy shard over budget does not yet reject writes; RSS is bounded by the pressure cascade + RSS watchdog. CHANGELOG limitation + tracked M1 follow-up.
• MEDIUM (deliberate, documented): pressure-cascade step 3 stays KV-only — the vector term already fired the trigger and step 2 sheds vector memory directly via offload; comment added.
• LOW (fixed): store-memory publish reordered above the elastic recompute so the classification reads this tick's own-shard vector figure.
• Verified safe: no double-counting (KV and vector atomics are disjoint), saturating arithmetic throughout, no stale readers of the undivided budget, SmallVec stays stack-only, resident_bytes is O(segments) on a 100ms tick.

Note on "auto maxmemory"

Already shipped as the G1 guardrail (apply_memory_guardrail, cgroup-aware v2+v1 with host-min, macOS sysctl) at 80% of detected RAM — kept at 80% rather than churning existing deployments; explicit --maxmemory always wins.

Verification

Dual-runtime: clippy -D warnings clean (monoio + tokio), fmt clean. New tests: info_reclamation_mmap_warm_bytes_reflects_live_counter, test_ivf_resident_bytes_accounts_heap, test_diskann_resident_bytes_accounts_pq, test_resident_bytes_includes_ivf_and_cold_tiers, test_run_eviction_counts_vector_memory, test_run_eviction_gates_on_aggregate_not_per_db, test_vec_warm_budget_divided_per_shard, recompute_elastic_budget_vector_heavy_shard_not_donor. Suites green both runtimes: timers 2, persistence_tick 7, shared_databases 8, eviction 26, holder 15, ivf 19, diskann 22, info_reclamation 9 (tokio batch 108 + 43 after fixes).

Summary by CodeRabbit

• New Features

  • Vector memory is now included in memory limits and eviction decisions, making budgeting more accurate for vector-heavy workloads.
  • Warm-tier vector mmap budgets now apply across the whole instance and are shared per shard.
  • Memory reporting now includes live warm-tier usage and a new counter for budget-based evictions.

• Bug Fixes

  • Improved accounting for IVF and DiskANN cold-tier memory so pressure and diagnostics reflect real resident usage.
  • Fixed elastic budget classification so vector-heavy shards are no longer treated as idle.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds resident-byte accounting for IVF and DiskANN cold-tier vector segments, incorporates vector memory into maxmemory eviction and elastic-budget calculations, changes warm-mmap-budget semantics to an instance-total cap divided per shard, updates INFO reclamation reporting with live counters, and documents changes in CHANGELOG.md.

Changes

Vector tiering accounting spine

Layer / File(s)	Summary
Resident-bytes accounting for vector tiers src/vector/diskann/pq.rs, src/vector/diskann/segment.rs, src/vector/segment/ivf.rs, src/vector/segment/holder.rs	Adds resident_bytes() to ProductQuantizer, DiskAnnSegment, and IvfSegment, and rolls IVF/cold-tier values into SegmentHolder::resident_bytes(), with unit tests validating each estimate.
Vector-aware elastic budget and eviction ordering src/shard/shared_databases.rs, src/shard/persistence_tick.rs	recompute_elastic_budget now includes per-shard vector resident bytes in the used-memory term; publish/recompute ordering in run_eviction_tick is moved after vector store updates; comments clarify the KV-only pressure cascade.
Aggregate maxmemory eviction across dbs src/shard/timers.rs	run_eviction now enforces an aggregate budget spanning KV and vector memory, evicting via a running remaining-budget loop instead of per-db checks; new tests cover aggregate gating and vector-triggered KV eviction.
Per-shard warm mmap budget and config docs src/config.rs, src/shard/event_loop.rs	Adds vec_warm_mmap_budget_bytes_per_shard() dividing an instance-total budget across shards with a 1-byte floor; wires the shard event loop to use it; updates docs for --vec-warm-mmap-budget, --segment-cold-after, and --vec-diskann-beam-width.
INFO reclamation reporting and CHANGELOG src/command/info_reclamation.rs, CHANGELOG.md	write_reclamation_section sources reclamation_mmap_warm_bytes from a live warm-resident counter and adds reclamation_mmap_budget_evictions_total; removes the unused RECL_MMAP_WARM_BYTES static; CHANGELOG documents the overall accounting-spine change.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Timers as Shard::run_eviction
  participant ShardDb as ShardDatabases
  participant VectorStore
  participant Eviction as Eviction helper

  Timers->>VectorStore: read published vector bytes (once)
  Timers->>ShardDb: sum estimated KV memory across dbs
  Timers->>Timers: compute aggregate remaining budget
  loop for each db over aggregate budget
    Timers->>Eviction: try_evict_if_needed_with_spill_and_total_budget(remaining)
    Eviction-->>Timers: freed memory
    Timers->>Timers: remaining -= freed
  end
  Timers->>ShardDb: publish_memory (KV estimate)
  Timers->>ShardDb: recompute_elastic_budget()
  ShardDb->>ShardDb: used = KV memory + vector resident bytes per shard

Loading

Possibly related PRs

• pilotspace/moon#170: Modifies the same elastic-budget machinery, including ShardDatabases::recompute_elastic_budget and eviction decisions in shard/persistence_tick.rs and shard/timers.rs, which this PR extends with vector-aware accounting.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is detailed but does not follow the required template sections, and it omits the checklist and performance impact fields.	Rewrite it into the template with Summary, Checklist, Performance Impact, and Notes sections, and fill in the required verification status.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly points to the tiering M1 memory-accounting spine change and matches the main theme of the PR.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/vector-tiering-accounting-spine

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (3)

src/vector/segment/holder.rs (1)

1-1950: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

File exceeds the 1500-line guideline.

This file is now well past 1500 lines (test module alone runs to ~1950). This PR adds more tests on top of an already-oversized module rather than splitting it into submodules.

As per coding guidelines, "No single .rs file should exceed 1500 lines; split larger modules into submodules."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/vector/segment/holder.rs` around lines 1 - 1950, The holder module is
over the 1500-line limit because the in-file tests are keeping
`src/vector/segment/holder.rs` oversized. Split the `#[cfg(test)] mod tests`
block out into one or more test submodules/files (for example around
`SegmentHolder`, `SearchSnapshot`, and the reload helpers) and keep
`SegmentList`, `SegmentHolder`, `SearchSnapshot`, and `MvccContext` in the main
module while preserving the existing test coverage and imports.

Source: Coding guidelines

src/command/info_reclamation.rs (1)

358-402: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: assert the newly emitted reclamation_mmap_budget_evictions_total field.

The new INFO key is emitted at Line 278 but isn't covered by the required-fields presence check, so a future regression that drops it wouldn't be caught.

💚 Add the key to the required list

             "reclamation_mmap_warm_bytes:",
+            "reclamation_mmap_budget_evictions_total:",
             "reclamation_mvcc_committed:",

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/command/info_reclamation.rs` around lines 358 - 402, The reclamation
section presence test is missing coverage for the newly emitted
reclamation_mmap_budget_evictions_total key, so a regression could remove it
unnoticed. Update info_reclamation_contains_all_required_fields to include
reclamation_mmap_budget_evictions_total in the required_fields list, using
write_reclamation_section as the source of truth for the emitted INFO keys.

src/config.rs (1)

2466-2499: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test doesn't cover the --shards 0 auto-detect case.

The new test only exercises explicit nonzero config.shards values. Given the concern above about self.shards possibly being 0 at auto-detect time, a case asserting behavior when shards == 0 would help pin down intended semantics.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/config.rs` around lines 2466 - 2499, The new
`test_vec_warm_budget_divided_per_shard` does not cover the auto-detect path
where `ServerConfig.shards` is 0. Add an assertion in this test (or a nearby
focused test) that exercises `vec_warm_mmap_budget_bytes_per_shard()` with
`shards == 0` and verifies the intended fallback behavior, using the existing
`ServerConfig::parse_from` setup and the `vec_warm_mmap_budget_bytes_per_shard`
accessor to pin down the semantics.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/config.rs`:
- Around line 1045-1060: The vec_warm_mmap_budget_bytes_per_shard helper is
using ServerConfig.shards directly, which can be 0 before auto-shard resolution
and causes the per-shard budget to be wrong. Update the call path so this logic
uses the resolved shard count (the same source used by
per_shard_pagecache_budget), either by passing num_shards into
vec_warm_mmap_budget_bytes_per_shard or by resolving shards before cloning
ServerConfig. Keep the 0-total and minimum-1-byte behavior intact, but base the
division on the resolved shard count.

In `@src/vector/diskann/segment.rs`:
- Around line 492-500: The resident_bytes estimate for DiskAnnSegment is
double-counting the inline ProductQuantizer struct overhead because
size_of::<Self>() already includes the pq field’s layout and pq.resident_bytes()
also starts with size_of::<ProductQuantizer>(). Update
DiskAnnSegment::resident_bytes to only add the truly extra heap-resident pieces
from pq (such as its trained codebook/owned buffers) or make
ProductQuantizer::resident_bytes exclude its own inline struct size, so the
accounting is not counted twice.

---

Nitpick comments:
In `@src/command/info_reclamation.rs`:
- Around line 358-402: The reclamation section presence test is missing coverage
for the newly emitted reclamation_mmap_budget_evictions_total key, so a
regression could remove it unnoticed. Update
info_reclamation_contains_all_required_fields to include
reclamation_mmap_budget_evictions_total in the required_fields list, using
write_reclamation_section as the source of truth for the emitted INFO keys.

In `@src/config.rs`:
- Around line 2466-2499: The new `test_vec_warm_budget_divided_per_shard` does
not cover the auto-detect path where `ServerConfig.shards` is 0. Add an
assertion in this test (or a nearby focused test) that exercises
`vec_warm_mmap_budget_bytes_per_shard()` with `shards == 0` and verifies the
intended fallback behavior, using the existing `ServerConfig::parse_from` setup
and the `vec_warm_mmap_budget_bytes_per_shard` accessor to pin down the
semantics.

In `@src/vector/segment/holder.rs`:
- Around line 1-1950: The holder module is over the 1500-line limit because the
in-file tests are keeping `src/vector/segment/holder.rs` oversized. Split the
`#[cfg(test)] mod tests` block out into one or more test submodules/files (for
example around `SegmentHolder`, `SearchSnapshot`, and the reload helpers) and
keep `SegmentList`, `SegmentHolder`, `SearchSnapshot`, and `MvccContext` in the
main module while preserving the existing test coverage and imports.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e7fc32ba-3119-4a63-9431-e931b8bc219a

📥 Commits

Reviewing files that changed from the base of the PR and between 571c309 and db4f7a9.

📒 Files selected for processing (11)

• CHANGELOG.md
• src/command/info_reclamation.rs
• src/config.rs
• src/shard/event_loop.rs
• src/shard/persistence_tick.rs
• src/shard/shared_databases.rs
• src/shard/timers.rs
• src/vector/diskann/pq.rs
• src/vector/diskann/segment.rs
• src/vector/segment/holder.rs
• src/vector/segment/ivf.rs

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether ServerConfig.shards gets mutated/resolved from 0 to the
# actual auto-detected count before shards are spawned, and how event_loop's
# Shard.num_shards is derived relative to it.
rg -n 'shards\s*==\s*0|shards\s*=\s*0' -A5 -B5 src/main.rs src/embedded.rs 2>/dev/null
rg -n '\.shards\s*=' -g '!*/config.rs' -g '*.rs' src
rg -n 'num_shards' -A3 -B3 src/shard/mod.rs 2>/dev/null

Repository: pilotspace/moon

Length of output: 4388

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the method and its call sites, plus the nearby per-shard pagecache helper
# that the original comment compares against.
rg -n 'vec_warm_mmap_budget_bytes_per_shard|per_shard_pagecache_budget|pagecache_budget' src/config.rs src/main.rs src/server/embedded.rs src -A4 -B4

# Show the relevant config methods around the target lines.
sed -n '1010,1085p' src/config.rs

# Show how config.shards is handled in the startup path.
sed -n '430,490p' src/main.rs
sed -n '110,145p' src/server/embedded.rs

Repository: pilotspace/moon

Length of output: 15977

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Trace RuntimeConfig construction and where `shards` is copied into it.
rg -n 'RuntimeConfig|runtime_config|shards:\s*config\.shards|shards:\s*num_shards|num_shards' src/main.rs src/server/embedded.rs src/shard -A6 -B6

# Inspect the shard spawn path and event-loop setup.
sed -n '490,760p' src/main.rs
sed -n '140,260p' src/server/embedded.rs
sed -n '440,690p' src/shard/event_loop.rs

Repository: pilotspace/moon

Length of output: 50372

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find where RuntimeConfig is built or cloned into shard state with shard counts.
rg -n 'RuntimeConfig::default\(\)|runtime_config|num_shards\s*:|shards\s*:' src/main.rs src/server/embedded.rs src/shard -A4 -B4

# Show the exact startup block that creates shard state in main.
sed -n '490,720p' src/main.rs

# Show the embedded startup path around the config mutation and shard spawn.
sed -n '120,260p' src/server/embedded.rs

Repository: pilotspace/moon

Length of output: 50372

---

Use the resolved shard count here

ServerConfig.shards can still be 0 when auto-shard resolution is enabled, so this helper can hand every shard the full instance budget. Pass the resolved num_shards in here (like per_shard_pagecache_budget) or resolve it before cloning the config.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/config.rs` around lines 1045 - 1060, The
vec_warm_mmap_budget_bytes_per_shard helper is using ServerConfig.shards
directly, which can be 0 before auto-shard resolution and causes the per-shard
budget to be wrong. Update the call path so this logic uses the resolved shard
count (the same source used by per_shard_pagecache_budget), either by passing
num_shards into vec_warm_mmap_budget_bytes_per_shard or by resolving shards
before cloning ServerConfig. Keep the 0-total and minimum-1-byte behavior
intact, but base the division on the resolved shard count.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Double-counts ProductQuantizer's struct overhead.

pq is an inline field of DiskAnnSegment, so size_of::<Self>() already includes ProductQuantizer's stack layout. Adding pq.resident_bytes() (which itself starts from size_of::<ProductQuantizer>()) re-adds that same fixed overhead a second time. Magnitude is small (tens of bytes) but the composition pattern is incorrect.

🔧 Proposed fix

     pub fn resident_bytes(&self) -> usize {
-        std::mem::size_of::<Self>() + self.pq_codes.len() + self.pq.resident_bytes()
+        std::mem::size_of::<Self>() + self.pq_codes.len()
+            + (self.pq.resident_bytes()
+                - std::mem::size_of::<crate::vector::diskann::pq::ProductQuantizer>())
     }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	/// Estimated resident heap bytes of this cold segment (accounting-spine
	/// A1): PQ codes (`num_vectors * m` bytes, kept in RAM) + the trained
	/// codebook, plus fixed struct overhead. The Vamana graph lives on NVMe
	/// and is deliberately excluded; the Linux io_uring read buffers are
	/// small, bounded, and also excluded. Previously this tier reported 0 —
	/// untracked resident memory (D9 quarantine).
	pub fn resident_bytes(&self) -> usize {
	std::mem::size_of::<Self>() + self.pq_codes.len() + self.pq.resident_bytes()
	}
	pub fn resident_bytes(&self) -> usize {
	std::mem::size_of::<Self>() + self.pq_codes.len()
	(self.pq.resident_bytes()
	- std::mem::size_of::<crate::vector::diskann::pq::ProductQuantizer>())
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/vector/diskann/segment.rs` around lines 492 - 500, The resident_bytes
estimate for DiskAnnSegment is double-counting the inline ProductQuantizer
struct overhead because size_of::<Self>() already includes the pq field’s layout
and pq.resident_bytes() also starts with size_of::<ProductQuantizer>(). Update
DiskAnnSegment::resident_bytes to only add the truly extra heap-resident pieces
from pq (such as its trained codebook/owned buffers) or make
ProductQuantizer::resident_bytes exclude its own inline struct size, so the
accounting is not counted twice.