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

feat: More aggressively prune the Crashpad database #698

Merged
merged 1 commit into from
Apr 13, 2022
Merged

Conversation

Swatinem
Copy link
Member

It seems that the defaults in the crashpad_handler only trigger after a long 10min delay, and let the db grow to up to 128M by default. We generally do not care about older/completed reports at all, so make sure to aggressively prune them.

@Swatinem Swatinem requested a review from a team April 13, 2022 09:15
Copy link
Contributor

@flub flub left a comment

Choose a reason for hiding this comment

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

do these conditions result in pruning the db even if the minidumps are unsent?

src/backends/sentry_backend_crashpad.cpp Show resolved Hide resolved
// complete database to a maximum of 8M. That might still be a lot for
// an embedded use-case, but minidumps on desktop can sometimes be quite
// large.
data->db->CleanDatabase(60 * 60 * 24 * 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

2 days still seems a long time to me, why not immediate? Or are we at risk of removing unsent minidumps?

Copy link
Member Author

Choose a reason for hiding this comment

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

crashpad has 3 states essentially: new, pending and completed. The function cleans pending and completed minidumps. I would have to look into what it does exactly on upload (create as new, move into pending when it starts uploading?)
Keeping the "most recent" minidump might be fair to do. At least the database should not grow seemingly unbounded.

Copy link
Contributor

Choose a reason for hiding this comment

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

in theory crashpad tries to upload as soon as the crash happens right? so this will only find new or pending minidumps if that failed somehow. I guess 2 days is reasonable, it's just really though to come up with something that suits all :)

Choose a reason for hiding this comment

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

I'd strongly prefer to wipe the minidumps as soon as they get uploaded.
What about making the parameters configurable though API calls?

@Swatinem Swatinem merged commit 9e12f81 into master Apr 13, 2022
@Swatinem Swatinem deleted the feat/db-prune branch April 13, 2022 10:46
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.

3 participants