Write the shared cache record format in the redis backend - #632
Merged
Conversation
The second step of the unification. The reader landed first so that instances sharing a database can already read this format before anything produces it; this switches the writer over. Store builds a blob under the empty key. The redis key already encodes the question, so repeating it in the value would only cost bytes, and the accessors need no change to read one: message() offsets by the key lengths, which are zero. That leaves 18 bytes per record over the old format, 8 for an expiry redis does not need since it has its own TTL and 10 for the empty key region. The drift this removes is not only cosmetic. The old format kept its timestamp in whole seconds, and Unix() truncates, so a redis-cached record was aged up to a second harder than the same entry in memory and the two backends did not return the same TTL for one answer. TestBackendsAgeIdentically now runs both sides through one layout, which is what makes it a regression test rather than a description. encodeCacheAnswer goes, and with it the pooled buffer handling in Store, since newCacheBlob returns its own allocation and can be handed to a background write as it is. That also retires the tests covering the old encoder's buffer aliasing, which asserted semantics that no longer exist, and the one encoding concurrently to show the pool is safe: TestMemoryBackendConcurrentStoreAndLookup drives the same pool from eight goroutines under -race. Reading version 1 keeps a fixture for the format now that nothing in the package writes it. This is not a performance change. Building a blob is ~180ns slower than the encoder it replaces, and the read side is unchanged since both layouts store the message as packed wire and both have to unpack it. Both are noise next to the round trip every lookup already pays.
folbricht
force-pushed
the
cache-format-writer
branch
from
September 5, 2026 09:20
e05595d to
ef0e20c
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.
Follows #631, which taught the Redis backend to read the shared record layout. This switches the writer over to it.
Hold this until #631 has shipped in a release. The point of the split is that every instance can read the format before anything writes it, which only holds once the reader is actually deployed and running.
What changes
Storebuilds a blob under the empty key. The Redis key already encodes the question, so repeating it in the value would only cost bytes, and the accessors need no change to read one:message()offsets by the key lengths, which are zero.That leaves 18 bytes per record over the old format, 8 for an expiry Redis does not need since it has its own TTL, and 10 for the zeroed key region. On the 124 B/record measured in #614 that is about 15%.
encodeCacheAnswergoes, and with it the pooled buffer handling inStore, sincenewCacheBlobreturns its own allocation and can be handed to a background write as it is.The drift this removes
Not only cosmetic. The old format kept its timestamp in whole seconds, and
Unix()truncates, so a Redis-cached record was aged up to a second harder than the same entry in memory, and the two backends did not return the same TTL for one answer.TestBackendsAgeIdenticallynow runs both sides through one layout, which is what turns it from a description into a regression test.Not a performance change
Worth saying plainly so it is not read as one. Measured at 300k iterations, 3 runs, single A answer:
encodeCacheAnswer, removednewCacheBlob, its replacementBuilding a blob is about 180ns slower, because it also writes the key region. The read side does not move: both layouts store the message as packed wire, so both have to unpack it, and those 9 allocs are
dns.Msg.Unpackeither way.Both are noise next to the round trip every lookup already pays. A localhost Redis GET is tens of microseconds, so the codec is a low single-digit percentage of the operation and less over a real network. The justification is consistency, not speed.
Compatibility
Reading version 1 stays, so a shared cache keeps working while instances are on either side of the upgrade. It does not have to stay for long: every record is stored with a TTL, since
Storederives one fromExpiryand returns early if it has already passed, so version 1 records drain within one DNS TTL of the last instance writing them stopping. Dropping the version 1 decoder is a later change.Tests
Three came out with the encoder. Two covered
encodeCacheAnswer's buffer aliasing, semantics that no longer exist now that the encoder returns its own allocation. The third encoded concurrently to show the shared buffer pool is safe, whichTestMemoryBackendConcurrentStoreAndLookupcovers: everyStorebuilds a blob, so it drives the same pool from eight goroutines under-race.Reading version 1 keeps an
encodeVersion1fixture, since nothing in the package writes that format any more. It goes when the decoder does.