Skip to content

perf: serve cache-worthy entries from result.parquet in the viewer (#71) - #72

Closed
paddymul wants to merge 2 commits into
mainfrom
perf/71-viewer-serve-result-read
Closed

paddymul wants to merge 2 commits into
mainfrom
perf/71-viewer-serve-result-read

Conversation

@paddymul

Copy link
Copy Markdown
Contributor

Problem

Opening a cache-worthy catalog entry (Aggregate/Join/Sort) in the viewer recomputes the entire expression DAG on every page interaction — once for the row count and again for each row window — instead of reading the materialised result.parquet the build already wrote.

BuckarooManager.ensure_session posts the entry's raw recipe build dir to /load_expr. That dir is the full recipe (from_catalog(...).group_by('plate').agg(...) over the multi-million-row source) with no read of result.parquet. So every expr.count().execute() and every LIMIT 300 window re-runs the whole DAG against datafusion. For an Aggregate, LIMIT 300 can't push down past the group-by, so even the first 300-row page pays the full aggregation. Per the profiling in #71, Aggregate/Join recipes cost 0.9–1.3s on each of count and window, tracking input size, not output.

Fix

In ensure_session, for a cache-worthy entry (cache_worthy(project, content_hash)), hand Buckaroo a build whose expression is a deferred_read_parquet of the materialised result rather than the raw recipe:

  • New ensure_result_build (in result_cache.py) calls ensure_result(...) to guarantee the parquet is present, then builds a tiny portable deferred_read_parquet(result) build, marker-gated and lock-serialized like ensure_expanded_build.
  • ensure_session expands that build into a dedicated .xorq_result_build_expanded dir and posts it. Cheap entries keep loading the recipe (their recompute ≈ a parquet read, so indirection buys nothing).
  • If result-build prep fails for any reason, it falls back to the recipe — a cache hiccup must not take down the detail page.

Push-down still holds: a parquet scan pushes filters/limit/sort down too, so reading result.parquet keeps the push-down benefit while dropping the re-aggregation.

Tests

  • test_unit_ensure_session_cache_worthy_serves_result_read — the posted build classifies as a cheap parquet read (classify_build worthy=False) while the recipe stays worthy=True. Seen failing on CI before the fix.
  • The two expansion-mechanism tests now use a cheap entry so they continue to exercise the recipe-expansion path.

Closes #71.

🤖 Generated with Claude Code

paddymul and others added 2 commits June 15, 2026 15:17
…the recipe

A cache-worthy entry (Aggregate/Join/Sort) is posted to Buckaroo's /load_expr
as its raw recipe, so count() and every LIMIT 300 page re-run the full DAG
instead of reading the materialised result.parquet. The new test asserts the
posted build_dir is a cheap parquet read (classify_build worthy=False); it
fails today because ensure_session hands over the expensive recipe.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…recipe (#71)

Opening a cache-worthy entry (Aggregate/Join/Sort) in the viewer re-ran the
whole expression DAG on every count() and every LIMIT 300 page, because
ensure_session posted the raw recipe build dir to /load_expr. The materialised
result.parquet the build already wrote went unused.

ensure_session now, for a cache-worthy entry, hands Buckaroo a build whose
expression is a deferred_read_parquet of the materialised result instead of the
recipe — count()/paging become a parquet scan, and push-down still holds on a
parquet scan. Cheap entries keep loading the recipe (their recompute ≈ a parquet
read). On any failure in result-build prep it falls back to the recipe so a
cache hiccup can't blank the detail page.

- ensure_result_build (result_cache.py): builds a portable deferred_read_parquet
  of the result (ensure_result guarantees the parquet first), marker-gated and
  lock-serialized like ensure_expanded_build.
- new entry_result_build_dir / entry_result_expanded_build_dir path helpers,
  both registered as per-entry caches (regenerated on demand).
- the recipe-expansion unit tests now use a cheap entry so they keep exercising
  the recipe path; cache-worthy entries take the new result-read path.

Closes #71.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@paddymul

Copy link
Copy Markdown
Contributor Author

no longer relevant after 74

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf: viewer /load_expr sends the bare recipe for cache-worthy entries — count + every page re-runs the whole DAG instead of reading result.parquet

1 participant