*: Phases 3+4 — drop FrostDB-backed tests, rip frostdb out of go.mod#6356
Open
thorfour wants to merge 1 commit into
Open
*: Phases 3+4 — drop FrostDB-backed tests, rip frostdb out of go.mod#6356thorfour wants to merge 1 commit into
thorfour wants to merge 1 commit into
Conversation
Phases 3 and 4 of the FrostDB removal, combined because the schema
proto removal that lets us drop frostdb from go.mod is part of the
test cleanup.
Per the agreed strategy (option C from the Phase 3 triage): the
FrostDB-backed tests aren't testing the new ClickHouse path — they're
exercising a backend that's about to be deleted. Carry-forward value
is debatable, and a fresh ClickHouse-targeted integration test (using
testcontainers, alongside the existing pkg/clickhouse/filter_test.go
pattern) is more honest than a port. Filed as a follow-up.
Deleted:
* pkg/ingester/ — the FrostDB-only TableIngester. The tiny Ingester
interface that ProfileColumnStore consumes moved to pkg/profilestore.
* pkg/parcacol/querier.go — the FrostDB-only querier. ClickHouse has
its own querier (pkg/clickhouse/querier.go).
* pkg/parca/parca_test.go — TestPGOE2e, TestLabels, TestConsistency
(already t.Skip'd), Benchmark_WriteRaw (already b.Skip'd). All
spun up a real frostdb columnstore.
* pkg/profilestore/profilestore_test.go — Test_LabelName_Error and
BenchmarkProfileColumnStoreWriteSeries, both frostdb-backed.
* pkg/query/query_test.go — Benchmark_Query_Merge,
Benchmark_ProfileTypes, both frostdb-backed.
* The 9 frostdb-backed tests in pkg/query/columnquery_test.go
(TestColumnQueryAPI{QueryRangeEmpty,QueryRange,QuerySingle,
QueryFgprof,QueryCumulative,QueryDiff,Types,LabelNames,LabelValues}).
The remaining filter / merge / utility tests are kept along with
helper functions (MustReadAllGzip etc.) that other test files in
the package depend on.
* profile.Schema() — the function returning *dynparquet.Schema.
Reshaped:
* pkg/profile/schema.go: SchemaDefinition() now returns a small local
SchemaDef struct instead of *schemapb.Schema. The proto-defined
encoding/compression/sorting hints are dropped — they were only
consumed by the parquet writer. The few fields actually read at
runtime (Name, Type, Repeated, Nullable, Dynamic) survive on a
hand-rolled struct. This severs the last frostdb proto import.
* pkg/profilestore: gains a thin Ingester interface (the same shape
the old pkg/ingester defined) so ProfileColumnStore can keep its
dependency-injected ingester.
* pkg/query/sources_test.go: parcacol.NewQuerier is gone, so the
source-only test now uses a tiny in-file nopQuerier — its methods
are never actually called for SourceOnly=true requests.
go.mod / go.sum:
* github.com/polarsignals/frostdb — gone.
* github.com/polarsignals/iceberg-go — gone (was only used by the
experimental Iceberg path inside the FrostDB branch).
* github.com/parquet-go/parquet-go — gone (was a transitive of frostdb;
the AppendParquetValues paths in the vendored builders were
stripped in Phase 2.B).
Net diff: -22759 lines.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
✅ Meticulous spotted 0 visual differences across 288 screens tested: view results. Meticulous evaluated ~4 hours of user flows against your PR. Expected differences? Click here. Last updated for commit |
6 tasks
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
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.
Summary
Final phases of the FrostDB removal. Stacks on #6355 (Phase 2.B). Net diff: -22,759 lines.
Combined into one PR because the schema-proto removal that unblocks dropping
frostdbfromgo.modis part of the test/cleanup work.Strategy (option C from the Phase 3 triage)
The FrostDB-backed tests aren't testing the new ClickHouse path — they're exercising a backend that's about to be deleted. Their carry-forward value is debatable, and a fresh ClickHouse-targeted integration test (using testcontainers, alongside the existing
pkg/clickhouse/filter_test.go) is more honest than a port. Filed as a follow-up.Deleted
pkg/ingester/— the FrostDB-onlyTableIngester. The tinyIngesterinterface thatProfileColumnStoreconsumes moved topkg/profilestore.pkg/parcacol/querier.go— the FrostDB-only querier. ClickHouse has its own (pkg/clickhouse/querier.go).pkg/parca/parca_test.go—TestPGOE2e,TestLabels,TestConsistency(alreadyt.Skip'd),Benchmark_WriteRaw(alreadyb.Skip'd). All spun up a real frostdb columnstore.pkg/profilestore/profilestore_test.go— both tests were frostdb-backed.pkg/query/query_test.go— both benchmarks were frostdb-backed.pkg/query/columnquery_test.go(TestColumnQueryAPI*). The remaining filter / merge / utility tests are kept along with helper functions (MustReadAllGzipetc.) that other test files in the package depend on.profile.Schema()— the function returning*dynparquet.Schema.Reshaped
pkg/profile/schema.go:SchemaDefinition()now returns a small localSchemaDefstruct instead of*schemapb.Schema. The proto-defined encoding/compression/sorting hints are dropped — they were only consumed by the parquet writer. The few fields actually read at runtime (Name,Type,Repeated,Nullable,Dynamic) survive on a hand-rolled struct. This severs the last frostdb proto import.pkg/profilestore: gains a thinIngesterinterface (same shapepkg/ingesterdefined).pkg/query/sources_test.go:parcacol.NewQuerieris gone, so the source-only test now uses a tiny in-filenopQuerier— its methods are never actually called forSourceOnly=truerequests.go.mod/go.sumgithub.com/polarsignals/frostdb— gone.github.com/polarsignals/iceberg-go— gone (was only used by the experimental Iceberg path inside the FrostDB branch).github.com/parquet-go/parquet-go— gone (was a transitive of frostdb; theAppendParquetValuespaths in the vendored builders were stripped in *: Phase 2.B — vendor pqarrow/builder + arrowutils into pkg/query/internal #6355).Test plan
go build ./...go vet ./...go test -short ./...— all pass🤖 Generated with Claude Code