Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: drop malformed lines at compaction #4051

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kolesnikovae
Copy link
Collaborator

@kolesnikovae kolesnikovae commented Mar 26, 2025

It's currently possible, that we ingest and store a profile with locations that refer to non-existent functions. This is causing panics at compaction, because we assume that such locations are sanitized at ingest time. If a profile does not have symbols, its locations should not include lines and functions.

@kolesnikovae
Copy link
Collaborator Author

I'm going to figure out why we allow such locations to get to the storage before merging.

@korniltsev
Copy link
Collaborator

I may have a repro for this issue (or some other issue)

  1. start a fresh pyroscope server with no previous data
  2. Ingest the profile
cat 547df064-46df-465a-9ebc-6e7506bc9111.profile | curl -X POST --data-binary @- http://localhost:4040/ingest\?\name\=amixir\&format\=pprof 

547df064-46df-465a-9ebc-6e7506bc9111.profile.zip

  1. Qurey

image

  1. Restart the pyroscope server
  2. Query again

image

The results are different.

The pprof comes from grafana/alloy#2920

* c604b2feb 2025-03-25 | skip interpreter trampoline, add interpreter lines (HEAD -> korniltsev/otel-profiler, origin/korniltsev/otel-profiler) [Tolya Korniltsev]

I have not checked the pprof file itself, but it is likely mailformed or incorrect. I will take a look at the pprof sometime later next week.

Eventhough pprof is mailformed it should not cause different query results or compaction issues, we still need to tackle this in the database.

@kolesnikovae
Copy link
Collaborator Author

Thanks for the sample! This is very weird, indeed – I'll take a careful look

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