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 panic when dealing with missing mapping ID #3188

Conversation

simonswine
Copy link
Contributor

I have not gotten to the bottom of why the mapping ID is missing when
maxNodes!=0.

This is presenting a workaround until we full understand the issue.

Fixes #3164

I have not gotten to the bottom of why the mapping ID is missing when
maxNodes!=0.

This is presenting a workaround until we full understand the issue.

Fixes grafana#3164
@simonswine
Copy link
Contributor Author

@kolesnikovae I was able to reproduce the panic consistently with and this PR fixes it.

Have a look at this code, the block is in a shared bucket that you should be able to access after being logged in: simonswine@3cf4015

All boils down to a single series

@simonswine simonswine marked this pull request as ready for review April 10, 2024 16:12
@simonswine simonswine requested a review from a team as a code owner April 10, 2024 16:12
@aleks-p
Copy link
Contributor

aleks-p commented Apr 10, 2024

How easy would it be to add a test that covers this specific case?

Copy link
Collaborator

@kolesnikovae kolesnikovae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!

I realize that the root cause of the problem lies a bit deeper – the profile that causes the issue is malformed: it has a single sample with the sentinel stack trace ID (0). Temporary fix: #3193. I'll investigate this further: #3195. Apart from the read path fixes, I think we should make sure such profiles do get into the block: #3194

This specific issue is caused by the fact that we do not expect that a profile has samples and does not have a mapping, therefore we reference the default one. Now that we rely on the mappings (containing the version and the git ref), I think we should always create a stub for truncated nodes – to be fixed separately: #3196.

@kolesnikovae kolesnikovae merged commit aca54af into grafana:main Apr 11, 2024
16 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.

Panic: Index out of range error when merging profiles
3 participants