Skip to content

CORE-2611 Changed prerendering to vCPU scaling, give each task at least 2 vCPU (1 per thread) - #3123

Open
Dantemss wants to merge 8 commits into
mainfrom
change-scaling-to-vcpu
Open

CORE-2611 Changed prerendering to vCPU scaling, give each task at least 2 vCPU (1 per thread)#3123
Dantemss wants to merge 8 commits into
mainfrom
change-scaling-to-vcpu

Conversation

@Dantemss

@Dantemss Dantemss commented Aug 4, 2026

Copy link
Copy Markdown
Member

https://openstax.atlassian.net/browse/CORE-2611

The last prerendering fix showed that we're not as memory constrained as I though. This new setup based on vCPUs instead of memory should give us more uniform prerender times. Also remove some max memory constraints that wouldn't be hit in practice with vCPU scaling.

@Dantemss
Dantemss requested a lite review from Copilot August 4, 2026 16:32
@Dantemss Dantemss self-assigned this Aug 4, 2026
@TomWoodward
TomWoodward temporarily deployed to rex-web-change-scaling--bt0rkf August 4, 2026 16:32 Inactive

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the prerender worker fleet sizing strategy from memory-based capacity targeting to vCPU-based targeting, aiming for more consistent prerender performance by ensuring tasks have CPU headroom.

Changes:

  • Switched EC2 Fleet target capacity units from memory-mib to vcpu, renaming the capacity parameter accordingly.
  • Explicitly reserved 1 vCPU for the prerender worker container and adjusted desired task count (32 → 16).
  • Relaxed prior maximum memory-related instance requirements that are no longer relevant under vCPU targeting.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
script/prerender/work.ts Updates inline documentation to reflect CPU-based task sizing assumptions for worker threading.
script/prerender/fleet.ts Adjusts desired task count and passes vCPU-based fleet capacity to the CloudFormation stack.
script/prerender/cfn.yml Renames and applies the vCPU capacity parameter, changes EC2 Fleet target capacity to vCPU, and sets container CPU reservation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread script/prerender/fleet.ts
Comment thread script/prerender/cfn.yml
@Dantemss
Dantemss temporarily deployed to rex-web-change-scaling--bt0rkf August 4, 2026 16:37 Inactive
@Dantemss
Dantemss requested a lite review from Copilot August 4, 2026 16:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

script/prerender/fleet.ts:66

  • The comment implies TotalVCpuCount must equal the desired task count * 2, but that relationship is a sizing policy/rationale rather than an ECS/CloudFormation constraint. Rewording here will prevent future edits (e.g., changing desired count vs. CPU per task) from being misinterpreted as "invalid" rather than a tradeoff.
// Total fleet vCPU capacity (must equal DESIRED_TASK_COUNT * 2)
const TOTAL_VCPU_COUNT = DESIRED_TASK_COUNT * 2;

@Dantemss Dantemss changed the title CORE-2611 Changed prerendering to vCPU scaling, give each task at least 1 vCPU CORE-2611 Changed prerendering to vCPU scaling, give each task at least 2 vCPU (1 per thread) Aug 4, 2026
@Dantemss
Dantemss marked this pull request as ready for review August 4, 2026 22:56
@Dantemss
Dantemss requested a review from a team as a code owner August 4, 2026 22:56
@Dantemss
Dantemss requested review from RoyEJohnson and removed request for RoyEJohnson August 4, 2026 22:56
@Dantemss
Dantemss marked this pull request as draft August 4, 2026 23:01
@Dantemss
Dantemss marked this pull request as ready for review August 4, 2026 23:01
@Dantemss
Dantemss requested a review from bethshook August 4, 2026 23:02
@Dantemss

Dantemss commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

I tested and it worked but... when the instance got killed due to low spot capacity the tasks were not moved to the new instance. I have to figure out why.

@Dantemss
Dantemss marked this pull request as draft August 4, 2026 23:17
@Dantemss
Dantemss removed the request for review from bethshook August 4, 2026 23:17
@Dantemss

Copy link
Copy Markdown
Member Author

I believe we're now using the instances efficiently. I saw a cluster running all 16 tasks in my tests and the instances were properly packed with tasks.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

script/prerender/cfn.yml:113

  • 3686 is an unexplained value coupled to the 4096 MiB instance minimum below. Since this determines how many tasks ECS can place per host, please document how it is derived (such as the intended host-memory reserve) so a future instance-requirement change does not silently invalidate the packing assumptions.
          MemoryReservation: 3686

@Dantemss
Dantemss marked this pull request as ready for review August 18, 2026 19:52
@bethshook

Copy link
Copy Markdown
Collaborator

I was tagged on this initially but maybe @TomWoodward wants to review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants