From 69307eaf2dbc028a13be7dfc924abab9e6c395e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20M=C4=99drek?= Date: Fri, 25 Jul 2025 16:33:59 +0200 Subject: [PATCH] db/commitlog: Extend error messages for corrupted data We're providing additional information in error messages when throwing an exception related to data corruption: when a segment is truncated and when it's content is invalid. That might prove helpful when debugging. Closes scylladb/scylladb#25190 (cherry picked from commit 408b45fa7e7c3847b0cb26cf4c3ae33e69b9a1bb) Closes scylladb/scylladb#25459 --- db/commitlog/commitlog.cc | 23 ++++++++++++++++------- db/commitlog/commitlog.hh | 6 ++---- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/db/commitlog/commitlog.cc b/db/commitlog/commitlog.cc index 02e3af861d..de46b5d037 100644 --- a/db/commitlog/commitlog.cc +++ b/db/commitlog/commitlog.cc @@ -3252,9 +3252,13 @@ const db::commitlog::config& db::commitlog::active_config() const { return _segment_manager->cfg; } +db::commitlog::segment_data_corruption_error::segment_data_corruption_error(std::string_view msg, uint64_t s) + : _msg(fmt::format("Segment data corruption: {}", msg)) + , _bytes(s) +{} -db::commitlog::segment_truncation::segment_truncation(uint64_t pos) - : _msg(fmt::format("Segment truncation at {}", pos)) +db::commitlog::segment_truncation::segment_truncation(std::string_view reason, uint64_t pos) + : _msg(fmt::format("Segment truncation at {}. Reason: {}", pos, reason)) , _pos(pos) {} @@ -3444,7 +3448,8 @@ db::commitlog::read_log_file(const replay_state& state, sstring filename, sstrin while (rem < size) { if (eof) { - throw segment_truncation(block_boundry); + auto reason = fmt::format("unexpected EOF, rem={}, size={}", rem, size); + throw segment_truncation(std::move(reason), block_boundry); } auto block_size = alignment - initial.size_bytes(); @@ -3455,7 +3460,8 @@ db::commitlog::read_log_file(const replay_state& state, sstring filename, sstrin if (tmp.size_bytes() == 0) { eof = true; - throw segment_truncation(block_boundry); + auto reason = fmt::format("read 0 bytes, while tried to read {}", block_size); + throw segment_truncation(std::move(reason), block_boundry); } crc32_nbo crc; @@ -3490,10 +3496,12 @@ db::commitlog::read_log_file(const replay_state& state, sstring filename, sstrin auto checksum = crc.checksum(); if (check != checksum) { - throw segment_data_corruption_error("Data corruption", alignment); + auto reason = fmt::format("checksums do not match: {:x} vs. {:x}", check, checksum); + throw segment_data_corruption_error(std::move(reason), alignment); } if (id != this->id) { - throw segment_truncation(pos + rem); + auto reason = fmt::format("IDs do not match: {} vs. {}", id, this->id); + throw segment_truncation(std::move(reason), pos + rem); } } tmp.remove_suffix(detail::sector_overhead_size); @@ -3768,7 +3776,8 @@ db::commitlog::read_log_file(const replay_state& state, sstring filename, sstrin co_await read_chunk(); } if (corrupt_size > 0) { - throw segment_data_corruption_error("Data corruption", corrupt_size); + auto reason = fmt::format("corrupted size while reading file: {}", corrupt_size); + throw segment_data_corruption_error(std::move(reason), corrupt_size); } } catch (...) { p = std::current_exception(); diff --git a/db/commitlog/commitlog.hh b/db/commitlog/commitlog.hh index df79c6ae6b..b2e54338ff 100644 --- a/db/commitlog/commitlog.hh +++ b/db/commitlog/commitlog.hh @@ -392,9 +392,7 @@ public: class segment_data_corruption_error: public segment_error { std::string _msg; public: - segment_data_corruption_error(std::string msg, uint64_t s) - : _msg(std::move(msg)), _bytes(s) { - } + segment_data_corruption_error(std::string_view msg, uint64_t s); uint64_t bytes() const { return _bytes; } @@ -425,7 +423,7 @@ public: std::string _msg; uint64_t _pos; public: - segment_truncation(uint64_t); + segment_truncation(std::string_view reason, uint64_t position); uint64_t position() const; const char* what() const noexcept override;