Utilize idle tabs in puppeteer for concurrent rendering #3860
Conversation
![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: 234084aa5b
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR refactors the prerenderer to enable concurrent rendering across multiple Chrome tabs per server, replacing the previous per-realm serialization model. The changes introduce tab-level queuing and a configurable per-realm tab maximum to optimize resource utilization.
Changes:
- Removed global per-realm serialization, moving concurrency control to tab-level queuing within PagePool
- Introduced
PRERENDER_REALM_TAB_MAXenvironment variable to limit concurrent tabs per realm - Updated RenderRunner to use a lease/release pattern with try-finally blocks for proper resource cleanup
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/realm-server/tests/prerendering-test.ts | Added TestSemaphore and comprehensive tests for concurrent tab usage, queuing behavior, and tab reassignment logic |
| packages/realm-server/prerender/render-runner.ts | Refactored to acquire page leases with release callbacks and ensure cleanup via try-finally blocks; simplified auth change detection |
| packages/realm-server/prerender/prerenderer.ts | Removed per-realm promise chaining and global semaphore acquisition; simplified retry logic by delegating concurrency control to PagePool |
| packages/realm-server/prerender/page-pool.ts | Implemented TabQueue for tab-level queuing, added tab selection logic favoring idle aligned tabs, and introduced transitioning/closing flags to prevent race conditions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 234084aa5b
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 13 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Preview deployments |
This PR updates how our prerenderer works so that all chrome tabs are used for rendering, such that a single server can concurrently handle multiple render requests.
Also, it started to drive me nuts, so i added additional logic to spit out lots of detail when we get a
fetch failederror in our host tests, enough so that we should be able to feed the logs thru codex to actually fix these flaky tests that result infetch failednow.PRERENDER_REALM_TAB_MAXenv var to parameter store and infra (let's start with 4 to keep our current SLA)