Skip to content

refactor(BA-6927): rename usage_bucket_entries.amount to resource_usage#12968

Closed
jopemachine wants to merge 2 commits into
fix/BA-6927-usage-bucket-cross-productfrom
refactor/BA-6927-rename-resource-seconds
Closed

refactor(BA-6927): rename usage_bucket_entries.amount to resource_usage#12968
jopemachine wants to merge 2 commits into
fix/BA-6927-usage-bucket-cross-productfrom
refactor/BA-6927-rename-resource-seconds

Conversation

@jopemachine

@jopemachine jopemachine commented Jul 20, 2026

Copy link
Copy Markdown
Member

📚 Stacked PRs

Merge in order, bottom-up. Each PR is based on the one above it, so its diff shows only its own layer.

Summary

  • Rename usage_bucket_entries.amount to resource_usage. The column holds resource-seconds, not a resource amount, and under the old name amount * duration_seconds reads like the quantity a caller wants — when both columns are already sums and their product is a cross product, which is exactly the shape of the bug fixed in fix(BA-6927): stop cross-multiplying usage bucket amounts and durations #12965.
  • resource_usage is what kernel_usage_records and the three parent bucket tables already call this quantity, so the entries table now speaks the same vocabulary instead of introducing a second name for one concept.
  • Mechanical rename across the ORM model, the two repository read paths (resource_usage_history and fair_share) and the tests, plus an alembic column rename. No behaviour change.
  • Split out of fix(BA-6927): stop cross-multiplying usage bucket amounts and durations #12965 so the correctness fix can be reviewed without the rename churn, and so the rename can be dropped independently if it is not wanted. The table is internal — only the three repository modules read it, and the GraphQL/REST surface exposes the bucket tables' resource_usage JSONB rather than these entries.

Test plan

  • pants test tests/unit/manager/repositories/resource_usage_history/ tests/unit/manager/repositories/retention/
  • pants fmt/fix/lint pass on the changeset; pants check (mypy) clean
  • Confirmed no consumer outside repositories/{resource_usage_history,fair_share,retention} references the column

Resolves BA-6927

🤖 Generated with Claude Code

@github-actions github-actions Bot added the size:M 30~100 LoC label Jul 20, 2026
@github-actions github-actions Bot added comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated labels Jul 20, 2026
jopemachine added a commit that referenced this pull request Jul 20, 2026
The original PR was opened with the wrong head branch and reopened as
#12968.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
FairShareAggregator accumulated the raw resource amounts and the slice
durations of a bucket separately and multiplied them afterwards, yielding
(sum amount_k) * (sum duration_k) instead of sum(amount_k * duration_k).
The cross product inflates a bucket by the number of kernel slices folded
into it, so a user running 4 GPU kernels saw 4x their real fGPU-seconds.
The factor compounds up the hierarchy, since each level aggregates the
kernels of everything beneath it.

BucketDelta.slots becomes resource_usage -- it no longer holds an
allocation but the allocation already integrated over the slice, which is
what every other carrier of this quantity is called (the resource_usage
column on kernel_usage_records and on all three bucket tables). Naming it
resource_seconds would have put it next to duration_seconds and read as a
second duration. The three repository call sites no longer re-multiply by
the accumulated duration.

usage_bucket_entries.amount widens to 32 digits: it stores a daily total
per entity, and a domain-level mem bucket on a large cluster exceeds the
previous 1e18 ceiling.

Existing buckets cannot be corrected in place, in either the JSONB mirror
or the normalized entries, so the migration rebuilds both from
kernel_usage_records, which stores per-slice resource-seconds and was
never affected. The oldest retained day is excluded because retention
purges kernel records by period_end and may have truncated it; buckets
older than the kernel record retention window keep their inflated values
rather than being silently zeroed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jopemachine
jopemachine force-pushed the fix/BA-6927-usage-bucket-cross-product branch from 81f0067 to 2d41a41 Compare July 20, 2026 04:56
The column holds resource-seconds, not a resource amount. Under the old
name `amount * duration_seconds` reads like the quantity a caller wants,
when both columns are already sums and their product is a cross product
-- the same shape as the bug fixed in the parent commit.

resource_usage is what kernel_usage_records and the three parent bucket
tables already call this quantity, so the entries table now speaks the
same vocabulary rather than introducing a second name for one concept.

Mechanical rename across the ORM model, the two repository read paths and
the tests, plus the alembic column rename. No behaviour change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jopemachine
jopemachine force-pushed the refactor/BA-6927-rename-resource-seconds branch from 6a17147 to 8f0ed08 Compare July 20, 2026 04:59
@jopemachine jopemachine changed the title refactor(BA-6927): rename usage_bucket_entries.amount to resource_seconds refactor(BA-6927): rename usage_bucket_entries.amount to resource_usage Jul 20, 2026
@jopemachine
jopemachine force-pushed the fix/BA-6927-usage-bucket-cross-product branch from 2d41a41 to aceb7db Compare July 20, 2026 05:01
@jopemachine

Copy link
Copy Markdown
Member Author

Folded into #12965. The rename was split out while it was resource_seconds — a second name for a quantity the schema already calls resource_usage — which made it a debatable design choice worth isolating. Renaming to resource_usage instead makes it plain consistency with the existing columns, so it belongs with the fix.

@jopemachine
jopemachine deleted the refactor/BA-6927-rename-resource-seconds branch July 20, 2026 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated size:M 30~100 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant