Repository navigation
perf(insights): build the hogql database once per insight run - #88183
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JhmceYYUyY1427j1FeMFv
|
😎 Merged successfully - details. |
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
render-query/src/render-query/render-query.js |
21.67 MiB | 🟢 -258.2 KiB (-1.2%) |
posthog-app/src/lib/components/Shortcuts/utils/DebugCHQueriesImpl.js |
14.9 KiB | 🟢 -18.5 KiB (-55.3%) |
exporter/src/queries/schema.js |
1.16 MiB | 🔺 +11.2 KiB (+1.0%) |
posthog-app/src/queries/schema.js |
1.16 MiB | 🔺 +11.2 KiB (+1.0%) |
posthog-app/_parent/products/visual_review/frontend/scenes/VisualReviewRunScene.js |
52.6 KiB | 🔺 +3.7 KiB (+7.6%) |
posthog-app/_parent/products/logs/frontend/LogsScene.js |
42.4 KiB | 🟢 -3.3 KiB (-7.2%) |
posthog-app/_parent/products/engineering_analytics/frontend/scenes/EngineeringAnalyticsScene.js |
61.8 KiB | 🔺 +3.0 KiB (+5.1%) |
posthog-app/_parent/products/metrics/frontend/MetricsScene.js |
61.8 KiB | 🔺 +2.8 KiB (+4.7%) |
exporter/src/exporter/scenes/ExporterNotebookScene.js |
3.37 MiB | 🔺 +1.9 KiB (+0.1%) |
posthog-app/src/scenes/onboarding/Onboarding.js |
797.0 KiB | 🟢 -1.5 KiB (-0.2%) |
posthog-app/_parent/products/signals/frontend/inbox/InboxScene.js |
704.0 KiB | 🔺 +1.5 KiB (+0.2%) |
posthog-app/_parent/products/replay_vision/frontend/replay_scanners/ReplayScannersScene.js |
41.2 KiB | 🔺 +1.4 KiB (+3.5%) |
posthog-app/_parent/products/stamphog/frontend/scenes/StamphogDigestsScene/StamphogDigestsScene.js |
7.2 KiB | 🟢 -1.2 KiB (-14.3%) |
posthog-app/_parent/products/replay_vision/frontend/replay_scanners/ReplayScanner.js |
185.0 KiB | 🔺 +1.1 KiB (+0.6%) |
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.36 MiB · 22 files | 🔺 +1.5 KiB (+0.1%) | ███░░░░░░░ 30.2% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.77 MiB · 3,235 files | 🔺 +2.7 KiB (+0.0%) | █████████░ 90.2% of 9.71 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 306.5 KiB | ../node_modules/.pnpm/posthog-js@1.418.17/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 258.5 KiB | ../node_modules/.pnpm/posthog-js@1.418.17/node_modules/posthog-js/dist/module.js |
| 246.0 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 154.2 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.6 KiB | src/lib/api.ts |
| 95.1 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.25 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.25 MiB · 17 files | 🔺 +1.5 KiB (+0.1%) | ████░░░░░░ 39.3% of 5.72 MiB |
| Deferred (lazy) | 2.09 MiB · 33 files | 🟢 -964 B (-0.0%) | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 745.9 KiB | dist/toolbar/toolbar-app-QDEUOQFO.css |
| 582.9 KiB | dist/toolbar/chunk-chunk-YRYIO5AK.js |
| 484.6 KiB | dist/toolbar/chunk-chunk-7TYJBGPX.js |
| 133.8 KiB | dist/toolbar/chunk-chunk-HUJ6YVZZ.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.2 KiB | dist/toolbar/toolbar-app-2USHGT3U.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-5I6DF2DA.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-WRHMR2RW.js |
| 12.2 KiB | dist/toolbar/chunk-chunk-PIK3PADE.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🟢 -4.11 MiB (-0.3%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1460.50 MiB · 🟢 -4.11 MiB (-0.3%)
⚠️ Backend coverage — 97.0% of changed backend lines covered — 3 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ███████████████████░ 97.0% (113 / 116)
| File | Patch | Uncovered changed lines |
|---|---|---|
posthog/hogql_queries/test/test_query_runner.py |
95.7% | 242–243 |
posthog/hogql_queries/query_runner.py |
96.3% | 1729 |
🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 32906840198 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
platform_features |
██░░░░░░░░░░░░░░░░░░ 12.1% |
7 / 58 |
warehouse_sources_queue |
██████░░░░░░░░░░░░░░ 29.4% |
91 / 309 |
demo |
███████████░░░░░░░░░ 56.9% |
1,516 / 2,666 |
data_tools |
█████████████░░░░░░░ 66.2% |
90 / 136 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
batch_exports |
████████████████░░░░ 79.8% |
19,258 / 24,143 |
apm |
█████████████████░░░ 83.3% |
1,234 / 1,481 |
signals |
█████████████████░░░ 83.6% |
29,806 / 35,674 |
cdp |
█████████████████░░░ 85.1% |
4,146 / 4,874 |
actions |
█████████████████░░░ 86.6% |
716 / 827 |
wizard |
█████████████████░░░ 86.7% |
1,090 / 1,257 |
data_modeling |
█████████████████░░░ 86.9% |
9,644 / 11,092 |
notebooks |
█████████████████░░░ 87.3% |
8,736 / 10,003 |
cohorts |
██████████████████░░ 87.9% |
6,768 / 7,704 |
product_tours |
██████████████████░░ 87.9% |
1,303 / 1,482 |
data_warehouse |
██████████████████░░ 89.1% |
12,630 / 14,181 |
business_knowledge |
██████████████████░░ 89.1% |
4,440 / 4,984 |
dashboards |
██████████████████░░ 89.2% |
6,392 / 7,165 |
exports |
██████████████████░░ 89.3% |
7,887 / 8,830 |
engineering_analytics |
██████████████████░░ 89.7% |
7,009 / 7,815 |
visual_review |
██████████████████░░ 89.8% |
6,057 / 6,745 |
tasks |
██████████████████░░ 89.9% |
59,126 / 65,756 |
alerts |
██████████████████░░ 90.4% |
4,707 / 5,209 |
mcp_analytics |
██████████████████░░ 90.5% |
4,000 / 4,418 |
links |
██████████████████░░ 90.6% |
183 / 202 |
streamlit_apps |
██████████████████░░ 90.7% |
2,630 / 2,901 |
managed_warehouse |
██████████████████░░ 90.7% |
8,555 / 9,427 |
conversations |
██████████████████░░ 90.9% |
20,495 / 22,558 |
error_tracking |
██████████████████░░ 91.5% |
12,246 / 13,380 |
canvas |
██████████████████░░ 92.5% |
4,512 / 4,880 |
early_access_features |
███████████████████░ 92.5% |
1,323 / 1,430 |
managed_migrations |
███████████████████░ 92.6% |
1,556 / 1,681 |
slack_app |
███████████████████░ 92.6% |
12,123 / 13,096 |
stamphog |
███████████████████░ 92.6% |
5,510 / 5,948 |
context_layer |
███████████████████░ 92.7% |
2,285 / 2,464 |
notifications |
███████████████████░ 92.8% |
1,074 / 1,157 |
web_analytics |
███████████████████░ 93.0% |
17,575 / 18,908 |
mcp_store |
███████████████████░ 93.2% |
7,438 / 7,984 |
surveys |
███████████████████░ 93.3% |
5,942 / 6,372 |
marketing_analytics |
███████████████████░ 93.4% |
16,881 / 18,081 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
posthog_ai |
███████████████████░ 93.5% |
1,371 / 1,467 |
ai_observability |
███████████████████░ 93.6% |
17,489 / 18,684 |
product_analytics |
███████████████████░ 93.7% |
7,367 / 7,863 |
approvals |
███████████████████░ 93.7% |
3,485 / 3,718 |
billing_alerts |
███████████████████░ 93.9% |
2,047 / 2,181 |
legal_documents |
███████████████████░ 94.0% |
1,914 / 2,037 |
workflows |
███████████████████░ 94.3% |
9,781 / 10,374 |
endpoints |
███████████████████░ 94.3% |
8,813 / 9,346 |
review_hog |
███████████████████░ 94.3% |
9,975 / 10,576 |
tracing |
███████████████████░ 94.4% |
2,678 / 2,837 |
annotations |
███████████████████░ 95.1% |
734 / 772 |
customer_analytics |
███████████████████░ 95.1% |
16,694 / 17,548 |
skills |
███████████████████░ 95.3% |
4,794 / 5,031 |
experiments |
███████████████████░ 95.5% |
29,198 / 30,563 |
messaging |
███████████████████░ 95.8% |
3,586 / 3,745 |
logs |
███████████████████░ 95.8% |
13,255 / 13,836 |
replay_vision |
███████████████████░ 95.8% |
22,600 / 23,585 |
feature_flags |
███████████████████░ 96.1% |
19,158 / 19,941 |
access_control |
███████████████████░ 96.2% |
5,078 / 5,280 |
revenue_analytics |
███████████████████░ 96.3% |
1,841 / 1,911 |
user_interviews |
███████████████████░ 96.5% |
2,639 / 2,735 |
data_quality |
███████████████████░ 96.5% |
2,692 / 2,789 |
growth |
███████████████████░ 96.6% |
7,656 / 7,926 |
warehouse_sources |
███████████████████░ 97.2% |
400,106 / 411,737 |
analytics_platform |
████████████████████ 98.1% |
2,286 / 2,330 |
data_catalog |
████████████████████ 98.2% |
3,203 / 3,262 |
metrics |
████████████████████ 98.3% |
3,256 / 3,312 |
pulse |
████████████████████ 98.4% |
2,017 / 2,049 |
live_debugger |
████████████████████ 99.2% |
613 / 618 |
field_notes |
████████████████████ 99.4% |
158 / 159 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
Every covered runner printed its response hogql against a freshly built second database, doubling compile cost per fresh insight. QueryRunner now owns one lazily built database per run; execution and printing share it via build_hogql_context() and response_hogql(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R8Uyj628NgticUdB6DJJ7x
Prompt To Fix All With AI### Issue 1
posthog/hogql/database/database.py:733
**Process-wide foreign-key build lock**
If independent warehouse-heavy insight requests build deferred foreign keys concurrently, this process-wide lock serializes their complete catalog build passes, increasing tail latency and reducing insight throughput even though the databases share no mutable build state.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "perf(insights): build the hogql database..." | Re-trigger Greptile |
A process-wide lock serialized unrelated requests' deferred FK builds in one process. Nothing copies or pickles a Database, so the lock is safe as instance state created in __init__. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R8Uyj628NgticUdB6DJJ7x
…t-touch Funnels compare mode executes both periods in threads whose first access to shared_database is concurrent. Without a lock both threads build a database, and both record into the runner's shared HogQLTimings, which is not thread-safe: the interleaved measure() calls corrupt the timing tree or raise KeyError and fail the query. Double-checked lock so one thread builds while the other waits, and only the builder records the timing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R8Uyj628NgticUdB6DJJ7x
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R8Uyj628NgticUdB6DJJ7x
The instance settings page only renders settings on its frontend allowlist, so the API-editable setting was invisible without this. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R8Uyj628NgticUdB6DJJ7x
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
|
/trunk merge |
Problem
A fresh insight builds the same HogQL schema at least twice before it returns anything. One build serves the executed query. A second, identical build happens inside the response SQL printer, which only renders the display-only
hogqlstring in the response payload. On teams with a handful of tables nobody notices. On our internal project, with ~6,800 warehouse objects, each build costs seconds: a fresh single-series trends took ~8 s, and the printer's duplicate build was roughly half of it.Trends multiplies the waste further, because every series thread also built its own private database. A fresh 3-series trends performed four GIL-serialized builds and took over 20 seconds, with ClickHouse itself needing ~0.4 s.
The printer's build has run with warehouse access control bypassed since #67539, which fixed userless refreshes failing closed but cemented the duplicate build on every path.
Changes
QueryRunnernow owns one lazily built database per run, and the insight runners use it for both execution and printing. A fresh 3-series trends goes from four schema builds to one; every other covered insight goes from two to one. Nothing a user sees changes.Staff get a runtime kill switch: instance setting
HOGQL_SHARED_INSIGHT_DATABASE_ENABLED, default on, editable on the instance settings page. Off makesshared_databasereturn a fresh database per access, so query threads never share schema state and every runner falls back to per-call builds. Workers pick up a change within 60 seconds. The off path records no timings because concurrent threads reach it andHogQLTimingsis not thread-safe.The base class gains three small members:
shared_database(built on first use, so cache hits never pay for it, and dropped by_on_user_changedso access control follows the user),build_hogql_context()for execute calls, andresponse_hogql(query)for the display-only response SQL. The runners wired onto them are trends, funnels, funnel correlation, retention, lifecycle, paths, paths v2, boxplot, and the four AI taxonomy and vector-search runners. Each call site is a one-line helper call; that part of the diff is mechanical.The risky part is trends, where the shared database crosses its per-series threads and makes the deferred foreign-key build concurrent. That build used to run behind a plain re-entrancy boolean, which was enough when every thread owned a private database. It now runs under a per-database lock (created in
Database.__init__, so unrelated concurrent requests never contend), and the re-entrancy guard becomes the builder's thread ident: the building thread may re-enterget_tablewhile wiring join targets without deadlocking on the non-reentrant lock, while any other thread blocks until the finished join graph is visible instead of reading a half-wired one. A mid-pass failure still leaves the work pending for the next access rather than leaving a corrupt graph behind.The second concurrent path is funnels compare mode, which runs both periods in threads whose first access to
shared_databaseraces. The property now builds behind a per-runner lock: one thread builds once, the other waits and reuses the same instance. Only the builder records into the runner'sHogQLTimings, which is not thread-safe.Two printer paths are deliberately not moved:
QueryRunnerWithHogQLContextrunners already build and reuse one database from construction, and the endpoints materialization printer needs its own patched schema.Note
The response SQL printer previously built its own schema with warehouse access control bypassed (#67539) so userless refreshes would not fail closed. When a runner passes its database in, the bypass becomes moot: the printer reuses the execution schema, built exactly as the execution path builds it. Callers that pass no database keep the bypass path.
How did you test this code?
test_multi_series_builds_database_once(trends) andtest_builds_database_once_per_run(funnels) countDatabase.create_forcalls across a full run and fail if any series, executor, or the printer builds a second schema.test_shared_database_is_reused_and_rebuilt_on_user_changepins the base behavior: the same object across accesses, and a rebuild after the user changes. The other runners use the same two base helpers, and their existing suites pin the printedhogqlstrings and results.test_shared_database_kill_switch_disables_sharingflips the setting off and checks two property accesses return different instances, then back on, the same instance. It catches a refactor that silently drops the switch and with it the emergency lever.test_shared_database_first_touch_is_thread_safefirst-touches the property from two threads around a slowed build. Without the lock it fails on the double build; the timing-tree assertion catches theHogQLTimingscorruption. These tests pass locally against the dev stack. A local A/B benchmark against a ~1,200-object warehouse catalog measures the compile-only path (response printing plus per-series SQL generation) of a fresh 3-series TrendsQuery: master does 4 database builds in ~2.8 s, this branch does 1 build in ~0.6 s. The foreign-key build's cross-thread wait path has no unit test because unit tests run series threads sequentially by design.Automatic notifications
Docs update
None. No user-facing behavior or documented workflow changes; the new instance setting is staff-only.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored with Claude Code, directed by the assignee, during an investigation into slow fresh insight queries on large warehouse schemas. Skills invoked: /writing-tests, /writing-pr-descriptions, /writing-code-comments, /adopting-generated-api-types. The first version covered trends only; the assignee directed extending it to every runner that printed against a second database. A threading review before merge found the funnels compare first-touch race; the assignee directed the property lock fix. The kill switch started as #88716; the assignee directed folding it in, plus the instance settings UI allowlist entry that makes the setting visible.