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

util/backtrace: Optimize formatter to reduce memory allocation overhead #2572

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Dec 10, 2024

This commit addresses a critical memory allocation issue in backtrace formatting by directly specializing fmt::formatter for backtrace types, eliminating dependency on iostream-based formatting.

Problem:

When Seastar applications experience high memory pressure, logging backtrace information could fail due to additional memory allocation required by iostream formatting. This resulted in errors like:

ERROR 2024-12-10 01:59:16,905 [shard 0:main] seastar_memory - seastar/src/core/memory.cc:2126 @void seastar::memory::maybe_dump_memory_diagnostics(size_t, bool): failed to log message: fmt='Failed to allocate {} bytes at {}': std::__ios_failure (error iostream:1, basic_ios::clear: iostream error)"

Solution:

  • Implement direct fmt::formatter specialization for backtrace
  • Remove reliance on operator<< for string representation
  • Reduce memory allocation pressure during out-of-memory scenarios
  • Improve reliability of backtrace logging under extreme memory constraints

Additional Compatibility Note:

Due to changes in fmt library versioning, this implementation ensures fmt::formatter is always defined for backtrace types, even when using fmt versions earlier than 9.0. Previously, these formatters were conditionally defined based on the fmt version. Now, the formatters are consistently available, with operator<< implemented using the new fmt::formatter specialization.

This change ensures that backtrace logging can proceed even when the system is under significant memory pressure, providing more reliable post-mortem debugging information.

@tchaikov
Copy link
Contributor Author

tested with scylla master HEAD. the new implementation is able to format the backtrace, like

