Skip to content

fix(storage#741): drop page cache before read_threads memory guard - #48

Merged
FileSystemGuy merged 1 commit into
mainfrom
fix/storage-741-drop-caches-before-memory-guard
Jul 9, 2026
Merged

FileSystemGuy merged 1 commit into
mainfrom
fix/storage-741-drop-caches-before-memory-guard

Conversation

@FileSystemGuy

Copy link
Copy Markdown

Problem

ConfigArguments.validate in utils/config.py samples
psutil.virtual_memory().available and caps read_threads against it
(the mlcommons/storage#448 per-node budget fix). On POSIX filesystems
whose client pins a large reclaimable page cache (Lustre in
particular), MemAvailable understates by the size of that
cache — so after run 1 warms the cache, runs 2..N of a 5-run
submission see the cache as "unavailable" and DLIO refuses to launch:

Per-node memory budget exceeded on host X: reader.read_threads=17 x
local_ranks=16 = 288 worker processes, estimated ~144 GB (available
RAM on this node: 159 GB; total: 188 GB). Reduce reader.read_threads
to at most 16.

This aborts initialize() before the per-epoch page-cache flush
ever gets to run. reportgen then reports INVALID: 0 runs for a
submission whose only real problem is that the benchmark's own
per-epoch flush (which drops that exact cache every epoch) hadn't run
yet. Reporter's evidence: run 1 passes at ~170 GB MemAvailable; runs
2–5 crash at ~147–150 GB; sync; echo 1 > drop_caches restores ~174
GB and the guard passes again — confirming the "used" RAM is
reclaimable cache.

Filed as mlcommons/storage#741 (@mirajeev).

Fix

Right before sampling virtual_memory(), drop the local host's page
cache using the same sudo -n sh -c 'echo 3 > /proc/sys/vm/drop_caches'
command already invoked per-epoch in main.py. Gated on
local_rank() == 0 so exactly one flush runs per host, followed by a
comm().Barrier() so non-leader ranks read post-drop memory.

Design choices worth flagging on review

local_rank() not MPI.node(). MPI.node() is computed from
cumulative offsets of mpi_ppn_list (utility.py:313–318) and assumes
contiguous per-node rank blocks. Under --map-by node (which
mlpstorage sets by default for multi-host) that assumption is
violated: rank 0 → host 0, rank 1 → host 1, …, rank N → host 0, and
two different physical hosts end up computing the same node index.
This is the same anti-pattern PR mlcommons/storage#675 fixed for the
host_memory_GB collector array (the [376.18 × 7, 187.90, 0.0 × 7]
shape). local_rank() derives from MPI.COMM_TYPE_SHARED — the
MPI-3 primitive that groups ranks by shared memory — and is
independent of rank assignment. Worst-case failure mode is
redundant/idempotent flushes on a host, never a missed flush on a host
that needed one.

No SSH dependency. The flush runs inside the DLIO process, which
is already up on each host via whatever launcher was used (OpenMPI,
PALS palsd, Slurm slurmstepd). We piggyback on the transport that
already worked. In particular this is compatible with the
--skip-ssh-check / PALS/Slurm auto-skip path in
mlcommons/storage#740 — those users need this fix precisely because
their sites disallow passwordless SSH between compute nodes, so any
mlpstorage-side fan-out via mpirun would have been unusable for
them.

Fail-open matches per-epoch flush. Every failure mode — sudo -n
refused, kernel timeout, Barrier() error, subprocess OSError —
is swallowed. The guard downstream still runs; the worst case is that
it still trips on this host, which is exactly current behavior. No
regression path.

Same env-var timeout knob. Uses _resolve_drop_caches_timeout()
(30s default, DLIO_DROP_CACHES_TIMEOUT override). Operators already
tune this for the per-epoch flush; introducing a second knob would
double the operational surface for no gain.

Skipped when MPI not initialized. In child processes and test
harnesses mpi_state != MPI_INITIALIZED; the flush is skipped so
comm() isn't called and the guard still runs unchanged.

Test plan

  • 18 new unit tests in tests/test_drop_caches_before_memory_guard.py:
    MPI-state gating, local_rank() gate (leader + non-leader
    parametrized), fail-open across every subprocess exception class
    + non-zero exit + Barrier failure on leader/non-leader, timeout
    plumbing (default + env override + bad-env-value).
  • Existing tests/test_drop_caches_timeout.py (22 tests) still
    passes — no changes to _resolve_drop_caches_timeout.
  • uv run pytest tests/ --ignore=<benchmark end-to-end suites> —
    183 passed, 2 skipped, no regressions.
  • uv run pytest tests/test_fast_ci.py — 92 passed, 1 skipped.
    Includes TestEndToEndSmoke::test_train_npy_smoke which
    exercises the modified validate() path with read_threads > 0
    and PyTorch data loader.

Related

The guard in `ConfigArguments.validate` reads
`psutil.virtual_memory().available` and caps `read_threads` against it.
On POSIX filesystems whose client pins a large reclaimable page cache
(Lustre in particular), `MemAvailable` understates once a prior run has
warmed the cache — a legal config that passed the guard on run 1 will
trip it on runs 2..N of a 5-run submission, aborting the second run in
`initialize()` before the per-epoch flush ever gets to run.  reportgen
then sees `INVALID: 0 runs` for the submission even though every rejected
run's memory pressure came from evictable cache the benchmark drops
every epoch anyway.

Fix: right before sampling `virtual_memory()`, drop the local host's
page cache via the same `sudo -n sh -c 'echo 3 > /proc/sys/vm/drop_caches'`
command already used per-epoch in `main.py`.  Gate on `local_rank() == 0`
so exactly one flush runs per host, then barrier so non-leaders read
post-drop memory.

Why `local_rank()` not `MPI.node()`: `MPI.node()` is unreliable under
`--map-by node` (the same anti-pattern PR mlcommons/storage#675 fixed
for the `host_memory_GB` collector array — two hosts' local-rank-0 can
compute the same node index).  `local_rank()` derives from
`MPI.COMM_TYPE_SHARED` which is independent of rank assignment.
Worst-case failure mode is redundant (idempotent) flushes, never a
missed flush on a host that needed one.

Why this doesn't need SSH: the added flush runs inside the DLIO
process, which is already up on each host via whatever launcher
mlpstorage used (OpenMPI, PALS palsd, Slurm slurmstepd).  We piggyback
on the transport that already worked — no new privilege or reachability
requirement beyond the passwordless-sudo already assumed by the
per-epoch flush.

Fail-open: every subprocess or barrier failure is swallowed so the
guard still runs.  Matches the per-epoch flush's posture; the worst
case is the guard trips on this host, which is current behavior.

Timeout is the existing `DLIO_DROP_CACHES_TIMEOUT` env var (default
30s), so operators keep one knob for both the pre-guard and per-epoch
flushes.
@FileSystemGuy
FileSystemGuy requested a review from a team July 9, 2026 22:26
@FileSystemGuy
FileSystemGuy merged commit 95c6a9d into main Jul 9, 2026
7 checks passed
@FileSystemGuy
FileSystemGuy deleted the fix/storage-741-drop-caches-before-memory-guard branch July 9, 2026 22:29
FileSystemGuy added a commit to mlcommons/storage that referenced this pull request Jul 9, 2026
Picks up DLIO_local_changes PR #48 (`fix(storage#741): drop page
cache before read_threads memory guard`), which was merged after
the previous storage bump PR (#746) had already been opened
against DLIO main HEAD 86945a7a.

What #48 fixes (from the DLIO PR body / storage#741):
The read_threads per-node memory guard in ConfigArguments.validate
samples psutil.virtual_memory().available and caps read_threads
against it.  On POSIX filesystems whose client pins a large
reclaimable page cache (Lustre in particular), MemAvailable
understates by the size of that cache — so after run 1 warms the
cache, runs 2..N of a 5-run submission see the cache as
"unavailable" and DLIO refuses to launch in initialize(), before
the per-epoch flush ever gets to run.  reportgen then reports
INVALID: 0 runs for a submission whose only real problem is that
the benchmark's own per-epoch flush hadn't run yet.

The fix drops the local host's page cache right before
virtual_memory() is sampled, gated on local_rank() == 0 (not
MPI.node(), which is unreliable under --map-by node) with a
Barrier so non-leaders read post-drop memory.  Reuses the
existing DLIO_DROP_CACHES_TIMEOUT env var — no new operator
knob.

Not bumping mlpstorage version: this is a pure DLIO-side pin
advance with no mlp-storage code change.  The version bump
already went out with #746 (3.0.39 -> 3.0.40); #746 was opened
before #48 merged, so this follow-up captures #48 into the same
3.0.40 line before it ships.

VERIFICATION
============

    $ uv lock --upgrade-package dlio-benchmark
    Resolved 108 packages in 1.33s
    Updated dlio-benchmark v3.0.2 (86945a7a) -> v3.0.2 (95c6a9d4)

    $ uv sync --active && uv pip install -e ./vdb_benchmark -e ./kv_cache_benchmark
    $ uv run pytest tests/unit -q            # 2689 passed, 1 skipped
    $ uv run pytest mlpstorage_py/tests -q   # 840 passed
    $ uv run pytest vdb_benchmark/tests -q   # 174 passed
    $ uv run pytest kv_cache_benchmark/tests -q  # 238 passed

    Total: 3941 passed, 1 skipped across all four CI suites.

Refs: #741
      mlcommons/DLIO_local_changes#48
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.

1 participant