fix(BA-6927): stop cross-multiplying usage bucket amounts and durations#12965
Draft
jopemachine wants to merge 1 commit into
Draft
fix(BA-6927): stop cross-multiplying usage bucket amounts and durations#12965jopemachine wants to merge 1 commit into
jopemachine wants to merge 1 commit into
Conversation
jopemachine
added a commit
that referenced
this pull request
Jul 20, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jopemachine
added a commit
that referenced
this pull request
Jul 20, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jopemachine
force-pushed
the
fix/BA-6927-usage-bucket-cross-product
branch
from
July 20, 2026 04:13
baf0826 to
81f0067
Compare
This was referenced Jul 20, 2026
jopemachine
force-pushed
the
fix/BA-6927-usage-bucket-cross-product
branch
8 times, most recently
from
July 20, 2026 09:46
6ed3a7c to
0f9b02b
Compare
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. The deltas the aggregator hands over now carry the per-slice products already summed, which is the only shape that fixes this: (sum amount, sum duration) does not determine sum(amount * duration), so the pairing has to survive into the delta. BucketDelta held exactly those two sums and had no other purpose, so it collapses to a plain ResourceSlot per bucket. usage_bucket_entries.amount becomes resource_usage, the name kernel_usage_records, the three bucket tables and the GraphQL and REST responses already use for this quantity, so one vocabulary runs the whole way through. The column also drops its precision limit: no fixed precision is defensible here -- a domain-level daily mem bucket runs to ~1e18 byte-seconds on a large cluster -- while PostgreSQL's unconstrained numeric has no ceiling, and dropping a typmod does not rewrite the table. duration_seconds goes too. It only ever existed so readers could reconstitute the product, nothing consulted it on its own, and it counted kernel-seconds rather than wall-clock, so keeping it would leave a column that invites the same misreading. The read paths previously summed the stored figure without combining it with duration at all, returning something 300x smaller than the resource-seconds the fair share calculator assumes (it divides by capacity * lookback_days * 86400), so scheduling priorities were computed on the wrong scale -- a second defect independent of the cross product. 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 products 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
force-pushed
the
fix/BA-6927-usage-bucket-cross-product
branch
from
July 21, 2026 01:33
0f9b02b to
0567840
Compare
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
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.
Summary
FairShareAggregatoraccumulated the raw resource amounts and the slice durations of a bucket separately and multiplied them afterwards, computing(sum amount_k) * (sum duration_k)instead ofsum(amount_k * duration_k). The cross product inflates a bucket by the number of kernel slices folded into it in one observation tick — i.e. by the number of concurrently running kernels. Reported symptom, one user with 4 GPU kernels for a day:kernel_usage_recordssums to345600fGPU-seconds (correct) whileuser_usage_bucketsreads1382400(345600 * 4). The factor compounds up the hierarchy, since each level aggregates the kernels of everything beneath it, which is why domain figures looked most implausible.usage_bucket_entries.amountbecomesresource_usageand drops its precision limit.resource_usageis whatkernel_usage_records, the three bucket tables and the GraphQL/REST responses already call this quantity, so one vocabulary now runs the whole way through and the adapters no longer translate. The column holds the product directly rather than a factor readers had to multiply back out. The typmod is dropped rather than widened because no fixed precision is defensible here: a domain-level dailymembucket runs to ~1e18byte-seconds on a large cluster (measured:NUMERIC(24,6)overflows past ~10.5 TB of RAM in scope), while PostgreSQL's unconstrainednumerichas no ceiling. Renaming a column and dropping a typmod are both metadata-only — verified on 200k rows:relfilenodeunchanged, 15 ms.duration_secondsis no longer read anywhere; it stays as provenance for the usage figure beside it.capacity * lookback_days * 86400), so scheduling priorities were computed on the wrong scale — a second defect independent of the cross product.c4a91d7e05b2performs that column change and rebuilds the corrupted values: existing buckets are wrong in both the JSONB mirror and the normalized entries and cannot be corrected in place, so both are rebuilt fromkernel_usage_records, which stores per-slice resource-seconds and was never affected. Two deliberate limits: the oldest retained day is excluded, because retention purges kernel records byperiod_endat a timestamp that falls mid-day and may have truncated it; and buckets older than the kernel-record retention window (90 days by default) keep their inflated values rather than being silently zeroed — they cannot be recovered, and destroying them would lose the only usage history that remains. The fair share lookback (~28 days) sits well inside the rebuildable window, so scheduling is fully corrected.Test plan
pants test tests/unit/manager/sokovan/scheduler/fair_share/— bucket aggregation assertions corrected to resource-seconds, plus a regression class parametrized over 1/2/4/10 concurrent kernels and a full-day 4-kernel case asserting the reported3456001200instead of600for a user (2x),3600instead of1200for a project (3x) and23100instead of3300for the domain (7x). Also pins that user buckets stay keyed by(user, project)so a user active in two projects does not collapse into one bucketpants test tests/unit/manager/repositories/resource_usage_history/ tests/unit/manager/repositories/retention/pants fmt/fix/lintpass on the changeset;pants check(mypy) clean across changed files{}ALTER COLUMN … TYPE numericmeasured on a 200k-row table:relfilenodeunchanged (no rewrite), 15 msDeployment note
The migration rewrites up to 90 days of bucket rows and their entries; on a large installation this takes time.
downgrade()restores the column definition but not the inflated values — those are unrecoverable by design. Back up before applying.Follow-ups (not in this PR)
jsonb_each+jsonb_object_aggcan do it in SQL. The entries path accumulates server-side, so the two representations can drift.period_endis not in theon_conflictupdate set, so the "period_end extension" strategy described in the bucket row docstrings never actually happens.usage_bucket_entries.duration_secondssums across concurrent kernels rather than wall-clock, so it is not a meaningful standalone figure. Nothing reads it now; worth either defining it properly or dropping it.calculate_usage_capacity_ratioreturns seconds, not a ratio. The GraphQL field descriptions say so ("86400 means full utilization for one day"), so the value is intended — the name is not.ResourceSlotis a barename -> Decimalmapping with no notion of unit, so an allocation and an already-integrated value are the same type. That is what made this defect expressible; distinguishing them properly spans ~15 files and belongs in its own change.Resolves BA-6927
🤖 Generated with Claude Code