Loadtest: make MySQL max open conns tunable (#44802)#48472
Conversation
Replace the hardcoded FLEET_MYSQL_MAX_OPEN_CONNS=10 in the loadtest terraform (both the root ecs.tf stack and the infra/ stack) with a mysql_max_open_conns variable defaulting to 20, applied to both the writer and read-replica pools. The loadtest runs on R-class Aurora instances with ample connection headroom, so 20 is safe and reduces enrollment connection-pool contention. Also document per-tier connection pool sizing in the AWS reference architecture: 20 for R-class instances (25k+ hosts), 10 for the smaller T-class instances (5k/10k hosts) whose low default max_connections (~90/135) leaves little headroom.
…ative default Address review feedback on #44802: drop the explanatory connection-pool section from the reference architecture in favor of a per-tier FLEET_MYSQL_MAX_OPEN_CONNS column (10 for T-class 5k/10k tiers, 20 for R-class 25k+ tiers), trim the terraform variable descriptions, and keep the loadtest default at the conservative 10 (override per run).
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Updates the loadtesting Terraform stacks to make Fleet’s MySQL connection pool size configurable instead of hardcoded, so operators can tune per-container DB connections based on Aurora capacity and loadtesting scale.
Changes:
- Adds a
mysql_max_open_connsTerraform variable (default10) to both loadtesting Terraform stacks. - Wires
mysql_max_open_connsinto both writer and read-replica Fleet MySQL pool env vars (FLEET_MYSQL_MAX_OPEN_CONNSandFLEET_MYSQL_READ_REPLICA_MAX_OPEN_CONNS). - Updates AWS reference architecture documentation (content excluded by policy).
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| infrastructure/loadtesting/terraform/variables.tf | Adds mysql_max_open_conns variable for the root loadtesting stack. |
| infrastructure/loadtesting/terraform/infra/variables.tf | Adds mysql_max_open_conns variable for the infra/ loadtesting stack. |
| infrastructure/loadtesting/terraform/infra/locals.tf | Applies the variable to both writer and read-replica pool env vars. |
| infrastructure/loadtesting/terraform/ecs.tf | Applies the variable to ECS task env vars for both writer and read-replica pools. |
| docs/Deploy/Reference-Architectures.md | Updates connection pool sizing guidance (content excluded). |
Files excluded by content exclusion policy (1)
- docs/Deploy/Reference-Architectures.md
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA new Terraform input variable 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@infrastructure/loadtesting/terraform/infra/variables.tf`:
- Around line 46-50: The `mysql_max_open_conns` variable description is using
the wrong sizing term and an outdated formula. Update the description in
`variables.tf` to reference `fleet_task_count` instead of `fleet_containers`,
and make it clear that this setting applies to both the writer and read-replica
pools so the worst-case connection math reflects the total impact. Keep the
change localized to the `variable "mysql_max_open_conns"` block and adjust the
wording to match the stack’s actual budget.
In `@infrastructure/loadtesting/terraform/variables.tf`:
- Around line 35-39: Update the mysql_max_open_conns variable description to
reflect the correct worst-case connection budget: it should mention that this
setting applies to both the writer and read-replica pools, and that a single
Aurora instance can see roughly 2 * fleet_containers * mysql_max_open_conns when
reader traffic collapses onto the writer. Keep the change limited to the
variable description in variables.tf so the wording matches the actual
connection behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c095ea4-e2b9-432d-8023-735de43b1014
⛔ Files ignored due to path filters (1)
docs/Deploy/Reference-Architectures.mdis excluded by!**/*.md
📒 Files selected for processing (4)
infrastructure/loadtesting/terraform/ecs.tfinfrastructure/loadtesting/terraform/infra/locals.tfinfrastructure/loadtesting/terraform/infra/variables.tfinfrastructure/loadtesting/terraform/variables.tf
|
@rachaelshaw Please approve the doc part of this PR |
Replace the hardcoded FLEET_MYSQL_MAX_OPEN_CONNS=10 in the loadtest terraform (both the root ecs.tf stack and the infra/ stack) with a mysql_max_open_conns variable, applied to both the writer and read-replica pools.
Also document per-tier connection pool sizing in the AWS reference architecture: 20 for R-class instances (25k+ hosts), 10 for the smaller T-class instances (5k/10k hosts) whose low default max_connections (~90/135) leaves little headroom.
Related issue: Resolves #44802
Summary by CodeRabbit