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

CORE-8695 crash_tracker: implement crash report upload markers #25130

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

pgellert
Copy link
Contributor

We will use empty files to track if a certain crash file has been uploaded through our metrics reporter telemetry mechanism.

To mark a crash file as uploaded, we touch a new file with the same name + the ".uploaded" suffix. To check if a crash file has been uploaded, we check if a file with the same name + the ".uploaded" suffix exists. We also implement the clean up of any upload markers on disk that do not have an associated crash report.

Fixes https://redpandadata.atlassian.net/browse/CORE-8695

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.3.x
  • v24.2.x
  • v24.1.x

Release Notes

  • none

Implement marking crash reports as uploaded and checking if a crash
report has been uploaded.

This is implemented using empty files created on disk with the same name
as the crash report + a ".uploaded" suffix.
Clean up any upload markers on disk that do not have an associated crash
report. This cleanup is done after the count-based removal of crash
reports (see remove_old_crashfiles()).
@pgellert pgellert requested a review from a team February 21, 2025 14:52
@pgellert pgellert self-assigned this Feb 21, 2025
@pgellert pgellert requested review from IoannisRP and removed request for a team February 21, 2025 14:52
Comment on lines +40 to +41
return crash_report_path.parent_path()
/ (crash_report_path.filename().string() + recorder::upload_marker_suffix);
Copy link
Member

Choose a reason for hiding this comment

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

is this different than return path + suffix?


// Create an empty upload marker
const auto marker_path = to_upload_marker_path(file_path).string();
auto f = co_await ss::open_file_dma(marker_path, ss::open_flags::create);
Copy link
Member

Choose a reason for hiding this comment

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

nit: without O_EXCL i'd expect this to succeed even if the file exists. So you could probably drop the is_uploaded() checked, unless there is something i'm missing.

}

for (const auto& entry :
std::filesystem::directory_iterator(crash_report_dir)) {
Copy link
Member

Choose a reason for hiding this comment

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

use utils/directory_walker.h?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants