Skip to content

testdrive: skip the catalog dump when the catalog store is not validated - #38704

Merged
ggevay merged 1 commit into
MaterializeInc:mainfrom
ggevay:gabor/testdrive-skip-catalog-dump
Sep 9, 2026
Merged

testdrive: skip the catalog dump when the catalog store is not validated#38704
ggevay merged 1 commit into
MaterializeInc:mainfrom
ggevay:gabor/testdrive-skip-catalog-dump

Conversation

@ggevay

@ggevay ggevay commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Motivation

check_catalog_state runs after every testdrive file at the default consistency-check level. It fetched /api/catalog/dump from environmentd's internal HTTP listener and parsed the whole document, and only then called with_catalog_copy, which returns None and skips the comparison whenever no catalog config was supplied, which is every composition that does not pass --validate-catalog-store (about 170 of 175 call sites). The dump is 109 MB at v26.41.0-rc.4, so that was about 700 ms of waste per file, plus a transient allocation of a few hundred MB in environmentd, and it is what the parallel benchmark's testdrive row had been reporting as a regression (QAR-159, SQL-689). Closes: QAR-160

Description

Return early from check_catalog_state when the catalog config is absent, before the HTTP fetch. The fetched fields were only used to feed with_catalog_copy and the on-disk comparison, so nothing else depends on them. Compositions that pass --validate-catalog-store keep the full check.

Verification

cargo check -p mz-testdrive and cargo clippy -p mz-testdrive --all-targets -- -D warnings are clean. The testdrive nightly with --validate-catalog-store still exercises the comparison path.

🤖 Generated with Claude Code

check_catalog_state fetched and parsed the whole /api/catalog/dump after
every file and only then discovered, inside with_catalog_copy, that no
catalog config was supplied and the comparison had to be skipped. Only a
handful of compositions pass --validate-catalog-store; everywhere else
the fetch was wasted, about 700 ms per file at a 109 MB dump, plus a
transient allocation of a few hundred MB in environmentd. Return early
before the fetch when the config is absent.

Closes: QAR-160

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ggevay
ggevay marked this pull request as ready for review September 9, 2026 12:06
@ggevay
ggevay requested a review from a team as a code owner September 9, 2026 12:06
@ggevay
ggevay merged commit 08ae3e9 into MaterializeInc:main Sep 9, 2026
75 checks passed
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.

2 participants