Repository navigation
fix(plugin): reclaim raw pages after 48h, not 7 days; v0.78.0 - #186
Merged
Merged
Conversation
Two different expiries were confused in the original table definition. `expiresAt` is when a row stops being SERVED — `render.raw.expiry`, typically the next local midnight, so <= 24h. The table's `expiration` is when the row and its BLOB are reclaimed, and it shipped at 7 days. So every raw page sat in storage for six days after it could no longer answer anything: ~3.5x the footprint for no benefit, replicated to every node. On a corpus of catalog facets that is the difference between a few GB per node and a few tens of GB. 48h leaves a full day of slack past a midnight expiry. Because it is a schema constant it also bounds `render.raw.expiryMs`, which IS configurable — set an interval near or past 48h and rows are reclaimed while still nominally servable, which reads as a silent miss rather than an error. config.js now warns above 36h, since that is the one raw-cache setting that can be made incoherent from config alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request bumps the version of @harperfast/prerender to 0.78.0 and reduces the RawPage table expiration from 7 days to 48 hours in the GraphQL schema to optimize storage footprint. Additionally, a configuration warning is introduced in config.js to alert users when render.raw.expiryMs is set close to or beyond this 48-hour limit. Feedback suggests replacing the magic number 129600000 with 36 * HOUR to improve readability.
| // nominally servable — the row and its blob vanish, and the next request is a miss that silently | ||
| // re-fetches. The schema constant is 48h (schemas/schema.graphql, RawPage), and it cannot be | ||
| // configured, so this is the one raw-cache setting that can be made incoherent from config alone. | ||
| if (target.render.raw.enabled && target.render.raw.expiry === 'interval' && target.render.raw.expiryMs > 129600000) { |
There was a problem hiding this comment.
The magic number 129600000 represents 36 hours. Using the HOUR constant imported at the top of the file improves readability and maintainability.
Suggested change
| if (target.render.raw.enabled && target.render.raw.expiry === 'interval' && target.render.raw.expiryMs > 129600000) { | |
| if (target.render.raw.enabled && target.render.raw.expiry === 'interval' && target.render.raw.expiryMs > 36 * HOUR) { |
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.
Two different expiries were confused in the table definition #181 shipped.
expiresAt(per row)render.raw.expiry, typically the next local midnight@table(expiration:)So every raw page sat in storage for six days after it could no longer answer anything — roughly 3.5× the footprint for no benefit, replicated to every node. On a corpus of catalog facets that is the difference between a few GB per node and a few tens of GB.
48h leaves a full day of slack past a midnight expiry.
The coupling this creates, and the guard for it
Because it is a schema constant it also bounds
render.raw.expiryMs, which is configurable. Set an interval near or past 48h and rows are reclaimed while still nominally servable — which surfaces as a silent miss and a re-fetch, not an error.config.jsnow warns above 36h.Verified: a 48h interval warns, 6h does not, and
expiry: midnightdoes not.Why a constant rather than config
Harper's schema is static GraphQL, so table expiration cannot be driven from config. A constant is the only option; the warning is what keeps the configurable half honest against it.
1155 plugin tests, 276 console, lint and format clean.
Found while answering a question about whether the expiration could be smaller — it could, and it should have been from the start.
🤖 Generated with Claude Code