Skip to content

feat(perf): compare uncached and cached DuckDB - #1160

Open
bill-ph wants to merge 2 commits into
mainfrom
codex/perf-disable-worker-cache
Open

feat(perf): compare uncached and cached DuckDB#1160
bill-ph wants to merge 2 commits into
mainfrom
codex/perf-disable-worker-cache

Conversation

@bill-ph

@bill-ph bill-ph commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Compare pgwire_uncached (baseline), pgwire_cached, Trino, and Athena in the same frozen perf run. Both DuckDB variants use identical SQL, resources, and iteration counts. Trino and Athena run once each.
  • Configure cache settings only inside the explicit perf drivers. Uncached disables external-file data, Parquet metadata, and shared HTTP metadata caches; cached restores DuckDB's normal external-file cache while leaving metadata caches off.
  • Pin each perf connection and apply settings lazily before its first warmup, outside query timing. Each target completes before the next starts. Configuration failures and lost connections fail closed rather than silently recording a different mode.
  • Preserve legacy pgwire behavior. Version query/intent IDs as balanced_v4 to separate the new methodology from earlier history; no artifact schema changes or data migration.
  • Document scope and safe local usage. Query-local buffers, prefetching, DuckLake catalog caching, optional cache extensions, and separate cache proxies are unaffected.

This replaces the initial broad worker-startup change. There are no production worker startup or configuration changes in the final diff. Explicit cache modes are used only by the perf harness.

Validation

  • TDD red/green for cache-mode setup, phase order, routing, factory selection, and distinct artifact labels.
  • just test-unit and just test-perf passed.
  • Driver race tests passed for 10 repetitions.
  • Real local Duckgres PGWire smoke test: both drivers constructed before execution; uncached warmup/repeated queries verified all three settings false; cached repeated queries verified external-file cache true and both metadata caches false.
  • git diff --check passed.
  • just lint reports six pre-existing staticcheck findings in unchanged control-plane/main files. golangci-lint run --new-from-rev=origin/main reports zero issues.
  • Read-only code review found no actionable issues.

Rollout

Run the frozen scenario from this PR branch to validate the four result sets before merging. No remote deployment or perf run has been triggered by this change. Local cache-mode runs must use a dedicated test warehouse because DuckDB cache settings are global within the worker; do not use a shared production worker.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Test Impact Plan

Deterministic summary of how this PR changes tests, CI runners, and coverage-risk signals.

Summary

Area Added Changed Deleted
Test files 0 15 0
E2E/journey files 0 0 0
Workflow files 0 0 0

Signals

  • Test cases: +7 / -0
  • Assertions: +34 / -1
  • Skips or known failures added: 0
  • Workflow continue-on-error added: 0
  • Workflow path filters added: 0
  • Test commands removed from justfile: 0
  • E2E/journey retry lines added: 0

Coverage risk: neutral or increased

No coverage-reduction warnings detected.

@bill-ph bill-ph changed the title fix: disable remote-file caches in DuckDB workers feat(perf): compare uncached and cached DuckDB Sep 7, 2026
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.

1 participant