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.

Sign up for GitHub

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

feat: More aggressively prune the Crashpad database #698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Removed the `SENTRY_PERFORMANCE_MONITORING` compile flag requirement to access performance monitoring in the Sentry SDK. Performance monitoring is now available to everybody who has opted into the experimental API.
- New API to check whether the application has crashed in the previous run: `sentry_get_crashed_last_run()` and `sentry_clear_crashed_last_run()` ([#685](https://github.com/getsentry/sentry-native/pull/685)).
- Allow overriding the SDK name at build time - set the `SENTRY_SDK_NAME` CMake cache variable.
- More aggressively prune the Crashpad database. ([#698](https://github.com/getsentry/sentry-native/pull/698))

## 0.4.15

Expand Down
18 changes: 18 additions & 0 deletions src/backends/sentry_backend_crashpad.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ extern "C" {
#include "client/crash_report_database.h"
#include "client/crashpad_client.h"
#include "client/crashpad_info.h"
#include "client/prune_crash_reports.h"
#include "client/settings.h"

#if defined(__GNUC__)
Expand Down Expand Up @@ -426,6 +427,22 @@ sentry__crashpad_backend_last_crash(sentry_backend_t *backend)
return crash_time;
}

static void
sentry__crashpad_backend_prune_database(sentry_backend_t *backend)
{
crashpad_state_t *data = (crashpad_state_t *)backend->data;

// We want to eagerly clean up reports older than 2 days, and limit the
// 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 :)

Copy link

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?

crashpad::BinaryPruneCondition condition(crashpad::BinaryPruneCondition::OR,
new crashpad::DatabaseSizePruneCondition(1024 * 8),
new crashpad::AgePruneCondition(2));
crashpad::PruneCrashReportDatabase(data->db, &condition);
}

sentry_backend_t *
sentry__backend_new(void)
{
Expand All @@ -451,6 +468,7 @@ sentry__backend_new(void)
backend->user_consent_changed_func
= sentry__crashpad_backend_user_consent_changed;
backend->get_last_crash_func = sentry__crashpad_backend_last_crash;
backend->prune_database_func = sentry__crashpad_backend_prune_database;
Swatinem marked this conversation as resolved.
Show resolved Hide resolved
backend->data = data;
backend->can_capture_after_shutdown = true;

Expand Down
1 change: 1 addition & 0 deletions src/sentry_backend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ struct sentry_backend_s {
const sentry_options_t *options);
void (*user_consent_changed_func)(sentry_backend_t *);
uint64_t (*get_last_crash_func)(sentry_backend_t *);
void (*prune_database_func)(sentry_backend_t *);
void *data;
bool can_capture_after_shutdown;
};
Expand Down
4 changes: 4 additions & 0 deletions src/sentry_core.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,11 @@ sentry_init(sentry_options_t *options)

// after initializing the transport, we will submit all the unsent envelopes
// and handle remaining sessions.
SENTRY_TRACE("processing and pruning old runs");
sentry__process_old_runs(options, last_crash);
if (backend && backend->prune_database_func) {
backend->prune_database_func(backend);
}

if (options->auto_session_tracking) {
sentry_start_session();
Expand Down