Skip to content

Commit

Permalink
utils/logalloc: do not allocate memory in reclaim_timer::report()
Browse files Browse the repository at this point in the history
before this change, `reclaim_timer::report()` calls

```c++
fmt::format(", at {}", current_backtrace())
```

which allocates a `std::string` on heap, so it can fail and throw. in
that case, `std::terminate()` is called. but at that moment, the reason
why `reclaim_timer::report()` gets called is that we fail to reclaim
memory for the caller. so we are more likely to run into this issue. anyway,
we should not allocate memory in this path.

in this change, a dedicated printer is created so that we don't format
to a temporary `std::string`, and instead write directly to the buffer
of logger. this avoids the memory allocation.

Fixes scylladb#18099
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
  • Loading branch information
tchaikov committed Mar 29, 2024
1 parent 4c0dade commit a97e3cd
Showing 1 changed file with 30 additions and 1 deletion.
31 changes: 30 additions & 1 deletion utils/logalloc.cc
Expand Up @@ -204,6 +204,34 @@ migrate_fn_type::unregister_migrator(uint32_t index) {
static_migrators().remove(index);
}


namespace {

// for printing extra message in reclaim_timer::report when stall is detected
struct extra_msg_when_stall_detected {
bool stall_detected;
saved_backtrace backtrace;
extra_msg_when_stall_detected(bool detected, saved_backtrace&& backtrace)
: stall_detected{detected}
, backtrace{std::move(backtrace)}
{}
};

}

template <>
struct fmt::formatter<extra_msg_when_stall_detected> {
constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); }
auto format(const extra_msg_when_stall_detected& msg, fmt::format_context& ctx) const {
if (msg.stall_detected) {
return fmt::format_to(ctx.out(), ", at{}", msg.backtrace);
} else {
return ctx.out();
}
}
};


namespace logalloc {

// LSA-specific bad_alloc variant which allows adding additional information on
Expand Down Expand Up @@ -1511,7 +1539,8 @@ void reclaim_timer::report() const noexcept {
auto time_level = _stall_detected ? log_level::warn : log_level::debug;
auto info_level = _stall_detected ? log_level::info : log_level::debug;
auto MiB = 1024*1024;
auto msg_extra = _stall_detected ? fmt::format(", at {}", current_backtrace()) : "";
auto msg_extra = extra_msg_when_stall_detected(_stall_detected,
_stall_detected ? current_backtrace() : saved_backtrace{});

timing_logger.log(time_level, "{} took {} us, trying to release {:.3f} MiB {}preemptibly, reserve: {{goal: {}, max: {}}}{}",
_name, (_duration + 500ns) / 1us, (float)_memory_to_release / MiB, _preemptible ? "" : "non-",
Expand Down

0 comments on commit a97e3cd

Please sign in to comment.