UN-3974 [DEV] Cut dashboard cron DB time by splitting the metrics schedule by tier and indexing workflow_execution on created_at - #2265
Open
kirtimanmishrazipstack wants to merge 7 commits into
Conversation
… workflow_execution on created_at Schedule split. One schedule ran every 15 minutes and wrote all three metric tiers. Dashboard daily and monthly figures do not need 15-minute freshness, so they move to hourly — 96 runs a day becomes 24 for the expensive DAY-granularity half of the work, while the hourly tier keeps its cadence. Both schedule rows point at the same task and differ only in a `tier` kwarg; a second task name would need its own worker registration and internal endpoint for the PG path. The lock is now keyed per tier, so the two runs that collide at the top of every hour do not starve each other. Omitting `tier` still writes all three tiers, so a manual trigger never silently writes nothing. Prefilter index. The active-org prefilter measures 1,849ms per call on production — the slowest single query on the instance. Nothing on workflow_execution leads with created_at: the two composite indexes are date-ordered only within one workflow or pipeline, and the partial index is empty in steady state. The split raises this query's call count, and UN-4045 will leave three more metric queries on the same bare date-range shape, so the index lands with the split rather than after it. Built CONCURRENTLY with atomic = False and guarded against a leftover INVALID index, matching migration 0026. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc
Contributor
|
| Filename | Overview |
|---|---|
| backend/dashboard_metrics/migrations/0006_split_aggregation_schedule.py | Splits the existing schedule while preserving scheduler ownership fields and declares the revised dependency on the preceding reconciliation migration. |
| backend/dashboard_metrics/tasks.py | Adds tier-aware query execution, writes, lock keys, and aggregation reporting while retaining all-tier behavior for callers that omit the tier. |
| backend/dashboard_metrics/internal_views.py | Forwards the optional tier through the internal aggregation endpoint and preserves the task default when omitted. |
| workers/scheduler/dashboard_metrics_tasks.py | Propagates scheduled tier kwargs through the worker proxy to the backend endpoint. |
| backend/workflow_manager/workflow_v2/migrations/0029_we_created_at_idx.py | Adds the workflow-execution created_at index using the migration’s non-blocking index operation. |
| backend/workflow_manager/workflow_v2/models/execution.py | Declares the new execution-date index in model metadata. |
| backend/dashboard_metrics/tests/test_pg_periodic_task_declarations.py | Verifies split schedule declarations and preservation of existing scheduler ownership state. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
S1[15-minute schedule] --> H[Aggregation task: hourly]
S2[Hourly schedule] --> D[Aggregation task: daily_monthly]
H --> HL[Hourly tier lock]
D --> DL[Daily/monthly tier lock]
HL --> HT[EventMetricsHourly]
DL --> DT[EventMetricsDaily]
DL --> MT[EventMetricsMonthly]
Reviews (8): Last reviewed commit: "Merge remote-tracking branch 'origin/UN-..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…d cut _run_aggregation's complexity Migration 0005 used update_or_create for the PG row of the schedule it was only re-keying, which reset pg_owned to False. converge_pg_scheduler disables a row's Beat twin when the PG scheduler adopts it, so on an adopted deployment the migration would have left the aggregation with no firer at all — Beat disabled, PG no longer owning it. It now updates only task_kwargs on that row, leaving enabled and pg_owned to the scheduler that owns them. Rollback is symmetric. Threading the tier through _run_aggregation took its cognitive complexity from 25 to 27 against a limit of 15. Extracted _collect_org_metrics and _aggregate_org, and hoisted the two static metric tables to module level so they are not rebuilt per call. Names match the same extraction on #2255 so the two reconcile cleanly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc
kirtimanmishrazipstack
changed the base branch from
UN-3883-Optimize-DB-cron-queries-causing-high-DB-load
to
main
August 31, 2026 14:55
The two index migrations carried 50-60 line docstrings restating the prod plan, deployment runbook and recovery steps. That detail belongs in the PR, not in files every future agent scans. Cut to purpose and key behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc
Contributor
Unstract test resultsPer-group results
Critical paths
|
Contributor
Author
|
SonarCloud: quality gate passes. The CRITICAL is fixed; the 5 remaining MINORs are being declined.
On the S117s: these are It was only flagged as new code because the lines are new, not because the pattern is. |
kirtimanmishrazipstack
changed the base branch from
main
to
UN-3883-Optimize-DB-cron-queries-causing-high-DB-load
August 31, 2026 15:37
…into UN-3974-split-metrics-schedule-and-prefilter-index
Contributor
Author
|
@greptile-apps please re review confidence score? |
The suite runs with --no-migrations, so neither 0005 nor 0029 ever executes in CI, and nothing pinned the schedule split's behaviour at all. 46 tests, at least one per acceptance criterion. AC-1 — cadence, and the tier reaching the task. 0005 creates one row and rewrites one, both scheduler tables agreeing, and the rewrite touches neither pg_owned nor enabled: on an adopted deployment converge_pg_scheduler has already disabled the Beat twin, so handing ownership back would leave the aggregation with no firer. Separately the internal endpoint and the worker proxy are pinned to carry `tier` — that leg fails silently, since _call_internal builds a body only when a tier is given and the existing worker test called the task without one. AC-2 — the split changes no figure. Runs the real _run_aggregation three times and diffs the metrics tables: `hourly` reproduces the pre-split hourly figures exactly, and hourly + daily_monthly reproduce every row `all` writes. Two guards keep it from going vacuous, the second because mutation testing caught the first version passing while _aggregate_single_metric was broken — the fixture produced only LLM metrics, leaving half the split unverified. AC-3 — the index. Migration shape (non-atomic, CONCURRENTLY both directions, the INVALID guard, AddIndex confined to state_operations), plus an integration test that EXPLAINs the query the aggregation actually issues, captured rather than rewritten: a hand-copied queryset would keep passing after the prefilter changed, which is the one thing it is for. Rows are inserted in ascending created_at order so the heap matches production's append order. The Query Insights half of AC-3 is a production reading and is deliberately not faked here. Every test verified to fail when the thing it guards breaks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc
…s 0005 UN-3445's 0004_pg_periodic_tasks is the parent of both this migration and UN-3973's reconciliation migration, so landing both would leave dashboard_metrics with two leaf nodes and no applicable graph. Depend on 0005_add_reconciliation_task instead, which puts the intended merge order (UN-3973 then UN-3974) in the graph rather than in the merge queue. This branch cannot migrate on its own until UN-3973 lands; its tests are unaffected, since the suite runs with --no-migrations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc
…-causing-high-DB-load' into UN-3974-split-metrics-schedule-and-prefilter-index
|
Contributor
Author
|
@greptile-apps migration order is revised. Review again. |
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.
What
Two changes from UN-3974, shipped together because the first makes the second necessary.
monthly move to once an hour.
Option 2b on the ticket is struck: the list of active organisations cannot be read from a
table only this cron writes (comment 45017).
Why
Daily and monthly figures do not need 15-minute freshness. Refreshing them hourly takes those runs
from 96 a day to 24, across all nine dashboard metrics.
The active-organisation lookup costs 1,849 ms per call — the slowest query on the instance.
Nothing organises executions by date alone, so it reads the whole table. The index is also a
prerequisite for UN-4045, not something UN-4045 replaces (comment 45307).
How
Both schedule rows run the same task and differ only in kwargs:
tierdashboard_metrics_aggregate_from_sources*/15 * * * *hourlyEventMetricsHourlydashboard_metrics_aggregate_daily_monthly0 * * * *daily_monthlyEventMetricsDaily,EventMetricsMonthlyA kwarg rather than a second task name: a new name would need its own worker registration and
internal endpoint, since the PG consumer has no Django.
AggregationTierdefaults toall, soa caller that omits it gets the pre-split behaviour. The lock is keyed per tier — the two schedules
collide at the top of every hour and would otherwise starve each other.
Can this PR break any existing features. If yes, please list possible items. If no, please explain why.
Daily and monthly figures can be up to an hour old instead of 15 minutes. That is the intended
trade and it is in the acceptance criteria. Hourly figures are unchanged.
If both schedules ever run at once they write different tables and recompute from scratch, so the
worst case is repeated work, never wrong numbers.
Database Migrations
dashboard_metrics/0006_split_aggregation_schedule.py— adds the hourly daily/monthly schedulerow and re-keys the existing one. Both schedulers are written from one definition so they cannot
drift, and whether a row is enabled or owned is left alone.
workflow_v2/0029_we_created_at_idx.py— builds the index without locking writes, and reversescleanly.
Migration Order
The three UN-3883 PRs stack on the same integration branch. Merge in this order.
dashboard_metrics/0005_add_reconciliation_task0004_pg_periodic_tasks(UN-3445, already onmain)file_execution/0007_wfe_status_created_idxfile_execution/0006_…dashboard_metrics/0006_split_aggregation_schedule0005_add_reconciliation_task(#2255)#2264's migration is in a different app and has no interaction with the other two. The one
hard dependency is #2265 on #2255 — merged out of order, #2265 fails at graph build:
Its tests are unaffected — the backend suite runs with
--no-migrations.Verified on a throwaway Postgres:
0004→0005→0006applies from an empty database,reverses, and re-applies, with
makemigrations --checkclean at each step.Env Config
None. Both cadences are schedule rows.
Relevant Docs
UN-3883 analysis §6.4 and §7 steps 4–5.
Related Issues or PRs
Parent UN-3883. UN-4045 — this index is its prerequisite. #2264 (UN-3972), #2255 (UN-3973) —
see Migration Order above.
Dependencies Versions
None.
Notes on Testing
46 tests plus 14 scenarios, green under tox. Detail in Jira comment 45426.
After deploy: confirm the index is valid, then re-pull the 6-hour window against the
1,849 ms/call baseline.
Screenshots
n/a — schedule and schema change.
Checklist
I have read and understood the Contribution Guidelines.