fix(services): make persistence.mode decide storage and server persistence - #79
Merged
Conversation
…tence
persistence.mode was declared, validated, defaulted, and then ignored by every
managed service. A service that said
persistence:
mode: ephemeral
rendered the same durable Docker volume as one that said nothing, and redis and
valkey additionally fsynced every write into it through a hardcoded
--appendonly yes. Measured across all ten drivers before this change: every one
of them owned a durable volume under mode: ephemeral. The field governs volume
ownership for workloads through runtime.go; services never consulted it.
Two owners now, and the split is what makes an override safe.
The mode owns Onebox's storage. An ephemeral service gets no durable volume,
neither materialised in the project by the loader nor rendered by the generator,
and a driver setting cannot buy one back — a server flag has no business
redefining what Onebox claims about data lifetime.
The mode supplies server defaults, and authored settings replace them. redis and
valkey run appendonly yes under durable, exactly as before, and appendonly no
with snapshots off under ephemeral. An author who sets appendonly explicitly
gets their value and only their value: options are merged before rendering
rather than appended after, so the command carries one value per option.
Appending is what produced `--appendonly yes --appendonly no` and made an author
compensate for the driver in the first place.
Six frozen corpus digests move, all of them redis=. Every application digest and
every other service digest is byte-identical, which is the evidence this reached
only what it claims to.
Durable redis and valkey do render one difference: --appendonly "yes" where the
literal command said --appendonly yes. The value now travels the same quoting
path as an authored setting, because it is now the same mechanism. redis-server
reads both identically. It does move the service digest, so those services
recreate on the next deploy — and #76's health-check fix, which this is stacked
on, already recreates them in the same release, so the marginal cost is nothing.
Not done here, and deliberately: canonical and doctor do not yet report the
effective persistence value or name a divergence between an authored setting and
the declared mode. That reporting is additive and is what makes an override
visible rather than merely permitted; it wants its own change.
Refs #75.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The mode table had keys for durable and ephemeral. The grammar allows a third:
external, meaning the operator owns this data outside Onebox.
A lookup that missed returned nothing, and because the append-only flag had just
moved out of the driver's literal command into that table, `mode: external`
rendered
exec redis-server --requirepass "$REDIS_PASSWORD"
with no --appendonly at all, while still mounting the durable volume. Redis
falls back to snapshots only. That is a silent durability downgrade on the one
mode that exists to say the data matters, and it would have reached anyone whose
service Onebox does not itself back up.
Any mode added later would have hit the same nil map. So the rule is inverted:
ephemeral turns persistence off, and everything else keeps the durable options.
A table that does not recognise a mode now errs toward keeping data.
The new test also fails if ePersistence grows, so a fourth mode cannot be added
without deciding what it means here.
Found by review of this branch, not by a test.
Refs #75.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two shapes the loader accepted and the rest of the system could not honour, both newly reachable now that persistence.mode decides whether a volume exists. An ephemeral service that names its own volumes. Skipping the default volume was not enough: an authored `volumes: [cache]` survived, so the name reached the canonical form, Spec.All's preflight collision check and firstServiceVolume, while renderService created and mounted nothing. The author's declaration was ignored rather than refused, and the collision check reserved a name for a volume that would never exist. An ephemeral service that declares protection. Protection is a contract about recovering durable data, and there is none. It loaded, then failed at apply time in the active-volume seed against a volume that was never created, with the sealed protected identity naming it regardless. Both now refuse at load time and say which declaration to change. Neither is a new rule so much as the loader catching up with what the renderer already does. Found by review of this branch. Not fixed here, and not confused with these: the health-probe budget for a large AOF, and refusing maxmemory without maxmemory-policy, both of which are policy decisions rather than defects. Refs #75. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vishr
changed the base branch from
fix/redis-health-is-write-sensitive
to
main
August 19, 2026 05:04
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
1 participant
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.
Refs #75. Stacked on #78 — review that first; this PR targets its branch, not
main.The defect
persistence.modewas declared, validated, defaulted, and then ignored by every managed service. Measured before this change, all ten drivers rendered a durable volume undermode: ephemeral:The field governs volume ownership for workloads via
runtime.go:130; services never consulted it. redis and valkey additionally fsynced every write into that volume through a hardcoded--appendonly yes.Two owners
settings?persistence.modeAn ephemeral service gets no durable volume — neither materialised by the loader nor rendered by the generator — and a driver setting cannot buy one back. A server flag has no business redefining what Onebox claims about data lifetime.
Rendered, all four cases:
Options are merged before rendering rather than appended after, so the command carries one value per option. Appending is what produced
--appendonly yes --appendonly noand made an author compensate for the driver in the first place.One rendering difference, stated plainly
Durable redis renders
--appendonly "yes"where the literal command said--appendonly yes. The value now travels the same quoting path as an authored setting, because it is now the same mechanism;redis-serverreads both identically.It does move the service digest, so durable redis and valkey recreate on the next deploy. #78 already recreates them in the same release, so the marginal cost is nothing — which is the other reason this is stacked rather than parallel.
The issue asked for durable to render byte-identically. It does not, and this is the reason.
Scope, from the digests
Six frozen corpus digests move, all
redis=. Every application digest and every other service digest is byte-identical:Not done here, deliberately
ob canonicalandob doctordo not yet report the effective persistence value or name a divergence between an authored setting and the declared mode. That reporting is what makes an override visible rather than merely permitted — it is additive and wants its own change. So #75 stays open after this merges.Verified
just check,golangci-lint run ./...,go test -race ./.... Four new tests cover all ten drivers, both durable forms, the ephemeral pair, and the override-exactly-once case.