[core][sandbox] Bound the image cache: LRU eviction and opt-in tarball by xyuzh · Pull Request #65748 · ray-project/ray

xyuzh · 2026-08-26T22:27:11Z

Why are these changes needed?

Nodes cache the extracted rootfs of every image they ever ran plus an uncompressed re-tar of it, and nothing reclaims the space. A long-lived node eventually fills its disk, which takes down far more than sandboxes: during Terminal-Bench 2.1 sweeps (89 distinct images, many multi-GB) under Harbor, aged nodes degraded into opaque 500s from the whole Ray stack while fresh nodes ran the same images fine.

Changes:

The image cache is bounded by default. Before each pull, ImageManager evicts the least recently extracted images until the cache fits the cap. The cap defaults to half of the filesystem that holds the cache; RAY_SANDBOX_IMAGE_CACHE_MAX_BYTES sets it explicitly and 0 disables eviction. Sibling and orphan <name>.tar archives count toward the cap.
Images in use are pinned through ImageManager. pull_image(..., instance_id=) registers the sandbox as a user of the image while holding the image's lock, and release_image(image, instance_id) drops the pin. Eviction re-checks users under the same lock, so a pull can never race an eviction, and mid-pull images (no .extracted marker) and images whose lock is held are skipped. The gVisor backend pins on pull, releases on every creation-failure path, and releases after teardown in delete_sandbox, once the overlay's lower layer is unused. Backends built on BaseImageManager get this for free.
The uncompressed tarball is gone. Its only reader was a restore branch that re-extracted it when the extracted directory had disappeared, which nothing exercised, and it doubled every image's disk footprint. Archives left by earlier versions count toward the cap and are evicted with their image.

RAY_SANDBOX_IMAGE_CACHE_MAX_BYTES is documented in the sandboxes guide under "Container images" and in the troubleshooting section.

Related issue number

Follow-up to #65570; sibling of #65737 / #65744 / #65745 (the Terminal-Bench fix series).

Checks

Signed off (DCO); ruff, black, and pydoclint pass with the pre-commit versions and flags.
Unit tests run without gVisor: test_image_cache_eviction (LRU order, in-use and kept images protected, unextracted skipped, orphan tarballs counted), test_pull_pins_image_until_release (pin on pull and on cache hit, eviction blocked until release), test_image_cache_max_bytes_default_and_env. The gVisor-gated remote-pull test asserts no archive is written.

gemini-code-assist

Code Review

This pull request introduces an image cache eviction mechanism for Ray's sandbox environment to prevent disk space exhaustion, alongside making the retention of uncompressed image tarballs opt-in. Feedback on these changes highlights critical robustness and safety concerns: a race condition and resource leak in sandbox creation could lead to failed container starts or permanent image locking, concurrent file deletions during eviction could prematurely abort the cleanup process, and invalid environment variable parsing for the cache size cap could crash the sandbox creation. Addressing these issues with more granular exception handling and proper ordering of the image usage marking will ensure a more resilient and stable implementation.

Nodes cache the extracted rootfs of every image they ever ran plus an uncompressed re-tar of it, and nothing reclaims the space: a long-lived node fills its disk, which takes down far more than sandboxes (observed as opaque 500s from the whole Ray stack during Terminal-Bench sweeps of 89 distinct multi-GB images). - RAY_SANDBOX_IMAGE_CACHE_MAX_BYTES caps the cache: before each new pull, least-recently-extracted images are evicted until the cache fits. Images backing live sandboxes are protected by refcount markers (their extracted rootfs is the overlay lower layer), mid-pull directories by the .extracted marker, and concurrent pulls by taking each candidate's per-image lock non-blocking. - The uncompressed tarball becomes opt-in (RAY_SANDBOX_KEEP_IMAGE_TARBALL): it doubled every image's footprint for a rarely-exercised restore path. Signed-off-by: xyuzh <xinyzng@gmail.com>

Review feedback, three parts: - Tolerate concurrent deletions per entry while scanning the cache instead of aborting the whole eviction on the first OSError. - Never evict images extracted within a grace period (10 minutes): it covers the window between a pull returning and its sandbox registering as a user. - Release the in-use marker on every sandbox-creation failure path (init, bundle preparation, boot): a failed creation never reaches delete_sandbox, and the marker would have blocked eviction of that image forever. Signed-off-by: xyuzh <xinyzng@gmail.com>

test_pull_and_extract_remote_image pinned the old always-write-a-tar behavior; assert the new default (no tarball) and cover the opt-in env explicitly. Signed-off-by: xyuzh <xinyzng@gmail.com>

cursor

Stale Bugbot comment from a previous run.

pcmoritz

I have a few comments:

The default is not to evict anything, right? We should think about whether that's the right default, and also document this flag (especially if we don't evict anything by default, since that will use up the disk which will be very disruptive for anybody using it)
Ideally this would be part of ImageManager so every backend will use it automatically without having to make too many API calls. In that case, the image manager should be able to call mark_image_in_use automatically when the image is used, and we probably just need an explicit release call to be called at the right time. Not necessary to address before merging, but if there is a natural API as part of the image manager, we should do it (for sure once we implement more backends).

andrewsykim

LGTM, just some nits

andrewsykim · 2026-09-04T18:59:48Z

+        pass
+
+
+def release_image_use(image_dir: str, instance_id: str) -> None:


nit: just release_images?

or release_images_in_use

Renamed in 0ccea5d: pinning moved into ImageManager, so the public API is pull_image(..., instance_id=) plus release_image(image, instance_id), and the file-level helpers are private (_mark_image_in_use / _release_image_use).

andrewsykim · 2026-09-04T19:02:48Z


+_IMAGE_CACHE_MAX_BYTES_ENV = "RAY_SANDBOX_IMAGE_CACHE_MAX_BYTES"
+_KEEP_IMAGE_TARBALL_ENV = "RAY_SANDBOX_KEEP_IMAGE_TARBALL"
+_USERS_DIRNAME = ".users"


should this be an absolute path?

It's a directory name, not a path: each cached image gets a .users/ subdirectory with one marker file per live sandbox, joined onto that image's cache dir. Renamed to _USERS_SUBDIR with a comment in 0ccea5d to make that clearer.

andrewsykim · 2026-09-04T19:03:16Z

+    return total
+
+
+def evict_images_over_cap(images_dir: str, max_bytes: int) -> None:


nit: evict_least_recently_used_images?

Renamed to evict_least_recently_used_images in 0ccea5d.

…default - `ImageManager.pull_image(..., instance_id=)` registers the sandbox as a user of the image while holding the image's lock, and `ImageManager.release_image` drops it; the gVisor backend no longer touches marker files directly. Eviction re-checks users under the same lock, so a pull can never race an eviction. The mtime grace period, which never covered cache hits, is gone. - The cache cap now defaults to half of the filesystem holding the cache; `RAY_SANDBOX_IMAGE_CACHE_MAX_BYTES` overrides it and `0` disables eviction. Invalid values log a warning and fall back to the default. - Eviction counts sibling and orphan `<name>.tar` archives toward the cap, never evicts the image being pulled, and runs after teardown in `delete_sandbox` so the overlay lower layer is unused when released. - Rename to `evict_least_recently_used_images`; document the cache settings.

xyuzh · 2026-09-05T16:49:03Z

Addressed the review comments in 0ccea5d:

Default and docs. The cache is now bounded by default: the cap is half of the filesystem that holds the cache, RAY_SANDBOX_IMAGE_CACHE_MAX_BYTES overrides it, and 0 disables eviction. Both this and RAY_SANDBOX_KEEP_IMAGE_TARBALL are documented in the sandboxes guide under "Container images".
Part of ImageManager. Pinning moved into the manager as suggested: pull_image(..., instance_id=) marks the image in use while holding its lock, and release_image(image, instance_id) releases it. The gVisor backend only passes its sandbox id and calls release_image on failure and after teardown, so other backends get the same protection with no extra calls. Doing it under the pull lock also closes the eviction race Bugbot kept flagging.

cursor

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

^{Reviewed by Cursor Bugbot for commit 0ccea5d. Configure here.}

cursor · 2026-09-05T16:54:56Z


            shutil.rmtree(root_dir, ignore_errors=True)
+            # Only now is the overlay's lower layer unused.
+            self._image_manager.release_image(config.image, sandbox_id)


Delete can leak image pin forever

Medium Severity

delete_sandbox pops metadata first and only calls release_image after kill, runsc delete, and rmtree. An exception in that stretch (for example FileNotFoundError from subprocess.run) skips the release, and a retry finds no metadata. The .users marker stays on disk, so that image is never evicted again.

^{Reviewed by Cursor Bugbot for commit 0ccea5d. Configure here.}

The archive's only reader was the restore branch in the same function, which re-extracted it when the extracted directory had gone missing, a path nothing exercised. Drop the archive, the restore branch, and the RAY_SANDBOX_KEEP_IMAGE_TARBALL opt-in. Eviction still removes archives left by earlier versions and counts them toward the cap. Docs: call out RAY_SANDBOX_IMAGE_CACHE_MAX_BYTES in the troubleshooting section as well.

xyuzh · 2026-09-05T20:49:13Z

Good call on the tarball: its only reader was the restore branch in the same function (re-extract from the archive when the extracted directory had gone missing), which nothing exercised. Removed it entirely in 51cdda2, along with RAY_SANDBOX_KEEP_IMAGE_TARBALL; eviction still cleans up archives left by earlier versions. RAY_SANDBOX_IMAGE_CACHE_MAX_BYTES is documented under "Container images" and now also in the troubleshooting section.

Signed-off-by: Philipp Moritz <pcmoritz@gmail.com>

xyuzh requested a review from a team as a code owner August 26, 2026 22:27

gemini-code-assist Bot reviewed Aug 26, 2026

View reviewed changes

cursor Bot reviewed Aug 26, 2026

View reviewed changes

ray-gardener Bot added the core Issues that should be addressed in Ray Core label Aug 27, 2026

cursor Bot reviewed Aug 28, 2026

View reviewed changes

Comment thread python/ray/experimental/sandbox/_internal/image_utils.py Outdated

xyuzh mentioned this pull request Aug 28, 2026

feat(environments/ray) Add Ray Sandbox environment (Ray Python API) harbor-framework/harbor#2785

Open

xyuzh force-pushed the sandbox-image-cache branch from 2d8b9bb to 1b06549 Compare August 28, 2026 22:21

cursor Bot reviewed Aug 28, 2026

View reviewed changes

Comment thread python/ray/experimental/sandbox/_internal/image_utils.py Outdated

xyuzh added 3 commits September 1, 2026 15:51

[core][sandbox] Update the remote-pull test for the opt-in tarball

77fc569

test_pull_and_extract_remote_image pinned the old always-write-a-tar behavior; assert the new default (no tarball) and cover the opt-in env explicitly. Signed-off-by: xyuzh <xinyzng@gmail.com>

xyuzh force-pushed the sandbox-image-cache branch from 1b06549 to 77fc569 Compare September 1, 2026 22:52

cursor Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread python/ray/experimental/sandbox/_internal/image_utils.py Outdated

klueska mentioned this pull request Sep 3, 2026

[core][sandbox] Add GPU support to Ray Sandboxes via CDI #65898

Open

pcmoritz approved these changes Sep 3, 2026

View reviewed changes

andrewsykim approved these changes Sep 4, 2026

View reviewed changes

pcmoritz added the go add ONLY when ready to merge, run all tests label Sep 5, 2026

xyuzh requested a review from a team as a code owner September 5, 2026 16:48

cursor Bot reviewed Sep 5, 2026

View reviewed changes

pcmoritz added 2 commits September 5, 2026 18:17

Update sandboxes.md

b2ae612

Signed-off-by: Philipp Moritz <pcmoritz@gmail.com>

[core][sandbox] Make cache warning test logger-independent

3a7d398

Signed-off-by: Philipp Moritz <pcmoritz@gmail.com>

pcmoritz mentioned this pull request Sep 6, 2026

[core][sandbox] Bound the image cache via LRU eviction v2 #65964

Closed

pcmoritz merged commit 5151d14 into ray-project:master Sep 6, 2026
6 checks passed

xyuzh mentioned this pull request Sep 6, 2026

[core][sandbox] Support multi-uid user namespaces for network="public" sandboxes #65823

Open

		pass


		def release_image_use(image_dir: str, instance_id: str) -> None:

		return total


		def evict_images_over_cap(images_dir: str, max_bytes: int) -> None:

Uh oh!

Conversation

xyuzh commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

Related issue number

Checks

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pcmoritz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewsykim left a comment

Choose a reason for hiding this comment

Uh oh!

andrewsykim Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

andrewsykim Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

xyuzh Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

andrewsykim Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

xyuzh Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

andrewsykim Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

xyuzh Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

xyuzh commented Sep 5, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot Sep 5, 2026

Choose a reason for hiding this comment

Delete can leak image pin forever

Uh oh!

xyuzh commented Sep 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xyuzh commented Aug 26, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading

pcmoritz left a comment •

edited

Loading