From 19285ca35f92553639fcdb2e421c1b4aebb237ec Mon Sep 17 00:00:00 2001 From: Travis Downs Date: Thu, 16 Feb 2023 13:20:38 -0300 Subject: [PATCH 1/5] Allow abort on OOM to be set both ways There is a global "abort on OOM" setting in the seastar, and we allow enabling it via a public method in seastar::memory. There was no corresponding method to disable it, but such a method is useful in order to give applications full control over this setting. Introduce a new method which takes a boolean parameter in order to allow both enabling and disabling the abort behavior. The old call is deprecated and simply falls the new method with enabled=true. --- include/seastar/core/memory.hh | 20 ++++++++++++++++++++ src/core/memory.cc | 15 +++++++++++---- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/include/seastar/core/memory.hh b/include/seastar/core/memory.hh index 9972d8f48b1..30454aeac76 100644 --- a/include/seastar/core/memory.hh +++ b/include/seastar/core/memory.hh @@ -139,6 +139,8 @@ static constexpr size_t huge_page_size = void configure(std::vector m, bool mbind, std::optional hugetlbfs_path = {}); +// A deprecated alias for set_abort_on_allocation_failure(true). +[[deprecated("use set_abort_on_allocation_failure(true) instead")]] void enable_abort_on_allocation_failure(); class disable_abort_on_alloc_failure_temporarily { @@ -223,6 +225,24 @@ void set_reclaim_hook( /// \endcond +/// \brief Set the global state of the abort on allocation failure behavior. +/// +/// If enabled, an allocation failure (i.e., the requested memory +/// could not be allocated even after reclaim was attempted), will +/// generally result in `abort()` being called. If disabled, the +/// failure is reported to the caller, e.g., by throwing a +/// `std::bad_alloc` for C++ allocations such as new, or returning +/// a null pointer from malloc. +/// +/// Note that even if the global state is set to enabled, the +/// `disable_abort_on_alloc_failure_temporarily` class may override +/// the behavior tepmorarily on a given shard. That is, abort only +/// occurs if abort is globablly enabled on this shard _and_ there +/// are no `disable_abort_on_alloc_failure_temporarily` objects +/// currently alive on this shard. +void set_abort_on_allocation_failure(bool enabled); + + class statistics; /// Capture a snapshot of memory allocation statistics for this lcore. diff --git a/src/core/memory.cc b/src/core/memory.cc index 39ccb45ed0b..c8ad2f678cb 100644 --- a/src/core/memory.cc +++ b/src/core/memory.cc @@ -100,6 +100,10 @@ disable_abort_on_alloc_failure_temporarily::~disable_abort_on_alloc_failure_temp --abort_on_alloc_failure_suppressed; } +void enable_abort_on_allocation_failure() { + set_abort_on_allocation_failure(true); +} + static std::pmr::polymorphic_allocator static_malloc_allocator{std::pmr::get_default_resource()};; std::pmr::polymorphic_allocator* malloc_allocator{&static_malloc_allocator}; @@ -1600,8 +1604,8 @@ class disable_report_on_alloc_failure_temporarily { static std::atomic abort_on_allocation_failure{false}; static std::atomic dump_diagnostics_on_alloc_failure_kind{alloc_failure_kind::critical}; -void enable_abort_on_allocation_failure() { - abort_on_allocation_failure.store(true, std::memory_order_seq_cst); +void set_abort_on_allocation_failure(bool enabled) { + abort_on_allocation_failure.store(enabled, std::memory_order_seq_cst); } void set_dump_memory_diagnostics_on_alloc_failure_kind(alloc_failure_kind kind) { @@ -2268,8 +2272,11 @@ scoped_heap_profiling::scoped_heap_profiling() noexcept { scoped_heap_profiling::~scoped_heap_profiling() { } -void enable_abort_on_allocation_failure() { - seastar_logger.warn("Seastar compiled with default allocator, will not abort on bad_alloc"); +void set_abort_on_allocation_failure(bool enabled) { + if (enabled) { + seastar_logger.warn("Seastar compiled with default allocator, will not abort on bad_alloc"); + } +} } reclaimer::reclaimer(std::function reclaim, reclaimer_scope) { From 8870b9c14d08da1e0973b44839cecb2168c58755 Mon Sep 17 00:00:00 2001 From: Travis Downs Date: Thu, 16 Feb 2023 13:21:48 -0300 Subject: [PATCH 2/5] Allow application to query the abort on OOM state Add an is_abort_on_allocation_failure method which returns the global state of this reactor option. This is useful for applications that which to query this state in order to expose it for debugging or administration purposes. Applications could try to track this themselves, but this is unreliable since the reactor itself can change this value based on the command line flags. --- include/seastar/core/memory.hh | 6 ++++++ src/core/memory.cc | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/include/seastar/core/memory.hh b/include/seastar/core/memory.hh index 30454aeac76..2cc7691c52d 100644 --- a/include/seastar/core/memory.hh +++ b/include/seastar/core/memory.hh @@ -242,6 +242,12 @@ void set_reclaim_hook( /// currently alive on this shard. void set_abort_on_allocation_failure(bool enabled); +/// \brief Determine the abort on allocation failure mode. +/// +/// Return true if the global abort on allocation failure behavior +/// is enabled, or false otherwise. Always returns false if the +/// default (system) allocator is being used. +bool is_abort_on_allocation_failure(); class statistics; diff --git a/src/core/memory.cc b/src/core/memory.cc index c8ad2f678cb..d14d36005fd 100644 --- a/src/core/memory.cc +++ b/src/core/memory.cc @@ -1608,6 +1608,10 @@ void set_abort_on_allocation_failure(bool enabled) { abort_on_allocation_failure.store(enabled, std::memory_order_seq_cst); } +bool is_abort_on_allocation_failure() { + return abort_on_allocation_failure; +} + void set_dump_memory_diagnostics_on_alloc_failure_kind(alloc_failure_kind kind) { dump_diagnostics_on_alloc_failure_kind.store(kind, std::memory_order_seq_cst); } @@ -2277,6 +2281,9 @@ void set_abort_on_allocation_failure(bool enabled) { seastar_logger.warn("Seastar compiled with default allocator, will not abort on bad_alloc"); } } + +bool is_abort_on_allocation_failure() { + return false; } reclaimer::reclaimer(std::function reclaim, reclaimer_scope) { From 0262078b3922835369fe2a4060f387a67450475c Mon Sep 17 00:00:00 2001 From: Travis Downs Date: Thu, 16 Feb 2023 13:24:23 -0300 Subject: [PATCH 3/5] Add a test for enable/disable abort on OOM A basic test which flips the abort on OOM switch up and down and which checks that the is_enabled function returns the correct value. --- tests/unit/alloc_test.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/alloc_test.cc b/tests/unit/alloc_test.cc index f7c33a0eb7d..847ccec14d9 100644 --- a/tests/unit/alloc_test.cc +++ b/tests/unit/alloc_test.cc @@ -198,6 +198,20 @@ SEASTAR_TEST_CASE(test_realloc_nullptr) { return make_ready_future<>(); } +SEASTAR_TEST_CASE(test_enable_abort_on_oom) { + bool original = seastar::memory::is_abort_on_allocation_failure(); + + seastar::memory::set_abort_on_allocation_failure(false); + BOOST_CHECK(!seastar::memory::is_abort_on_allocation_failure()); + + seastar::memory::set_abort_on_allocation_failure(true); + BOOST_CHECK(seastar::memory::is_abort_on_allocation_failure()); + + seastar::memory::set_abort_on_allocation_failure(original); + + return make_ready_future<>(); +} + void * volatile sink; SEASTAR_TEST_CASE(test_bad_alloc_throws) { From dad0d992434e9a3bd5332fdc2547049879752e55 Mon Sep 17 00:00:00 2001 From: Travis Downs Date: Mon, 20 Feb 2023 10:49:39 -0300 Subject: [PATCH 4/5] Replace deprecated enable abort call Replace enable_abort_on_allocation_failure with set_abort_on_allocation_failure(true) as the former is deprecated. --- src/core/reactor.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/reactor.cc b/src/core/reactor.cc index 264f1e50a7b..ab3fedabefa 100644 --- a/src/core/reactor.cc +++ b/src/core/reactor.cc @@ -4205,7 +4205,7 @@ void smp::configure(const smp_options& smp_opts, const reactor_options& reactor_ } if (reactor_opts.abort_on_seastar_bad_alloc) { - memory::enable_abort_on_allocation_failure(); + memory::set_abort_on_allocation_failure(true); } if (reactor_opts.dump_memory_diagnostics_on_alloc_failure_kind) { From 3077ab32922bc5f60f4a1aaef954caa0287c7c81 Mon Sep 17 00:00:00 2001 From: Travis Downs Date: Wed, 22 Feb 2023 11:35:21 -0300 Subject: [PATCH 5/5] Update memory namespace comment. To include details of the set_abort_on_allocation_failure API. --- include/seastar/core/memory.hh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/include/seastar/core/memory.hh b/include/seastar/core/memory.hh index 2cc7691c52d..85d74d767f5 100644 --- a/include/seastar/core/memory.hh +++ b/include/seastar/core/memory.hh @@ -95,9 +95,11 @@ namespace seastar { /// /// Often, the best way to debug an allocation failure is a coredump. This /// feature allows dumping core on allocation failures, containing the stack of -/// the failed allocation, by means of aborting. To enable set the -/// `abort_on_seastar_bad_alloc` configuration option or the respective command -/// line flag. +/// the failed allocation, by means of aborting. To enable this behavior, set +/// `abort_on_seastar_bad_alloc` in `reactor_options` or pass the +/// `--abort-on-seastar-bad-alloc` command line flag. Additionally, applications +/// may enable or disable this functionality globally at runtime by calling +/// `set_abort_on_allocation_failure()`. /// /// ### Dump diagnostics report ///