multilevel: a chain with any writable level is writable#2778
Open
tycho wants to merge 1 commit into
Open
Conversation
MultiLevelStorage::check() started from ReadWrite and demoted the composite to ReadOnly as soon as any level reported ReadOnly. The server then wraps the whole storage in ReadOnlyStorage, so every cache write is refused before any write-error policy is consulted, including writes to writable levels and preprocessor cache updates. That inverts the documented semantics: MultiLevel.md promises that read-only levels are "automatically skipped during writes, regardless of write policy", and put() already implements exactly that per-level skipping. The practical fallout is that the documented read-only fallback topology (fast writable local disk in front of a read-only shared remote, e.g. SCCACHE_S3_RW_MODE=READ_ONLY or an azure SAS token without write permission) silently stops populating the local disk cache entirely. Aggregate in the other direction: the composite is writable when any level is writable, and read-only only when every level is read-only. The existing single-level read-only test still passes; new tests cover mixed chains in both orders and the all-read-only chain. Fixes mozilla#2773 Signed-off-by: Steven Noonan <steven@edera.dev>
tycho
force-pushed
the
pr/multilevel-mixed-rw-ro
branch
from
July 22, 2026 20:21
1e91ac5 to
49232d6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2773.
MultiLevelStorage::check()starts fromReadWriteand demotes the composite toReadOnlyas soon as any level reportsReadOnly. The server then wraps the entire storage inReadOnlyStorage("server has setup with ReadOnly"), so every cache write is refused before any write-error policy is consulted -- including writes to writable levels, and preprocessor cache updates ("Failed to update preprocessor cache: Cannot write to read-only storage").This inverts the documented semantics:
docs/MultiLevel.mdsays read-only levels are "automatically skipped during writes, regardless of write policy", andput()already implements exactly that per-level skipping. The practical fallout is that the documented read-only fallback topology -- a fast writable local disk in front of a read-only shared remote (SCCACHE_S3_RW_MODE=READ_ONLY, or an Azure SAS token without write permission, which fails the startup write probe and is treated as read-only) -- silently stops populating the local disk cache entirely.Reproduction (hello-world C compile, sccache 0.16.0):
diskdisk, s3/azure (read-only)disk, s3/azure (read-write)The fix aggregates in the other direction: the composite is writable when any level is writable, and read-only only when every level is read-only.
put()needs no changes -- it already routes writes only to writable levels under all three write-error policies.Tests: the existing
test_readonly_level_in_check(single-level, all-read-only chain) still passes; addedtest_mixed_readonly_chain_is_readwrite_in_check(writable+read-only in both orders) andtest_all_readonly_chain_is_readonly_in_check.Verified end to end against a real Azure container with a read-list-only SAS token: with this fix the same config that previously wrote nothing populates the local disk tier, and remote hits backfill to disk as documented.