querier: release LSS query result buffers deterministically - #447
Merged
Conversation
The C-allocated series-id result buffer returned by Querier::query was kept alive by the series sets and only freed lazily by a Go GC finalizer. Because the real bytes live off the Go heap (jemalloc), the GC sees almost no pressure and finalization lags, so thousands of over-allocated buffers (capacity = max matcher cardinality) pile up — the dominant on-heap consumer in heap profiles. Make every series set independent of LSSQueryResult so it can be closed right after construction: - Add idempotent LSSQueryResult.Close() (frees the C buffers, cancels the finalizer which stays a harmless fallback). - InstantSeries carries the series id inline next to its sample; mirror the field in the C++ SampleWithGoLabels struct so the shared ABI still matches. - StaleNaNSeries carries the series id inline. - ChunkSeriesSet takes the series id straight from the recoded chunks and no longer references the result at all. - Close the query results in selectInstant/selectRange and ChunkQuerier.Select once all in-flight C queries have completed. Co-authored-by: Cursor <cursoragent@cursor.com>
Instead of copying the series ids on the Go side after the query, the instant querier now writes the label-set id into SampleWithGoLabels.series_id in the same loop that fills the sample. store_series_id is a no-op for plain encoder::Sample (used in C++ tests) via an if-constexpr guard, so the generic querier is unaffected. NewInstantSeriesSet no longer needs the query result. Co-authored-by: Cursor <cursoragent@cursor.com>
Mirror the instant-query optimization for the stalenan path: the C++ data storage query now writes both the first-sample timestamp and the series id directly into a C-shared StaleNaNSeries slice, so the LSS query result buffer is no longer referenced after construction and can be released immediately. Co-authored-by: Cursor <cursoragent@cursor.com>
vporoshok
force-pushed
the
fix/lss-query-result-explicit-close
branch
from
July 24, 2026 05:38
dcfcff2 to
f56334c
Compare
cherep58
approved these changes
Jul 24, 2026
cherep58
approved these changes
Jul 24, 2026
vporoshok
marked this pull request as ready for review
July 29, 2026 12:58
vporoshok
enabled auto-merge (squash)
July 29, 2026 13:46
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
2 participants
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
The C-allocated series-id result buffer returned by
Querier::query(Go::Slice<uint32>) was kept alive by the series sets and freed only lazily by a Go GC finalizer. Because the real bytes live off the Go heap (jemalloc), the GC sees almost no pressure and finalization lags, so thousands of over-allocated buffers (capacity = max matcher cardinality, not the deduplicated result size) accumulate. In heap profiles this path (prompp_primitives_snapshot_query→Querier::query→realloc) was the dominant consumer (~10+ GB, ~14k live buffers, avg ~690 KB each).This makes every series set independent of
LSSQueryResultso the result can be closed deterministically right after the series set is built.Changes
LSSQueryResult.Close()— idempotent explicit free of the C buffers; cancels the finalizer, which remains a harmless fallback.InstantSeriescarries the series id inline next to its sample. Because this struct is shared with C++ by pointer (entrypoint::types::SampleWithGoLabels), the field is mirrored asseries_id_in the C++ struct so the ABI/stride still matches (both become 40 bytes; C++ only ever writes theSamplebase).StaleNaNSeriescarries the series id inline (Go-only struct).ChunkSeriesSettakes the series id straight from the recoded chunks and no longer references the result at all.selectInstant/selectRange/ChunkQuerier.Selectclose the query results (viacloseLSSQueryResults) once all in-flight C queries have completed.Net effect: the query result buffers are released immediately instead of waiting for a lagging GC finalizer — no GC hinting needed.
Test plan
cd pp && make build-entrypoint(ARM devcontainer)go test -tags stringlabels ./pp/go/storage/... ./pp/go/cppbridge/...— pass (instant tests would fail on any stride mismatch)go test -tags "stringlabels,testgcdouble" ./pp/go/storage/querier/... ./pp/go/cppbridge/...— passMade with Cursor