feature(sizing): migrate alternator test-case configs to constraint-based sizing - #15715
feature(sizing): migrate alternator test-case configs to constraint-based sizing#15715scylla-zeus-bot wants to merge 1 commit into
Conversation
✅ Test Summary: PASSED✅ Precommit: PASSED
✅ Tests: PASSED
|
|
@scylla-zeus-bot attend to review comment |
|
I addressed all 8 of fruch's inline review comments on PR #15715: in each flagged file, the sizing_db.vcpu constraint was changed from an exact value (8 or 2) to a minimum-bound constraint ('>=8' or '>=2'), matching the number requested in each comment, so those alternator test-case configs can now resolve on OCI's DenseIO shapes. I also removed the now-stale '# no OCI match (DenseIO min 16 vCPUs)' comment on each sizing_db: line, since under the new >= semantics OCI genuinely does match now (verified: OCI resolves each file's db role to VM.DenseIO.E5.Flex:8, which satisfies both vcpu>=8/memory>=64/disk>=1500 and vcpu>=2/memory>=16/disk>=350 patterns used across these files). The fix was committed (6abdb4e 'fix(sizing): use >= vcpu constraints in alternator-ttl sizing_db blocks') and independently reviewed by a reviewer and a pragmatist agent — both came back clean, with the reviewer noting only a trivial commit-message wording nit not worth another round. Phases run: implementer, reviewer, pragmatist (no RCA/plan needed — this was a review-comment round with unambiguous, per-comment target values). However, I could not push this commit: maia-push refused three times with an identical policy message ('refusing to push branch sizing-rollout-alternator to the fork — only maia-fix/* branches'), for a plain push, a --pr-head push, and a --pr-head --force-with-lease push. This branch's name predates this round and isn't maia-fix/*-namespaced, and per policy that's a hard, by-design refusal I must not keep retrying. So the fix exists as a commit on the local worktree branch only — PR #15715 has not been updated with it. A maintainer (or a differently-configured push path) will need to land this commit; the diagnosis and the correct fix are the deliverable of this round. |
|
drop test-cases/scylla-operator/* and test-cases/performance/*, they are not supposed to be using sizing for now |
@scylla-zeus-bot replay this one more happening into https://scylladb.atlassian.net/browse/QATOOLS-380 |
|
Logged this happening (the |
|
Done, with one blocker to flag. I dropped the sizing migration for the 5 files under test-cases/scylla-operator/* and test-cases/performance/* as requested: they're now restored byte-for-byte to their pre-migration hardcoded instance_type_db/loader/monitor fields (verified via empty git diffs against the pre-migration commit), while everything else on this branch (the alternator-ttl/longevity sizing migration, plus the earlier vcpu >= fix) is untouched. This is committed as f9b6072b8 'revert(sizing): keep operator and performance alternator configs on instance types'. Phases run: plan, implementer, reviewer, pragmatist — reviewer and pragmatist both came back clean with no findings. I also updated the PR #15715 description to remove the scylla-operator/performance bullets and add a note explaining they're intentionally out of scope for now (sizing doesn't handle k8s node config; perf-regression jobs stay pinned). However, I could not push either this commit or round 1's earlier vcpu fix (2 commits total, both local-only): maia-push refuses with 'refusing to push branch sizing-rollout-alternator to the fork — only maia-fix/* branches' — the same by-design policy refusal hit in the previous round, already exhaustively probed with --pr-head and --force-with-lease variants there, so I did not retry it again here. Net effect: the PR description on GitHub now correctly describes the narrowed scope, but the actual code changes (both the vcpu fix and this revert) exist only on the local worktree branch until someone with a push path not restricted to maia-fix/* branches lands them. |
Address review comments from fruch on PR scylladb#15715: the sizing_db vcpu constraint in 8 alternator-ttl test-case configs was an exact match (vcpu: N), which meant OCI's DenseIO shapes (16 vCPU floor) could never satisfy the constraint even though they have more than enough vCPUs. Change vcpu to a minimum-bound constraint (vcpu: '>=N') so OCI can match too. Also remove the now-stale '# no OCI match (DenseIO min 16 vCPUs)' comment on the sizing_db: line in each file, since OCI's smallest DenseIO shape (VM.DenseIO.E5.Flex:8, 16 vCPU/96 GB/6400 GB disk) now satisfies the relaxed vcpu/memory/disk constraints in every one of these blocks (verified via sct.py sizing preview). Reproducer: none — review-comment fix, verified via sct.py sizing preview on all 8 files (AWS/GCE/Azure/OCI all resolve).
6abdb4e to
3abbbb6
Compare
|
Picked up maia push blocker and landed the remaining work — branch is now at State when I took over: the What changed on top of
Verification, re-run from scratch on the final tree:
Nothing else on the branch was touched. |
|
huh? why was I tagged? |
oh sorry, it's a mistake. we have a bot named maia :) |
|
@scylla-zeus-bot rebase, and fix conflict on this one |
|
@scylla-zeus-bot rebase and solve the conflict, and make sure all review comments are addressed |
…ased sizing Replace hardcoded instance_type_db/instance_type_loader/instance_type_monitor and per-cloud instance type params with sizing_db/sizing_loader/sizing_monitor constraint dicts, per skills/migrate-to-sizing/SKILL.md (SCT-596, part of the sizing rollout epic SCT-586, following up on PR scylladb#14576). Migrated files: - test-cases/features/alternator-ttl/*.yaml (8 files) - test-cases/longevity/longevity-alternator-*.yaml (4 files) manager-regression-alternator-singleDC-set-distro.yaml was already migrated and needed no changes. test-cases/scylla-operator/* and test-cases/performance/* are intentionally left on hardcoded instance types: sizing doesn't handle k8s node configuration, and the perf-regression jobs stay pinned to fixed instance types until the sizing rollout is extended to cover them. DB vCPU constraints are minimum-bound ('>=8' / '>=2' / 16) rather than exact, so OCI DenseIO shapes -- which have a 16 vCPU floor -- can also match. Verified with `uv run python sct.py sizing preview <file>` on every file: all 12 resolve cleanly on AWS/GCE/Azure/OCI with no unresolved roles, with the OCI DenseIO memory ratio documented/adjusted where needed per the skill's guidance.
3abbbb6 to
db57e0e
Compare
|
Done. First, I confirmed all 8 of fruch's inline review comments are already resolved (he landed the vcpu >= fix and the scylla-operator/performance scope-narrowing himself between rounds, plus caught one file the earlier review round had missed) — verified via GitHub's GraphQL API, all 8 threads show isResolved: true, no other open threads exist, so no further action was needed there. Second, I rebased the branch onto the real upstream master. This took two attempts: the first rebase silently found nothing to do because this worktree's origin remote (the bot's own fork) had a stale local master ref frozen weeks behind; I caught that by cross-checking the real master SHA via the GitHub API and had the fix redone against the repo's actual upstream remote. The real rebase surfaced one genuine conflict in test-cases/longevity/longevity-alternator-3h-multidc.yaml, where upstream had rightsized instance_type_db to i8g.large while this branch had migrated that same field to the new sizing_db constraint syntax — resolved by keeping the constraint-based migration (vcpu >=2 / memory >=16 / local_disk_gb >=350, matching i8g.large's real specs and the pattern used elsewhere in this PR) while preserving upstream's unrelated tablets_initial_scale_factor addition to the same file. A reviewer pass confirmed the resolution is correct and nothing was lost from either side, the performance/scylla-operator scope-narrowing survived untouched, and commit authorship/message match the original. Pushed successfully with maia-push --pr-head --force-with-lease (the branch's mergeable status flipped from CONFLICTING to MERGEABLE). Phases run: implementer (rebase, redone once after a self-caught false negative) and reviewer; no plan/RCA needed for a pure rebase, no pragmatist needed since the only real decision was the single conflict resolution the reviewer already vetted. |
Goal
Migrate hardcoded instance types to constraint-based sizing syntax for all Alternator test-case YAML files, per SCT-596 (part of the sizing rollout epic SCT-586, following up on #14576).
Fixes: SCT-596
Changes
Replaced
instance_type_db/instance_type_loader/instance_type_monitor/gce_instance_type_db/azure_instance_type_db/gce_n_local_ssd_disk_dbwithsizing_db/sizing_loader/sizing_monitorconstraint dicts, followingskills/migrate-to-sizing/SKILL.md.test-cases/features/alternator-ttl/*.yaml(8 files)test-cases/longevity/longevity-alternator-*.yaml(4 files, including the multi-DC config)test-cases/manager/manager-regression-alternator-singleDC-set-distro.yamlwas already migrated in a prior change, so it needed no updates.For DB roles pinned at 16 vCPUs where the original memory constraint (128 GB, or 122 GB for the legacy
i3.4xlarge-based perf configs) exceeded OCI DenseIO.E5's fixed 12 GB/OCPU ratio (96 GB at 16 vCPUs), the memory constraint was lowered to>=96per the skill's OCI-compatibility guidance, with a YAML comment explaining why — other clouds still naturally resolve to their >=128 GB instances.Per review feedback, the
sizing_db.vcpuconstraint was tightened from an exact value to a minimum-bound (>=8/>=2) constraint on 9 of the migrated files — the 8 flagged in review pluslongevity-alternator-ttl-large-writes.yaml, which carried the identicalvcpu: 8constraint — so those configs can also resolve on OCI's DenseIO shapes (16 vCPU floor). The now-stale# no OCI match (DenseIO min 16 vCPUs)comments on those files were removed.Scope note:
test-cases/scylla-operator/*andtest-cases/performance/*are intentionally left on hardcoded instance types for now and are not part of this migration — sizing doesn't handle k8s node configuration (seeskills/migrate-to-sizing/SKILL.md), and the perf-regression jobs stay pinned to fixed instance types until sizing rollout is extended to cover them. Those two directories keep their originalinstance_type_*fields unchanged.No other YAML fields were modified.
Verification
Ran
sct.py sizing preview <file>on all 12 migrated files. Every role resolves on all four clouds — 12/12 files report(resolved)for db, loader and monitor on AWS, GCE, Azure and OCI, with zero unresolved rows:pre-commitpasses on the changed files (check yamlincluded).Acceptance Criteria
test-cases/features/alternator-ttl/*,test-cases/longevity/longevity-alternator-*) migrated to constraint syntaxuv run sct.py sizing previewsucceeds with no errors on each filetest-cases/scylla-operator/*andtest-cases/performance/*intentionally left out of scope, unchanged from their pre-migration hardcoded instance types