DEBUG 2024-12-10 15:18:12,736 [shard 0:main] seastar_memory - Failed to allocate 131072 bytes at 0x130cf7e 0x130d2a0 0x130d598 0x122be82 0x122a60f 0x122a59f 0x122c6c5 0x5559036 0x13c9b3f 0x13c95c5 0x130ed96
   --------
   seastar::continuation<seastar::internal::promise_base_with_type<int>, seastar::async<scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}>(seastar::thread_attributes, scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}&&)::{lambda()#2}, seastar::future<void>::then_impl_nrvo<seastar::async<scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}>(seastar::thread_attributes, scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}&&)::{lambda()#2}, seastar::future<int> >(scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}&&)::{lambda(seastar::internal::promise_base_with_type<int>&&, seastar::async<scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}>(seastar::thread_attributes, auto:1&&, (auto:2&&)...)::{lambda()#2}&, seastar::future_state<seastar::internal::monostate>&&)#1}, void>
   --------
   seastar::continuation<seastar::internal::promise_base_with_type<int>, seastar::future<int>::finally_body<seastar::async<scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}>(seastar::thread_attributes, scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}&&)::{lambda()#3}, false>, seastar::future<int>::then_wrapped_nrvo<seastar::future<int>, seastar::future<int>::finally_body<seastar::async<scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}>(seastar::thread_attributes, scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}&&)::{lambda()#3}, false> >(seastar::future<int>::finally_body<seastar::async<scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}>(seastar::thread_attributes, scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}&&)::{lambda()#3}, false>&&)::{lambda(seastar::internal::promise_base_with_type<int>&&, seastar::future<int>::finally_body<seastar::async<scylla_main(int, char**)::$_0::operator()() const::{lambda()#2}>(seastar::thread_attributes, auto:1&&, (auto:2&&)...)::{lambda()#3}, false>&, seastar::future_state<int>&&)#1}, int>
   --------
   seastar::continuation<seastar::internal::promise_base_with_type<int>, seastar::future<int>::finally_body<seastar::app_template::run(int, char**, std::function<seastar::future<int> ()>&&)::$_0::operator()()::{lambda()#2}, false>, seastar::future<int>::then_wrapped_nrvo<seastar::future<int>, seastar::future<int>::finally_body<seastar::app_template::run(int, char**, std::function<seastar::future<int> ()>&&)::$_0::operator()()::{lambda()#2}, false> >(seastar::future<int>::finally_body<seastar::app_template::run(int, char**, std::function<seastar::future<int> ()>&&)::$_0::operator()()::{lambda()#2}, false>&&)::{lambda(seastar::internal::promise_base_with_type<int>&&, seastar::future<int>::finally_body<seastar::app_template::run(int, char**, std::function<seastar::future<int> ()>&&)::$_0::operator()()::{lambda()#2}, false>&, seastar::future_state<int>&&)#1}, int>
   --------
   seastar::continuation<seastar::internal::promise_base_with_type<void>, seastar::app_template::run(int, char**, std::function<seastar::future<int> ()>&&)::$_0::operator()()::{lambda(int)#1}, seastar::future<int>::then_impl_nrvo<seastar::app_template::run(int, char**, std::function<seastar::future<int> ()>&&)::$_0::operator()()::{lambda(int)#1}, seastar::future<void> >(seastar::app_template::run(int, char**, std::function<seastar::future<int> ()>&&)::$_0::operator()()::{lambda(int)#1}&&)::{lambda(seastar::internal::promise_base_with_type<void>&&, seastar::app_template::run(int, char**, std::function<seastar::future<int> ()>&&)::$_0::operator()()::{lambda(int)#1}&, seastar::future_state<int>&&)#1}, int>
   --------
   seastar::continuation<seastar::internal::promise_base_with_type<void>, seastar::future<void>::or_terminate()::{lambda(auto:1&&)#1}, seastar::future<void>::then_wrapped_nrvo<void, seastar::future<void>::or_terminate()::{lambda(auto:1&&)#1}>(seastar::future<void>::or_terminate()::{lambda(auto:1&&)#1}&&)::{lambda(seastar::internal::promise_base_with_type<void>&&, seastar::future<void>::or_terminate()::{lambda(auto:1&&)#1}&, seastar::future_state<seastar::internal::monostate>&&)#1}, void>

the format is identical to existing implementation.

@tchaikov tchaikov force-pushed the backtrace-formatter branch from c0abaf9 to f0792c1 Compare December 10, 2024 07:27
@tchaikov tchaikov force-pushed the backtrace-formatter branch from f0792c1 to dd9c2b1 Compare December 10, 2024 10:28
This commit addresses a critical memory allocation issue in backtrace
formatting by directly specializing fmt::formatter for backtrace types,
eliminating dependency on iostream-based formatting.

Problem:
When Seastar applications experience high memory pressure, logging
backtrace information could fail due to additional memory allocation
required by iostream formatting. This resulted in errors like:

```
ERROR 2024-12-10 01:59:16,905 [shard 0:main] seastar_memory - seastar/src/core/memory.cc:2126 @void seastar::memory::maybe_dump_memory_diagnostics(size_t, bool): failed to log message: fmt='Failed to allocate {} bytes at {}': std::__ios_failure (error iostream:1, basic_ios::clear: iostream error)"
```

Solution:
- Implement direct fmt::formatter specialization for backtrace
- Remove reliance on operator<< for string representation
- Reduce memory allocation pressure during out-of-memory scenarios
- Improve reliability of backtrace logging under extreme memory constraints

Compatibility Improvements:
- Ensure consistent `fmt::formatter` availability across fmt library versions
- Deprecate two `simple_backtrace` constructors previously added in
  095a07b, as they are no longer needed with fmt::join
- Maintain robust formatting approach independent of library version

Impact:
This change enhances system resilience by enabling backtrace logging even
under significant memory pressure, providing more reliable post-mortem
debugging information.

Signed-off-by: Kefu Chai <[email protected]>
@tchaikov tchaikov force-pushed the backtrace-formatter branch from dd9c2b1 to 342e93c Compare December 10, 2024 12:06
@tchaikov
Copy link
Contributor Author

v2:

  • preserved the constructors accepting delimeter
  • marked the constructors accepting delimeter parameter deprecated.

@tchaikov
Copy link
Contributor Author

@scylladb/seastar-maint hello maintainers, could you help merge this change?

@avikivity avikivity merged commit 3133ecd into scylladb:master Dec 10, 2024
15 checks passed
@tchaikov tchaikov deleted the backtrace-formatter branch December 10, 2024 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants