Add a raw cache file format - #614
Merged
Merged
Conversation
Owner
Author
|
@emlimap This adds a new |
The cache file is newline-delimited JSON with each message base64'd inside it, so writing re-encodes every entry the cache already holds in wire format and reading parses it all back. Entries are stored as one packed blob since #610, which makes a file of those blobs almost free to produce and to load. 20 000 entries json raw size 345 B/rec 124 B/rec 2.8x smaller write 45.6 ms 2.1 ms 21x read 157.0 ms 24.2 ms 6.5x The format is a magic and version byte, then each entry as a uint32 length and the blob. The magic starts with a NUL so it can't be taken for the JSON format, whose records start with '{'. Which format is written is a config option, file-format, defaulting to json. Which format is read is not: it comes from the file itself, so turning the option on or off keeps an existing cache rather than dropping it. A file written by an older build is read as before; a raw file given to an older build fails to parse and starts with an empty cache, which is a cold start rather than a failure. Corruption is handled as the JSON format handles it, with one difference that the framing forces. A record that doesn't decode is skipped and reading continues. A length that can't be trusted has no next record to find, so reading stops there and keeps what it already has, and a length beyond the largest possible record is refused rather than allocated for.
maxRawCacheRecord was dns.MaxMsgSize plus a little, on the reasoning that an entry couldn't be bigger than a DNS message. It can. Nothing bounds an entry on the store path, and the cache packs without name compression, so a response that arrived well inside the wire limit is larger once stored: a 62KB answer measured here re-packs to 127KB. serializeRaw skipped anything over the limit without a word, so switching a working cache to the raw format quietly lost exactly the largest and most expensive entries, every restart, while the same cache written as JSON kept them. The limit is now what it should always have been, a sanity bound against a corrupt length asking for an unbounded allocation, set well above anything a real entry can reach. A skip past it also logs. Reading a record now checks the blob's version byte. Persisting blobs made that byte part of an on-disk format, where before it only ever lived in memory; without the check, a later layout change that bumped it but left the file version alone would read old records through accessors that no longer match them. Also corrects the speed claim in the docs, which quoted the projection rather than the measurement.
Eight test functions and fourteen subtests for one file format was more than the change needs. What is left is one case per way it can go wrong: the round trip, the format being detected rather than configured, the downgrade path, the corruption contract, and the two regressions the review turned up. Dropped the size comparison, which asserted little and belongs in the change description rather than the suite; the header-only and wrong file-version corruption cases, neither of which guards logic that can plausibly rot; and the same-format read-back combinations, which the round trip already covers. Folded the separate write-format test into the detection table, which now also checks the bytes on disk, so one table covers the default, the option selecting the writer, and the reader ignoring the option.
folbricht
force-pushed
the
cache-raw-file-format
branch
from
September 5, 2026 07:06
849c915 to
7c866c4
Compare
folbricht
added a commit
that referenced
this pull request
Sep 5, 2026
The memory backend stores an entry as a packed blob, the redis backend has its own binary record for the same thing, and the two have drifted: the version byte 0 against 1, a timestamp in nanoseconds against seconds, an expiry stored in one and not the other, and the prefetch flag defined twice on the same bit. Unifying them is a two step change, and this is the first step: the reader. A redis cache is shared between instances, which is the reason the backend exists, so a record in a format an instance does not recognise is a miss and an error log line on every lookup that finds it. Shipping the reader a release before the writer means that by the time anything emits the shared layout, the instances sharing a database can already read it, and a rollback lands on a build that can too. Nothing writes version 2 yet; the dispatch is here so that a later release can. That fixes the version constants now rather than later, since the reader has to recognise exactly what the writer will produce. blobVersion goes to 2, past the 1 the redis record already uses, and is written into the blob rather than left to the zero value an allocation happens to carry. 0 and 1 are both taken, and a record that either backend may read cannot be ambiguous about which layout it is in. Writing blobVersion makes the raw cache file's records version 2, so the file version goes to 2 with them. A version 1 file holds the same layout under blobVersion 0 and is now refused by the header check rather than losing every record one at a time to the per-record check, which reports a cold start instead of silently coming up empty. Those two constants have to move together and nothing in the types can say so, so a test pins both. It is the only thing that catches them drifting: a round trip writes and reads with the same constant and passes either way. That format is unreleased, no tag contains #614, so this costs nothing outside a dev build.
folbricht
added a commit
that referenced
this pull request
Sep 5, 2026
The memory backend stores an entry as a packed blob, the redis backend has its own binary record for the same thing, and the two have drifted: the version byte 0 against 1, a timestamp in nanoseconds against seconds, an expiry stored in one and not the other, and the prefetch flag defined twice on the same bit. Unifying them is a two step change, and this is the first step: the reader. A redis cache is shared between instances, which is the reason the backend exists, so a record in a format an instance does not recognise is a miss and an error log line on every lookup that finds it. Shipping the reader a release before the writer means that by the time anything emits the shared layout, the instances sharing a database can already read it, and a rollback lands on a build that can too. Nothing writes version 2 yet; the dispatch is here so that a later release can. That fixes the version constants now rather than later, since the reader has to recognise exactly what the writer will produce. blobVersion goes to 2, past the 1 the redis record already uses, and is written into the blob rather than left to the zero value an allocation happens to carry. 0 and 1 are both taken, and a record that either backend may read cannot be ambiguous about which layout it is in. Writing blobVersion makes the raw cache file's records version 2, so the file version goes to 2 with them. A version 1 file holds the same layout under blobVersion 0 and is now refused by the header check rather than losing every record one at a time to the per-record check, which reports a cold start instead of silently coming up empty. Those two constants have to move together and nothing in the types can say so, so a test pins both. It is the only thing that catches them drifting: a round trip writes and reads with the same constant and passes either way. That format is unreleased, no tag contains #614, so this costs nothing outside a dev build.
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.
The second of the three items left on the constrained-hardware list. Since #610 an entry is already one packed blob, so a file of those blobs is almost free to produce and to load, where the current file re-encodes every entry as JSON with the message base64'd inside it.
Numbers
20 000 entries, idle i7-7600U:
The earlier storage investigation projected 165x on write and 31x on read, so it is worth saying why the real figures are lower. Its write prototype wrote into a counting writer that discarded the bytes, so it never did the copy a real write does. Its read prototype skipped validating the messages; keeping that costs about 15 ms of the 24 ms here, and dropping it would make the read ~8 ms and ~19x. Size came out better than projected because these fixtures are short single-A answers, where JSON's field names and base64 weigh proportionally more.
I kept the validation. It matches what the JSON path does, keeps a corrupt record out of the cache instead of letting it take up an entry until a lookup evicts it, and 15 ms once at startup is not the cost worth optimising here.
Format
The magic starts with a NUL so it can't be taken for the JSON format, whose records start with
{, and so anything that opens the file can see it is binary. The version belongs to the file rather than to each record: it is checked once and a file can't hold a mix. The version byte inside a blob stays reserved for a record that travels on its own, which is what the Redis work would need.Configuration
file-formatisjson(default) orraw, and an unrecognised value is a config error rather than a silent fallback.It controls writing only. Which format is read comes from the file itself, so turning the option on or off keeps an existing cache instead of dropping it on the floor, in either direction. That seemed worth the five-byte peek; the reader was already buffered, so
Peekwas free.jsonrawjsonThe default stays
jsonfor now: a raw file is unreadable to a build that predates it, and while that degrades to a cold start rather than a failure, the format is worth leaving to soak for a release before the default moves.Corruption
Same contract as the JSON format, with one difference the framing forces. A record that doesn't decode is skipped and reading continues. A length that can't be trusted has no next record to find from, so reading stops there and keeps what it already has, and a length beyond the largest possible record is refused rather than allocated for; a corrupt four-byte length must not ask for gigabytes.
Tests
Round-trip including queue order and byte-identical rewrite; the format being detected rather than configured, across all four write-then-read combinations through the real backend; a raw file refused by the JSON reader, which is the downgrade path; and the corruption contract, covering a truncated tail, an implausible length, a corrupt message that costs only its own record, a header-only file and a wrong version.