Repository navigation
fcvm-fleet: tile AMI from ejc3/fcvm main 71bb448e (boot prefetch) - #717
Conversation
Tile image ami-098e582592b24a025 -> ami-0315bf37f11b3f577.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Tile AMI declaration fcvm-fleet.tf |
The default AMI changes to ami-0315bf37f11b3f577. Its description now records recipe 71bb448e, a 2,891-piece, 422 MiB boot prefetch with NVIDIA modules first, and the renderer’s wait for /dev/nvidia-uvm-tools. |
Priority:
Estimated code review effort: 1 (Trivial) | ~3 minutes
Change: Feature
Possibly related PRs
- ejc3/aws#698: This PR also updates the default tile AMI; this change replaces the image introduced there.
- ejc3/aws#713: This PR sets the AMI that this change replaces with the boot-prefetch image.
Merge Risk: 
78723
-
No blocking issue is established; validate the AMI and launch templates in AWS before deployment.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly identifies the tile AMI update, its source commit, and the boot-prefetch focus. It matches the main changes in the pull request. Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
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. |
|
@codex review Please review current head 7872302. The merge round is independently checking private AMI ownership, snapshot/recipe provenance, and deployed launch-template consumption. |
✅ Action performed
|
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Merged as 7dc989d and applied together with #720 from main 7cf2ccd. Independent artifact review verified private Fleet AMI/snapshot ownership, the actual builder’s S3 recipe hashes, successful build phases, and unchanged renderer/driver lineage. The combined merged tree passed all 52 fleet tests on ARM. Saved-plan apply changed only the coordinator AMI allowlist and the three tile launch templates. AWS readback confirms ami-0315bf37f11b3f577 at tile template versions 8/8/5, with IMDSv2 retained. Coordinator revision 55 remains healthy at 1 running/0 pending; no forced tile or host launch/restart was performed. Final full Terraform follow-up is empty. The separate Ohio quota drift was reconciled by resubmitting its existing 3,072 request; AWS approval is still pending, with 1,600 currently approved. |
Moves the tile AMI to the image built from ejc3/fcvm main 71bb448e (ejc3/fcvm#1407). The host AMI is unchanged. #713 and #716 are merged, so this is based on main.
fcvm_fleet_tile_amis)A fresh tile spent 45 to 84 s from launch to first frame because its root volume is made from the AMI's snapshot and fetches each block from S3 on first read (
systemd-modules-load35.4 s for 190 MiB of NVIDIA modules, the renderer unit 8.7 s). The new image carriesfcvm-fleet-prefetch.service, which reads the 2,891 pieces (422 MiB) that a first boot read, early and 256 at a time, and a renderer unit that waits for/dev/nvidia-uvm-tools.Built with
fleet/ami/build-ami.shfrom main: revised from the 728be8c3 tile image (same renderer;fleet/rendereris identical), phasestrace,units,bake. Builder terminated.measure-boot.sh tile(g4dn.xlarge, on demand because spot had no capacity at the time, launch request to first frame over HTTPS, capacity refusals not counted):On the new image
systemd-modules-loadtakes 2.9 to 3.0 s and the renderer unit 3.7 s; the CDI "failed to stat /dev/nvidia-uvm-tools" restart that the image before the unit fix logged on every boot is gone. All test instances terminated.Not changed here: the root volume's settings in the tile launch templates. Reading the 421 MiB from a volume made from the AMI's snapshot took 2.8 to 6.2 s at gp3 defaults and 0.55 s at 1000 MiB/s and 4000 IOPS; tile launches with that volume reached the first frame in 19.5 s (default 32 to 37 s on the intermediate image). It costs more per volume-hour, so it is for the owner to decide. A volume initialization rate of 300 MiB/s read it in 6.2 s, no faster than the default.
Plan: only the AMI default changes; running tiles keep their image until relaunched. Tiles stop as they do today.
Tested:
python3 -S -B scripts/test-fcvm-fleet.pyOK,terraform fmt -check.Summary by CodeRabbit