Repository navigation
perf(runtime): catch a request up through a window of recent invalidations - #445
soleil-colza wants to merge 8 commits into
Conversation
`tags.ts` held the tag view (`TagState`) and, below it, everything a tag's marks say of an entry: the marks known at no revision kept beside each other (`Marks`, `withMark`), and the validity a history or a set of marks gives an entry made at a moment. The file stood at 491 of the 500 lines the lint allows, with the view about to learn a window of recent invalidations. Those helpers move, unchanged, to `tag-marks.ts`, with `TagMark`, which `tags.ts` still exports. Nothing they do changes.
The Function's gateway client read a delivery record whole through a private method of its own (`#readWhole`), which used nothing of the client: it holds room in the isolate's read budget for the answer's length and reads the body once it has it. It moves, unchanged, beside the budget's other whole reads in `read-budget.ts` (`readWhole`), so `gateway.ts`, at 495 of the 500 lines the lint allows, has room for a tag read of its own.
A fresh isolate brings its view of the tags up by paging through the scope's whole delta, oldest first, one round trip to the authority a page — and a scope keeps a row for every tag it ever invalidated. The Function's client asked for no limit, so each page held the gateway's default of 500 tags, though a page may hold 1000 (`MAX_DELTA_PAGE_TAGS`) and the gateway already holds a larger ask to that. What the authority reads for each row is one lookup of the tag's history. The client now asks each page for 1000 tags: half the round trips for every full pull, which a regeneration waits for before it renders, and which a request behind a recent invalidation waits for up to a second.
A Function's isolate whose view of the tags is behind its scope can only bring it up through the delta: every tag the scope ever invalidated, oldest first, a round trip a page. The invalidations a request needs the view to hold are the newest, and they come last. The authority already answers the window an edge is told of (`recentDelta`): the invalidations past a revision that took effect from a moment on, each as it was recorded, the newest so many, truncated where the window held more, at the scope's revision. The gateway now answers it to a Function too, as a route of its own (`GET /tags/recent?since&from&limit`), with a shape of its own in the contract (`tagsRecentResponseSchema`): no more tags than a page, none past the revision it was read at, and no rule that a truncated answer ends at its last tag — a window cut at its oldest end is no page of the delta, and the delta's shape refuses one, or, where its newest row stood at the scope's revision, would take it for a page boundary. The Function's client reads it as the delta is read, within the same authority deadline (`Gateway.tagsRecent`); its host leaves platform tags out as `tagsDelta` does (`GatewayHost.tagsRecent`); and the runtime's host contract gains it as an optional call (`CacheHost.tagsRecent`). A gateway from before the route answers 404, which the client reports as `unsupported`, sharing what told a batch upload so. The native cache is left without it: no request there carries where its scope stands, so nothing would ask.
…tions A request whose scope stands past this isolate's view of the tags, at an invalidation made in the last ten minutes, waits up to a second for the view to catch up before it is routed (`catchUpWithScope`). The only way up was the delta, which a fresh isolate pages through from the scope's beginning, oldest first, a round trip a page: on a scope that has invalidated thousands of tags, every early request of a fresh isolate waited the whole second, and the invalidations the wait was for came last. The request now asks, beside the pull, for the window of the scope's recent invalidations past where the view stands (`CacheHost.tagsRecent`), one shared window in flight per isolate as there is one pull, and goes on once either has brought the view up. What the window brings is taken in as marks that judge what the request reads — by timestamps, as data reads and records are judged — and the view stands at its revision for that (`TagState.seen`), which is what the gate asks now. It moves nothing a regeneration claims. A window holds the newest invalidations and none before them, so: - `revision`, which a commit says its render was judged against, is still what the delta brought the view to; regenerations, captures, speculative renders and commits read it as before, and the pull goes on behind the request as it did, so the next regeneration's pull is short; - a window's marks go into a tag's history but never into what the view knows of the tag (`knownRevision`), which says whether it holds the tag whole (`holdsWhole`, `heldRevisionOf`): a mark past one the view let go of would otherwise have a commit claim the deadline it let go of; - a mark a window brought past the revision a render read against is one that render missed, whatever its time says (`#heldBy`), so a render being published answers no request at once after it; - a read of a tag still asks the host of it (`checkedAt` stays as it was), and a tag let go of is remembered at its window's revision too. A window that left some out takes its marks in but stands the view nowhere, and the request waits for the pull as before; so does one from a host without the call, or a gateway that answers it `unsupported`, which is asked no more. Tests: `recent-tags.test.ts` (the view), a regeneration after a window in `regenerate-held-tags.test.ts`, and the catch-up itself in `edge/test/scope-revision.test.ts`.
|
Arkor — Preview deployments for arkorlab-upwind Use the checkbox below to choose whether this pull request gets preview deployments.
Checking this builds the latest commit and future pushes. Unchecking stops queued and future builds; a running build can finish and an existing preview stays available. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 2 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. CodeRabbit Verify
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dcbb7e6ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| history = withTagHistory(history, mark); | ||
| latest = Math.max(latest, mark.revision); | ||
| } | ||
| this.#tags.set(tag.value, { ...known, history, recentRevision: latest }); |
There was a problem hiding this comment.
On a fresh isolate with more than 4096 historical tags, the parallel full pull can evict the window's recent invalidations from this shared LRU before reaching their revisions. seen nevertheless remains at the window's revision, so subsequent requests skip catch-up even though those marks are gone. If syncLocal returns a lagging regional copy, invalidated data becomes servable again: I reproduced validityOf changing from expired after the window to fresh after 5000 historical rows, then back to expired when the final delta page arrived. Protect the recent-window marks from historical-page eviction while requests rely on the window to proceed early.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 18 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
 [Medium impact] The PR appears safe to merge; no new blocking issue was found. SummaryAdds a recent-invalidation window beside the full delta pull, so requests can catch up sooner without changing the revision a regeneration claims.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Request behind a recent invalidation] --> B[Start or join full delta pull]
A --> C[Start or join recent window]
B --> D[Update revision and tag history]
C --> E[Keep recent marks separately]
E --> F{Window complete and marks retained?}
F -->|Yes| G[Advance seen]
D --> H[Continue when seen reaches request revision]
G --> H
H --> I[Judge request reads]
D --> J[Regeneration claims delta revision]
Reviews (4) · Last reviewed commit: "fix(runtime): stand at the delta's revis..." · Reviewed by Greptile |
A window's marks went into the tag view's one bound of tags (`MAX_TAGS_HELD`), ahead of the delta a fresh isolate pages through after them, oldest first: on a scope with more tags than the bound, the delta let the window's marks go first — the newest invalidations, the ones the view stood at the window's revision by (`seen`). A read of such a tag then asked a regional copy of it, which may be behind them still, and invalidated data could be served as fresh again until the delta reached them (Codex). The marks are now held apart (`#recent`), judged with the rest — a request's reads, what a render's view missed, a tag's latest moment — and let go of as the delta brings the view past them. Past as many as are held apart, the first let go of keeps the view at the revision the delta brought it to (`seen`), so a request behind its scope is caught up again, until the delta passes it or a window asked from there brings it back. Also from review (cubic): a window's row may carry no earlier deadline past the row's own revision, as the delta's rows may not; the route test asserts the exact revision; the cache README says the window is asked beside the delta, not before it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85f0fd68d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (held.recentRevision <= this.#revision) { | ||
| this.#recent.delete(value); |
There was a problem hiding this comment.
If a tag is invalidated again after the window is read but before a historical delta page is queried, its current row moves to the newer revision: tagsDelta selects current tag rows ordered by revision (packages/cache/src/scope/tags.ts:394), not every historical invalidation. Passing the window mark's revision therefore does not guarantee that the delta has delivered that tag. The new separate map prevents the previously reported LRU eviction, but this cleanup can still remove the mark prematurely. With a window at revision 20 containing a tag's revision-5 mark, a revision-10 page, and that tag now at revision 30, I reproduced validity changing from expired to fresh while seen remained 20, until the final page arrived. Retain the mark until the delta actually supplies equivalent tag history.
Useful? React with 👍 / 👎.
| this.#recent.delete(value); | ||
| this.#recentLostPast = Math.max(this.#recentLostPast, held.recentRevision); |
There was a problem hiding this comment.
When successive windows exceed 4096 distinct tags while the full pull remains behind, this eviction removes an invalidation without updating losses or the state used by holdsWhole. Resetting seen is insufficient for publishing safety: publishing.ts permits an in-flight render to answer through answeringAtOnce when holdsWhole, losses.lostSince, and validityOf all pass. After one victim mark followed by five 1000-tag windows, I reproduced the victim becoming fresh with holdsWhole === true and lostSince === false, so a render that missed that invalidation becomes eligible again. Record the discarded mark in the loss tracker so publishing remains blocked even after its deadline history is gone.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
A window's mark was let go of once the delta's revision passed it. But the delta answers each tag's latest row, so a tag invalidated again since the window comes only at its later revision, past pages that reach beyond the window's mark without it: the mark went, the view still stood at the window's revision (`seen`), and a read of the tag judged by a regional copy could call invalidated data fresh until the page with the tag arrived (Codex). The mark now goes once the delta brings the tag itself at its revision or later — and never on a regional read. And a mark let go of past the bound of those held apart said nothing of the loss: the tag was still held whole and a render that began before it could answer again (`answeringAtOnce`), with `seen` lowered but read by neither check (Codex, Greptile). It is now lost as a mark the delta brought is (`#lose`): recorded against the tag's revision and counted in `losses`.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5c00d76c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return this.#recentLostPast > this.#revision | ||
| ? this.#revision | ||
| : Math.max(this.#revision, this.#recentRevision); |
There was a problem hiding this comment.
When successive recent windows overflow the 4096-tag bound while the delta is behind, passing #recentLostPast does not prove that the delta restored the discarded tags: tagsDelta returns each tag's current row, so a discarded tag invalidated again can move to a later page. The per-tag cleanup now handles this case, but the overflow fallback still uses only a revision comparison. For example, evict a revision-5 victim through windows ending at revision 50, then invalidate that victim again at revision 60: an unrelated delta page past the evicted revisions can restore seen to 50 while the victim's mark remains absent and its validity is fresh. Requests requiring revision 50 then skip catch-up and can serve an invalidated regional copy. Retain recovery obligations for the discarded tags, or conservatively suppress the window revision until the full pull finishes.
Useful? React with 👍 / 👎.
…s a lost mark A window's mark let go of past the bound kept the view at the delta's revision (`seen`) only until the delta's revision passed the mark's. But the delta answers each tag's latest row: a tag invalidated again since comes at its later revision, so a pull that has only passed the mark has not brought the tag, and the view went back to the window's revision with the tag's invalidation missing — a request at that revision then skipped its catch-up (Codex). The view now stands at the delta's revision until a pull has run to its end past the lost mark, or a window asked from where the delta stands brings the marks back.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/runtime/src/cache/tags.ts">
<violation number="1" location="packages/runtime/src/cache/tags.ts:523">
P1: This lets a retrying recent-window request erase the marker for a lost tag after only a truncated delta page passed that marker. The request can then be judged at the window revision while the delta page containing the tag is still pending; only clear `#recentLostPast` when the window starts before the lost revision (or when a complete pull has finished).</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| * move, as it holds the newest invalidations and not every one. | ||
| */ | ||
| get seen(): number { | ||
| return this.#recentLostPast > 0 |
There was a problem hiding this comment.
P1: This lets a retrying recent-window request erase the marker for a lost tag after only a truncated delta page passed that marker. The request can then be judged at the window revision while the delta page containing the tag is still pending; only clear #recentLostPast when the window starts before the lost revision (or when a complete pull has finished).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/runtime/src/cache/tags.ts, line 523:
<comment>This lets a retrying recent-window request erase the marker for a lost tag after only a truncated delta page passed that marker. The request can then be judged at the window revision while the delta page containing the tag is still pending; only clear `#recentLostPast` when the window starts before the lost revision (or when a complete pull has finished).</comment>
<file context>
@@ -517,7 +520,7 @@ export class TagState {
*/
get seen(): number {
- return this.#recentLostPast > this.#revision
+ return this.#recentLostPast > 0
? this.#revision
: Math.max(this.#revision, this.#recentRevision);
</file context>
Summary
A request behind a recent invalidation no longer waits for a fresh isolate to page through the scope's whole tag history. It asks the authority for the window of recent invalidations, in one round trip, beside the full pull. Full pulls also ask for pages of 1000 tags instead of 500.
Why
catchUpWithScopewaits up to 1 s before routing a request whose scope revision is ahead of the isolate's view, when the latest invalidation is less than 10 minutes old. The only way to catch up was the delta. A fresh isolate reads it from the scope's beginning, oldest first, 500 rows a page, one round trip per page, and tag rows are never removed. On a scope with thousands of invalidated tags, every early request of a fresh isolate waited the full second, and the invalidations the wait was for arrived last.Changes
GET /tags/recent?since&from&limiton the gateway. It returns the existing recent window (recentDelta): the newest invalidations pastsincethat took effect fromfromon, at the scope revision,truncatedwhen there were more.tagsRecentResponseSchema). A truncated window is not a page of the delta, and the delta's page rule would reject it or mistake it for a page boundary.tagsRecent(since, from), under the same authority read deadline astagsDelta; platform tags are filtered; an older gateway's 404 is reported asunsupported. Plus optionalCacheHost.tagsRecentin the runtime host contract.tagsDeltaasks forlimit=1000(MAX_DELTA_PAGE_TAGS), which the route already clamps to.TagState.recent/seen:knownRevisionorcheckedAt, soholdsWhole,heldRevisionOfandsyncLocalbehave as before.seen.#recent), so a fresh isolate's delta, paging the scope's whole history after them, cannot let them go; each is let go of once the delta brings its tag at the mark's revision or later — not merely once the delta's revision passes it, since the delta answers each tag's latest row. Past as many as are held apart (4096), the first is lost as a delta mark is (#lost,losses: not held whole, an earlier render answers no more), and the view stands at the delta's revision forseenuntil a pull has run to its end past it, or a window asked from where the delta stands brings the marks back.catchUpWithScopestarts the window (shared per isolate) and the full pull together. It goes on onceseenreaches the scope revision, or once both have settled short of it — still for 1 s at most. The full pull keeps running behind the response.revision.tag-marks.ts, andreadWholeintoread-budget.ts.Tests
since,fromandlimitparsing; truncation; ordering.limit=1000on both reads; deadline parity; a 404 becomesunsupported; platform tags filtered.seento the delta's revision until the delta passes it; an evicted tag plus a window mark is not held whole; a window mark past a render's revision stopsansweringAtOnce;checkedAtstays unset; a truncated window leavesseenunchanged;unsupportedis asked once; a lost window mark is remembered.seencovers the scope.scope-revision), cache and host suites; eslint, oxfmt,tsc(runtime, host, edge, cache),check:vocabulary.Rollout
unsupported, and catches up through the delta alone, as before.tagsRecent: nothing there carries a scope revision, so it would never be asked.Review fixes
seenstood at the window's revision): the marks are held apart from the delta's tags, as above.seencame back once the delta's revision passed a lost mark, though the tag may come later): it comes back only after a whole pull past it, or a covering window.Not changed
tags.ts, a retried window clearing the lost-mark state after only a truncated delta page passed it): while a mark is lost the view stands at the delta's revision, so the retried window is asked from there (since=revision). It holds every tag's row past that revision — the rows the pages still out would bring included — and the rows at or before it are ones the delta has brought: rows only move forward, so a row there now was there when the page covering it was read. Only a window that left none out clears the state, before its own marks go in.