fix(config-advisor): Rule 2 vCPU guard for fat-executor EC2 fleets + fixture integrity (re-land of #195, stacked on #197) - #198
Draft
suthan-AWS wants to merge 2 commits into
Conversation
… 32c sources, promote throughput-bound fleets
Re-land of the 32c worker-awareness work onto main (the original merge
landed on a feature branch, not main), reconciled with the tier-table
disk sizing that shipped in the meantime.
PRESERVE (_preserve_xlarge_source): a Serverless source that ran
healthily on 32-vCPU workers keeps its shape instead of being silently
demoted through the 4/8/16c sizing matrix. Memory is right-sized from
measured peak JVM heap x1.25 into the three discrete 32c tiers
(54/108/219G usable): heap x2 would round back up to the wasteful tier
because lazy GC inflates peak on oversized executors. Guards: cores==32
Serverless source, spill <=10GB, fetch-wait <=20%, no retried stages.
Motivating field report: a healthy 32c/219G x12 source at 36% memory
utilization was recommended Small 4c/27G x79; the proven-best manual
config kept 32c and cut memory one tier (108G) — exactly what
preservation now produces.
PROMOTE (_promote_to_xlarge): fleets >=192 total vCPU consolidate onto
32c/219G workers when the profile is throughput-bound. Per-vCPU billing
makes equal-vCPU consolidation cost-neutral on compute; the win is
second-order (less N-to-N fan-in, more local shuffle reads, higher
per-disk IOPS, less driver coordination). Benchmarked 26-38%
faster/cheaper on TPC-DS 3TB at identical 192 vCPU; live A/B of actual
advisor outputs measured -7.5% cost at wall-clock parity, driven by
disk-hours (8 vs 60 attached disks).
Promotion gates (all must hold) — consolidation LOSES when shuffle
serving is IOPS/fan-in-bound or execution memory is contended:
- spill <=10GB. Field confirmation: a manual 32c probe on a
5.2TB-spill hash-agg job amplified spill 12.6x, collapsed map-side
combining, and hit its 3h timeout unfinished at 10.6x the billed
vCPU-h of the 16c run. This veto rejects that job at three orders
of magnitude past threshold.
- fetch-wait <=10% (stricter than preservation's 20%: promotion moves
to an unproven shape; validated points are 0% wins and a 27% source
where consolidation amplified fetch-wait to 75%, a 2.6x loss)
- no retried stages; serving floor satisfied by the consolidated host
count; shuffle write >=32GB per consolidated worker
EC2 sources are eligible: the signature is workload-intrinsic and the
platform-dependent signals transfer conservatively (EC2 overstates
spill and disk-driven fetch-wait, so clean-on-EC2 is stronger evidence).
Promoted memory is always the 219G tier (the only 32c tier preserving
6.75 GB/core); preservation right-sizes from the next run's event log.
Disk on the promoted shape flows through the tier-table rule (32c has
real throughput tiers up to 1.0 GB/s at 2000G).
Tests: tests/test_worker_32c.py (36 checks). Fixtures: promote-win
(multijoin 16c source), veto (IOPS-bound 16c source), preserve
(tier-cut TPC-DS source). Golden baseline: +3 apps (25 total), all 22
existing entries byte-identical.
… loudly on unparseable fixtures Rule 2 (EC2 over-provisioned cap) triggered on executor COUNT alone (>150), so a fat-executor idle fleet escaped work-based sizing: 100 x 32-core = 3,200 vCPU at 80% idle is only 100 executors, and the waste was copied into the Serverless recommendation. Guard is now orig_executors>150 OR orig_vcpu>1200 (the 150-executor guard at the 8-core executors Rule 2 was calibrated on). Same fix shape as Rule 3's vCPU-based guard for Serverless sources. Efficiency/shuffle conditions unchanged — busy fat fleets (eff>=0.40) are not affected, and the existing 118x15c fixture (1,770 vCPU, eff 0.73) confirms no change. 3 new unit checks (fat-idle capped, small-fleet unguarded, fat-busy unchanged). Also: ec2_migration/eventlog_v2_00g6c06tphv92o0b.json had an invalid JSON escape (bare quote inside a failure-reason string) since addition; the loader silently skips unparseable files, so it contributed ZERO coverage while appearing protective. Repaired (6 occurrences escaped; now parses: 273 stages, 26th golden app) and the golden harness now fails loudly on unparseable fixtures so silent coverage rot can't recur. Golden regenerated: 26 apps, all 25 pre-existing unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-land of #195 onto main. Stacked on #197 (32c re-land) — the first commit here is #197's; review only the tip commit. Will mark ready once #197 merges.
What
orig_vcpu > 1200, not justorig_executors > 150: fat-executor fleets (e.g. 96 executors x 16 cores = 1536 vCPU) were slipping past the executor-count guard while being exactly the over-provisioned pattern Rule 2 exists for.ec2_migrationfixture00g6c06tphv92o0b(six bare-quote escape defects made it unparseable, so it was silently skipped); it now parses and pins Small x384 as the 26th golden app.Tests
test_exec_limits_rule3.pyextended with the vCPU-guard cases — all pass--update-golden --reason; all prior entries unchanged except the newly-parseable fixture's addition