From: Casey Bodley Date: Thu, 2 Feb 2023 14:15:19 +0000 (-0500) Subject: rgw/sync: wrap lease renewal exceptions in lease_aborted X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=d72e2e4a8bf896ef1b0ab38d4e5ac153f3ca54c1;p=ceph.git rgw/sync: wrap lease renewal exceptions in lease_aborted Signed-off-by: Casey Bodley --- diff --git a/src/rgw/sync/lease.h b/src/rgw/sync/lease.h index 1457d0c8d1069..01b68982969c2 100644 --- a/src/rgw/sync/lease.h +++ b/src/rgw/sync/lease.h @@ -12,6 +12,7 @@ #pragma once +#include #include "include/scope_guard.h" #include "common/ceph_time.h" #include "detail/lease_state.h" @@ -33,16 +34,35 @@ class LockClient { }; +/// Exception thrown by with_lease() whenever the wrapped coroutine is canceled +/// due to a renewal failure. +class lease_aborted : public std::runtime_error { + std::exception_ptr eptr; + public: + lease_aborted(std::exception_ptr eptr) + : runtime_error("lease aborted"), eptr(eptr) + {} + + /// Return the original exception that triggered the cancellation. + std::exception_ptr original_exception() const { return eptr; }; +}; + + /// \brief Call a coroutine under the protection of a continuous lease. /// /// Acquires exclusive access to a timed lock, then spawns the given coroutine -/// \ref cr. The lock is renewed at intervals of \ref duration / 2. The -/// coroutine is canceled if the lock is lost before its completion. +/// \ref cr. The lock is renewed at intervals of \ref duration / 2, and released +/// after the coroutine's completion. +/// +/// Exceptions from acquire() propagate directly to the caller. Exceptions from +/// release() are ignored in favor of the result of \ref cr. /// -/// Exceptions thrown by release() are ignored, but exceptions from acquire() -/// and renew() propagate back to the caller. If renew() is delayed long -/// enough for the lock to expire, a boost::system::system_error exception is -/// thrown with an error code matching boost::system::errc::timed_out. +/// If renew() is delayed long enough for the timed lock to expire, a +/// boost::system::system_error exception is thrown with an error code matching +/// boost::system::errc::timed_out. That exception, along with any exception +/// from renew() itself, gets wrapped in a lease_aborted exception when +/// reported to the caller. Any failure to renew the lock will also result in +/// the cancellation of \ref cr. /// /// Otherwise, the result of \ref cr is returned to the caller, whether by /// exception or return value. @@ -91,9 +111,10 @@ auto with_lease(LockClient& lock, timer.expires_at(expires_at); timer.async_wait([state] (error_code ec) { if (!ec) { - state->abort(std::make_exception_ptr( + auto eptr = std::make_exception_ptr( boost::system::system_error( - ETIMEDOUT, boost::system::system_category()))); + ETIMEDOUT, boost::system::system_category())); + state->abort(std::make_exception_ptr(lease_aborted{eptr})); } }); @@ -101,7 +122,8 @@ auto with_lease(LockClient& lock, co_await lock.renew(duration); expires_at = detail::lease_clock::now() + duration; } catch (const std::exception&) { - state->abort(std::current_exception()); + state->abort(std::make_exception_ptr( + lease_aborted{std::current_exception()})); expires_at = detail::lease_clock::zero(); // don't release below break; } diff --git a/src/test/rgw/test_rgw_sync_lease.cc b/src/test/rgw/test_rgw_sync_lease.cc index d0bd15fd24c86..6f543752b9be3 100644 --- a/src/test/rgw/test_rgw_sync_lease.cc +++ b/src/test/rgw/test_rgw_sync_lease.cc @@ -417,10 +417,16 @@ TEST(with_lease, renew_after_timeout) ASSERT_TRUE(*result); try { std::rethrow_exception(*result); - } catch (const boost::system::system_error& e) { - EXPECT_EQ(e.code(), boost::system::errc::timed_out); - } catch (const std::exception& e) { - EXPECT_THROW(throw, boost::system::system_error); + } catch (const lease_aborted& e) { + try { + std::rethrow_exception(e.original_exception()); + } catch (const boost::system::system_error& o) { + EXPECT_EQ(o.code(), boost::system::errc::timed_out); + } catch (const std::exception&) { + EXPECT_THROW(throw, boost::system::system_error); + } + } catch (const std::exception&) { + EXPECT_THROW(throw, lease_aborted); } } @@ -467,10 +473,16 @@ TEST(with_lease, renew_exception_after_timeout) ASSERT_TRUE(*result); try { std::rethrow_exception(*result); - } catch (const boost::system::system_error& e) { - EXPECT_EQ(e.code(), boost::system::errc::timed_out); - } catch (const std::exception& e) { - EXPECT_THROW(throw, boost::system::system_error); + } catch (const lease_aborted& e) { + try { + std::rethrow_exception(e.original_exception()); + } catch (const boost::system::system_error& o) { + EXPECT_EQ(o.code(), boost::system::errc::timed_out); + } catch (const std::exception&) { + EXPECT_THROW(throw, boost::system::system_error); + } + } catch (const std::exception&) { + EXPECT_THROW(throw, lease_aborted); } } @@ -520,10 +532,16 @@ TEST(with_lease, renew_cancel_after_timeout) ASSERT_TRUE(*result); try { std::rethrow_exception(*result); - } catch (const boost::system::system_error& e) { - EXPECT_EQ(e.code(), boost::system::errc::timed_out); - } catch (const std::exception& e) { - EXPECT_THROW(throw, boost::system::system_error); + } catch (const lease_aborted& e) { + try { + std::rethrow_exception(e.original_exception()); + } catch (const boost::system::system_error& o) { + EXPECT_EQ(o.code(), boost::system::errc::timed_out); + } catch (const std::exception&) { + EXPECT_THROW(throw, boost::system::system_error); + } + } catch (const std::exception&) { + EXPECT_THROW(throw, lease_aborted); } } @@ -600,10 +618,16 @@ TEST(with_lease, renew_cancel) ASSERT_TRUE(*result); // throws on cancellation try { std::rethrow_exception(*result); - } catch (const boost::system::system_error& e) { - EXPECT_EQ(e.code(), asio::error::operation_aborted); - } catch (const std::exception& e) { - EXPECT_THROW(throw, boost::system::system_error); + } catch (const lease_aborted& e) { + try { + std::rethrow_exception(e.original_exception()); + } catch (const boost::system::system_error& o) { + EXPECT_EQ(o.code(), asio::error::operation_aborted); + } catch (const std::exception&) { + EXPECT_THROW(throw, boost::system::system_error); + } + } catch (const std::exception&) { + EXPECT_THROW(throw, lease_aborted); } }