Hand the generated payload to the S3 cache write - #9440
Merged
Conversation
Retrieve generated the payload for the response and then deferred a Store job that generated it a second time to write it. Every cache miss therefore serialized twice, which matters most while a cache is cold and misses are the common case. Store now has nothing left to do beyond calling S3Cache::Write, so it goes, and Retrieve defers the write itself with the payload it already built. The key is passed with it rather than recomputed in the job. Store derived the key from the record it reloaded through its GlobalID, so a record that changed between the request and the job running would have had the old payload filed under the new version's key, where it would then be read back as current. Capturing key and payload together keeps them the same version. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VDJXJXDiyrvKZBnXMuqGif
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.
Each of the three S3 caches (
Solution::CachedSerializedView,Concept::CachedContent,Exercise::CachedContent) generated its payload twice on every cache miss: once inRetrievefor the response, and again inside the deferredStorejob in order to write it.Retrievenow defers the write itself, passing the payload it has already built. That halves the work a miss costs, which matters most while a cache is cold and misses are the common case.With the regeneration gone,
Storedid nothing beyond callingS3Cache::Write, so it's removed from all three.The key is passed too, not recomputed
Worth a look in review, as it's a behaviour change rather than just a saving.
Storederived the cache key from the record it reloaded through its GlobalID, so it read whatever state that record was in when the job ran. If the record changed between the request and the job, the payload generated from the old version would be written under the new version's key — and then read back as if it were current.Passing the key alongside the payload keeps the two on the same version by construction. The trade is that when a record does change mid-request, we now write an entry that is immediately superseded rather than opportunistically filling the new version's; the next read regenerates it. That seemed clearly preferable to being able to serve stale content.
Testing
Storeunit tests with coverage onRetrieve: that a miss generates exactly once and hands that payload to the write, and that the write is keyed on the version that was serialized.test/commands/{solution/cached_serialized_view,concept/cached_content,exercise/cached_content,s3_cache}all pass; rubocop clean.test/controllers/tracks/community_solutions_controller_test.rbhas 11 pre-existing asset-pipeline errors (The asset 'track.css' was not found in the load path) that reproduce identically on an unmodifiedmain, so they're untouched by this.🤖 Generated with Claude Code
https://claude.ai/code/session_01VDJXJXDiyrvKZBnXMuqGif