Conversation
83aa8d3 to
cd9543f
Compare
cd9543f to
bdac30a
Compare
bdac30a to
d34d4b6
Compare
d34d4b6 to
32b28f7
Compare
a16831d to
d17e5b7
Compare
| -- caps a user's minutes per half hour across templates, can look up one user's | ||
| -- rows through that prefix. The upsert's conflict target names the same | ||
| -- columns in the parent's order, satisfied by the unique index. | ||
| CREATE TABLE template_usage_stats_session_families ( |
There was a problem hiding this comment.
Do we need this table? My instinct is that in the database we should only need to record per app rather than family. The family grouping seems more like a display-time thing to me that we do in code. We need the individual counts anyway to provide the breakdown in the dashboard, right? So eventually this table and associated queries would go unused.
I think the reduction in query complexity would be worth it, unless summing families is prohibitively expensive or something.
There was a problem hiding this comment.
Dropped it. Folding in Go is within noise of the family table on reads, and the rollup gets ~20% faster with ~30% less storage. Folding in SQL is the slow path, that's the cost worth avoiding. Can share numbers.
The registry is out of SQL now (@app_families and its dbauthz validation are gone) and attribution applies to all history instead of freezing at rollup time. I also grouped GetTemplateInsightsByTemplate by app so Prometheus and the API agree.
Semantic change: a family total is the sum of its apps, not distinct minutes, so VS Code + Cursor in the same minute counts twice. No-op today since agents report one name per family, starts mattering with #28338. The report's rows already overlap and the UI divides by the row sum, so this keeps families consistent with the per-app breakdown.
|
|
||
| COMMENT ON COLUMN template_usage_stats_session_families.usage_mins IS 'Total minutes the user has been using the family. Minutes shared by two apps of the family count once.'; | ||
|
|
||
| CREATE TABLE template_usage_stats_session_apps ( |
There was a problem hiding this comment.
I forget what we landed on in the RFC, but maybe this should be a jsonb column just to be consistent with the web app minutes? Might need to rename the existing column web_app_usage_mins or something though
There was a problem hiding this comment.
That was my first experiment but the reads are x2.4 slower compared to a separate table, I can provide the benchmark results for this. It would've let us drop the digest, the FK and the delete/insert pair, but not worth 2.4x on the insights read.
066a75f to
44025d6
Compare
Replace the fixed family-minute columns with per-app and per-family child tables, and skip the child writes for buckets whose session usage digest is unchanged.
Drop template_usage_stats_session_families and fold app names into families in Go, so the registry no longer reaches SQL and attribution applies to every bucket rather than only the ones the rollup revisits.
44025d6 to
cea8c2f
Compare
2 participants
Store per-app session minutes in
template_usage_stats_session_apps, replacing the fixed family-minute columns. A new app no longer requires a rollup schema change, and the API, Prometheus, and telemetry outputs are unchanged.Reads group app names into families through the registry in
codersdk, so the registry never reaches SQL and attribution applies to all history rather than only the buckets the rollup still revisits.GetTemplateInsightsByTemplate, which feeds Prometheus, groups by app for the same reason, so both surfaces report a family identically. A digest with length-prefixed names skips child-row writes for unchanged buckets without confusing names that contain delimiters.Semantic change: a family total is the sum of its apps, not the distinct minutes any of them was active, so a minute two apps of one family share counts in both. Reported numbers are unchanged today, because every agent reports one canonical app name per family. The difference appears once #28338 lands the producers and clients send names such as
cursororzed. The apps report already overlaps across rows, and the dashboard builds its percentage denominator from the sum of the rows, so a family row stays consistent with the per-app breakdown it will sit above.Migration
000596: backfill the family totals the fixed columns recorded, sftp included, under the family name as the app name, which the registry maps back to itself. Workspace web-app usage stays separate. The migration alterstemplate_usage_stats; the exclusive locks the ALTERs acquire last until the migration transaction commits. Downgrading folds app names back into the five fixed columns and discards the per-app detail. Already-merged migration000590is unchanged.The second commit is the response to review and reads on its own; the first is the rest of the work squashed.
Follows merged #28337 and #29134; #28338 enables the producers. Covers the storage portion of #27413. Dynamic external projections (#27411 and the remaining #27413 work) and the connection-log migration (#27412) remain separate.
Design decisions and performance context
work_mem=8MB.template_usage_stats: jsonb is cheaper to write and store but roughly 2.3x the read, and it widens the main row by 22% for every query that scans it.GetTemplateInsights429 ms with the family table against 444 ms folding in Go, 777 ms folding in SQL, and 1034 ms from a jsonb column. Cold rollup over 422k agent stats 3386 ms against 2687 ms. Storage 221 MB against 151 MB. These are fixture measurements, not production guarantees.Description updated by Coder Agents for @EhabY.
🤖 Generated with Claude Code