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

Log and throw when oversized writer blows up #5673

Merged
merged 4 commits into from
Sep 21, 2023
Merged

Log and throw when oversized writer blows up #5673

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
58b2c3b
Log AND throw when oversized writer blows up
eddyashton Sep 20, 2023
13b50a3
Merge branch 'main' into log_oversized_errors
eddyashton Sep 21, 2023
c7823bd
Merge branch 'main' into log_oversized_errors
achamayou Sep 21, 2023
238aa45
Merge branch 'main' into log_oversized_errors
achamayou Sep 21, 2023
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
40 changes: 29 additions & 11 deletions src/ds/oversized.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@
#include <fmt/format.h>
#include <unordered_map>

#define LOG_AND_THROW(ERROR_TYPE, ...) \
do \
{ \
const auto msg = fmt::format(__VA_ARGS__); \
LOG_FAIL_FMT("{}", msg); \
throw ERROR_TYPE(msg); \
} while (0)
namespace oversized
{
enum OversizedMessage : ringbuffer::Message
Expand Down Expand Up @@ -141,18 +148,24 @@ namespace oversized
fragment_progress({})
{
if (max_fragment_size >= max_total_size)
throw std::logic_error(fmt::format(
{
LOG_AND_THROW(
std::invalid_argument,
"Fragment sizes must be smaller than total max: {} >= {}",
max_fragment_size,
max_total_size));
max_total_size);
}

constexpr auto header_size = sizeof(InitialFragmentHeader);
if (max_fragment_size <= header_size)
throw std::logic_error(fmt::format(
{
LOG_AND_THROW(
std::invalid_argument,
"Fragment size must be large enough to contain the header for the "
"initial fragment, and some additional payload data: {} <= {}",
max_fragment_size,
header_size));
header_size);
}
}

virtual WriteMarker prepare(
Expand All @@ -164,7 +177,8 @@ namespace oversized
// Ensure this is not called out of order
if (fragment_progress.has_value())
{
throw std::logic_error("This Writer is already preparing a message");
LOG_AND_THROW(
std::logic_error, "This Writer is already preparing a message");
}

// Small enough to be handled directly by underlying writer
Expand All @@ -175,21 +189,23 @@ namespace oversized

if (total_size > max_total_size)
{
throw std::logic_error(fmt::format(
LOG_AND_THROW(
std::invalid_argument,
"Requested a write of {} bytes, max allowed is {}",
total_size,
max_total_size));
max_total_size);
}

// Need to split this message into multiple fragments

if (!wait)
{
throw std::logic_error(fmt::format(
LOG_AND_THROW(
std::invalid_argument,
"Requested write of {} bytes will be split into multiple fragments: "
"caller must wait for these to complete as fragment writes will be "
"blocking",
total_size));
total_size);
}

// Prepare space for the first fragment, getting an id for all related
Expand Down Expand Up @@ -227,7 +243,8 @@ namespace oversized
// to us
if (fragment_progress->remainder != 0)
{
throw std::logic_error(
LOG_AND_THROW(
std::logic_error,
"Attempting to finish an oversized message before the entire "
"requested payload has been written");
}
Expand Down Expand Up @@ -278,7 +295,8 @@ namespace oversized
if (!next.has_value())
{
// Intermediate fragment failed - this is unexpected
throw std::logic_error(
LOG_AND_THROW(
std::logic_error,
"Failed to create fragment for oversized message");

// If this path is hit it is likely because we have allowed oversized
Expand Down