Skip to content

feat(cache): opt-in whole-session disk admission and free-volume reserve - #961

Draft
Nebr88 wants to merge 3 commits into
superuser404notfound:mainfrom
Nebr88:feat/session-disk-admission
Draft

Nebr88 wants to merge 3 commits into
superuser404notfound:mainfrom
Nebr88:feat/session-disk-admission

Conversation

@Nebr88

@Nebr88 Nebr88 commented Nov 1, 2026 •

Copy link
Copy Markdown

Opt-in disk admission accounts for each loopback session's resident and in-flight fMP4 bytes, transfers staging reservations once on adoption, and uses UUID staging ownership so an old muxer cannot delete a newer file.

Maintainer feedback addressed:

  • typed admission distinguishes capacity denial, cancellation and invalid requests; the actual muxer write callback reports capacity denial through onStorageExhausted;
  • the free-volume reserve works independently of the hard cap, including the live-session shape;
  • volumeAvailableCapacityKey is consistent with the other engine paths; requested reserves fail closed on unknown capacity;
  • diskAdmissionWaiting is separate from producerParked's forward-window meaning;
  • docs state the limit is per session (N engines may use N caps), defaults nil/zero;
  • formatting noise removed; rebased onto main a6fbf0a (4 November), preserving its forwardBufferSegmentsForSession call and current option names;
  • spatial test fixtures adopt their completed fragments before releasing the muxer, respecting staging ownership when optional audio traits are enabled.

Validation on Apple Silicon, macOS26.7.1, Xcode27.2 / Swift6.4.2:

  • isolated branch: targeted cache/storage/API41 Swift Testing +4 XCTest pass;
  • docs links, test hygiene and diff whitespace pass;
  • full default-trait suite attempted:5731 tests/775 suites, NOT green: APACEncodePipelineTests byte equality and HLSLocalServerSlowPeerTests timeout. The latter passed all8 tests in an isolated downstream rerun. APAC implementation and its test are unchanged by this PR; no successful full-suite/CI claim.
  • isolated optional-trait spatial/cache run:43 tests pass, including APAC/PCM click source-position round trips after fixture adoption;
  • downstream AppleTV4K2/A12 tvOS27.2, engine7.56.0 plus both local patches: full187.968s P7-to-P8.1/TrueHD-to-APAC7.1.4 clip reaches ended; cache peak442356905B, rejectedWrites0, one session, files removed at stop. This is combined downstream evidence, not an isolated PR-branch hardware soak. No physical disk-fill test is performed.

Still draft pending hardware soak/isolated qualification and investigation of the full-suite failures. Defaults and non-opt-in admission remain unchanged. No private media or diagnostics are published.

@superuser404notfound

Copy link
Copy Markdown
Owner

Thanks for this, the opt-in shape is right: nothing changes unless a host sets the new options, and the adoption accounting (reservation transferred once, UUID staging names so a late muxer deinit cannot delete a newer file, eviction bookkeeping matching pruneOutsideWindow) looks sound. A few things before this can leave draft:

  1. A rejected admission kills the pump without a typed error. When the reserve denies a write, onFragmentBytes only sets writeFailed. The next cutFragmentForNextSegment then returns .failed, the pump dies, the revives run and the host ends up on its fallback. A reserve denial should either surface as storageExhausted (call onStorageExhausted) or wait like the hard-limit path does, not fail the cut.
  2. The reserve only applies together with the hard limit. admitStagingWrite returns true immediately when hardDiskLimitBytes is nil, and HLSVideoEngine passes nil for every live session. docs/api.md says the reserve is independent. Either make it independent or document that it requires sessionCacheHardLimitBytes and never applies to live.
  3. Free space is measured differently from the rest of the engine. The admission reads systemFreeSize, while HLSVideoEngine (volume safety limit) and SoftwarePlaybackHost use volumeAvailableCapacityKey. Please use the same measure, otherwise the volume safety limit and the reserve can disagree about the same disk.
  4. producerParked now also means "waiting for disk". Folding hardDiskWaiters > 0 into it makes "forward window full" and "disk admission blocked" indistinguishable in SessionCacheStatus. A separate field (e.g. diskAdmissionWaiting) would keep the existing meaning.
  5. The limit is per session. With concurrent engines (multiview) the total is N times the limit, unlike the process-wide retention claim. Worth one sentence in the docs.
  6. Housekeeping: the MP4SegmentMuxer diff carries unrelated reformatting (defer, guard ... else, while bodies), please drop it. The branch is 24 commits behind main and conflicts in AetherEngine+ReloadWithOptions.swift (the option-name list), so it needs a rebase.

As with #960, I'll pick it up once it is out of draft, ideally with the isolated PR-branch run and the hardware soak you mention.

@Nebr88
Nebr88 force-pushed the feat/session-disk-admission branch from cb4490c to dc88c50 Compare November 2, 2026 09:20
@Nebr88
Nebr88 force-pushed the feat/session-disk-admission branch from dc88c50 to be8251e Compare November 4, 2026 11:49
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