Skip to content

Commit a674144

Browse files
adamemersonadamemerson
authored
Merge pull request ceph#63698 from adamemerson/wip-71066
rgw/multisite: Fix lifetime issues Reviewed-by: Casey Bodley <[email protected]>
2 parents ca6b06c + ae89a56 commit a674144

36 files changed

+2467
-2111
lines changed

‎src/cls/fifo/cls_fifo_types.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,4 +610,8 @@ template<>
610610
struct fmt::formatter<rados::cls::fifo::info> : fmt::ostream_formatter {};
611611
template<>
612612
struct fmt::formatter<rados::cls::fifo::part_header> : fmt::ostream_formatter {};
613+
template<>
614+
struct fmt::formatter<rados::cls::fifo::journal_entry> : fmt::ostream_formatter {};
615+
template<>
616+
struct fmt::formatter<rados::cls::fifo::update> : fmt::ostream_formatter {};
613617
#endif

‎src/include/neorados/RADOS.hpp‎

Lines changed: 67 additions & 96 deletions
Large diffs are not rendered by default.

‎src/neorados/RADOS.cc‎

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -897,9 +897,10 @@ void RADOS::make_with_cct_(CephContext* cct,
897897
asio::io_context& ioctx,
898898
BuildComp c) {
899899
try {
900-
auto r = new detail::NeoClient{std::make_unique<detail::RADOS>(ioctx, cct)};
900+
auto r = std::make_shared<detail::NeoClient>(
901+
std::make_unique<detail::RADOS>(ioctx, cct));
901902
r->objecter->wait_for_osd_map(
902-
[c = std::move(c), r = std::unique_ptr<detail::Client>(r)]() mutable {
903+
[c = std::move(c), r = std::move(r)]() mutable {
903904
asio::dispatch(asio::append(std::move(c), bs::error_code{},
904905
RADOS{std::move(r)}));
905906
});
@@ -910,14 +911,17 @@ void RADOS::make_with_cct_(CephContext* cct,
910911
}
911912

912913
RADOS RADOS::make_with_librados(librados::Rados& rados) {
913-
return RADOS{std::make_unique<detail::RadosClient>(rados.client)};
914+
return RADOS{std::make_shared<detail::RadosClient>(rados.client)};
914915
}
915916

916917
RADOS::RADOS() = default;
917918

918-
RADOS::RADOS(std::unique_ptr<detail::Client> impl)
919+
RADOS::RADOS(std::shared_ptr<detail::Client> impl)
919920
: impl(std::move(impl)) {}
920921

922+
RADOS::RADOS(const RADOS&) = default;
923+
RADOS& RADOS::operator =(const RADOS&) = default;
924+
921925
RADOS::RADOS(RADOS&&) = default;
922926
RADOS& RADOS::operator =(RADOS&&) = default;
923927

@@ -934,7 +938,8 @@ asio::io_context& RADOS::get_io_context() {
934938
void RADOS::execute_(Object o, IOContext _ioc, ReadOp _op,
935939
cb::list* bl,
936940
ReadOp::Completion c, version_t* objver,
937-
const blkin_trace_info *trace_info) {
941+
const blkin_trace_info *trace_info,
942+
std::uint64_t subsystem) {
938943
if (_op.size() == 0) {
939944
asio::dispatch(asio::append(std::move(c), bs::error_code{}));
940945
return;
@@ -953,14 +958,16 @@ void RADOS::execute_(Object o, IOContext _ioc, ReadOp _op,
953958
trace.event("init");
954959
impl->objecter->read(
955960
*oid, ioc->oloc, std::move(op->op), ioc->snap_seq, bl, flags,
956-
std::move(c), objver, nullptr /* data_offset */, 0 /* features */, &trace);
961+
std::move(c), objver, nullptr /* data_offset */, 0 /* features */, &trace,
962+
subsystem);
957963

958964
trace.event("submitted");
959965
}
960966

961967
void RADOS::execute_(Object o, IOContext _ioc, WriteOp _op,
962968
WriteOp::Completion c, version_t* objver,
963-
const blkin_trace_info *trace_info) {
969+
const blkin_trace_info *trace_info,
970+
std::uint64_t subsystem) {
964971
if (_op.size() == 0) {
965972
asio::dispatch(asio::append(std::move(c), bs::error_code{}));
966973
return;
@@ -985,7 +992,7 @@ void RADOS::execute_(Object o, IOContext _ioc, WriteOp _op,
985992
impl->objecter->mutate(
986993
*oid, ioc->oloc, std::move(op->op), ioc->snapc,
987994
mtime, flags,
988-
std::move(c), objver, osd_reqid_t{}, &trace);
995+
std::move(c), objver, osd_reqid_t{}, &trace, subsystem);
989996
trace.event("submitted");
990997
}
991998

@@ -1382,8 +1389,12 @@ class Notifier : public async::service_list_base_hook {
13821389
std::deque<id_and_handler> handlers;
13831390
std::mutex m;
13841391
uint64_t next_id = 0;
1392+
std::shared_ptr<detail::Client> neoref;
13851393

13861394
void service_shutdown() {
1395+
if (neoref) {
1396+
neoref = nullptr;
1397+
}
13871398
if (linger_op) {
13881399
linger_op->put();
13891400
}
@@ -1394,10 +1405,11 @@ class Notifier : public async::service_list_base_hook {
13941405
public:
13951406

13961407
Notifier(asio::io_context::executor_type ex, Objecter::LingerOp* linger_op,
1397-
uint32_t capacity)
1408+
uint32_t capacity, std::shared_ptr<detail::Client> neoref)
13981409
: ex(ex), linger_op(linger_op), capacity(capacity),
13991410
svc(asio::use_service<async::service<Notifier>>(
1400-
asio::query(ex, boost::asio::execution::context))) {
1411+
asio::query(ex, boost::asio::execution::context))),
1412+
neoref(std::move(neoref)) {
14011413
// register for service_shutdown() notifications
14021414
svc.add(*this);
14031415
}
@@ -1534,7 +1546,7 @@ void RADOS::watch_(Object o, IOContext _ioc, WatchComp c,
15341546
uint64_t cookie = linger_op->get_cookie();
15351547
// Shared pointer to avoid a potential race condition
15361548
linger_op->user_data.emplace<std::shared_ptr<Notifier>>(
1537-
std::make_shared<Notifier>(get_executor(), linger_op, queue_size));
1549+
std::make_shared<Notifier>(get_executor(), linger_op, queue_size, impl));
15381550
auto& n = ceph::any_cast<std::shared_ptr<Notifier>&>(
15391551
linger_op->user_data);
15401552
linger_op->handle = std::ref(*n);
@@ -1957,6 +1969,17 @@ uint64_t RADOS::instance_id() const {
19571969
return impl->get_instance_id();
19581970
}
19591971

1972+
uint64_t RADOS::new_subsystem() const
1973+
{
1974+
return impl->objecter->unique_subsystem_id();
1975+
}
1976+
1977+
void RADOS::cancel_subsystem(uint64_t subsystem) const
1978+
{
1979+
impl->objecter->subsystem_cancel(subsystem,
1980+
asio::error::operation_aborted);
1981+
}
1982+
19601983
#pragma GCC diagnostic push
19611984
#pragma GCC diagnostic ignored "-Wnon-virtual-dtor"
19621985
#pragma clang diagnostic push

‎src/neorados/RADOSImpl.cc‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,6 @@
1616

1717
#include <boost/system/system_error.hpp>
1818

19-
#include "common/common_init.h"
20-
21-
#include "global/global_init.h"
22-
2319
namespace neorados {
2420
namespace detail {
2521

@@ -83,17 +79,7 @@ RADOS::RADOS(boost::asio::io_context& ioctx,
8379
}
8480

8581
RADOS::~RADOS() {
86-
if (objecter && objecter->initialized) {
87-
objecter->shutdown();
88-
}
89-
90-
mgrclient.shutdown();
91-
monclient.shutdown();
92-
93-
if (messenger) {
94-
messenger->shutdown();
95-
messenger->wait();
96-
}
82+
shutdown();
9783
}
9884

9985
bool RADOS::ms_dispatch(Message *m)
@@ -116,5 +102,19 @@ bool RADOS::ms_handle_refused(Connection *con) {
116102
return false;
117103
}
118104

105+
void RADOS::shutdown() {
106+
if (objecter && objecter->initialized) {
107+
objecter->shutdown();
108+
}
109+
110+
// These shutdowns are idempotent
111+
mgrclient.shutdown();
112+
monclient.shutdown();
113+
114+
if (messenger) {
115+
messenger->shutdown();
116+
messenger->wait();
117+
}
118+
}
119119
} // namespace detail
120120
} // namespace neorados

‎src/neorados/RADOSImpl.h‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@
1414
#ifndef CEPH_NEORADOS_RADOSIMPL_H
1515
#define CEPH_NEORADOS_RADOSIMPL_H
1616

17+
#include <atomic>
1718
#include <memory>
18-
#include <string>
1919

2020
#include <boost/intrusive_ptr.hpp>
2121

22+
#include "common/async/service.h"
23+
2224
#include "common/ceph_context.h"
2325
#include "common/ceph_mutex.h"
2426

@@ -40,15 +42,14 @@ namespace detail {
4042

4143
class NeoClient;
4244

43-
class RADOS : public Dispatcher
44-
{
45+
class RADOS : public Dispatcher {
4546
friend ::neorados::RADOS;
4647
friend NeoClient;
4748

4849
boost::asio::io_context& ioctx;
4950
boost::intrusive_ptr<CephContext> cct;
5051

51-
ceph::mutex lock = ceph::make_mutex("RADOS_unleashed::_::RADOSImpl");
52+
ceph::mutex lock = ceph::make_mutex("neorados::detail::RADOSImpl");
5253
int instance_id = -1;
5354

5455
std::unique_ptr<Messenger> messenger;
@@ -57,6 +58,7 @@ class RADOS : public Dispatcher
5758
MgrClient mgrclient;
5859

5960
std::unique_ptr<Objecter> objecter;
61+
std::atomic<bool> finished = false;
6062

6163
public:
6264

@@ -70,9 +72,10 @@ class RADOS : public Dispatcher
7072
mon_feature_t get_required_monitor_features() const {
7173
return monclient.with_monmap(std::mem_fn(&MonMap::get_required_features));
7274
}
75+
void shutdown();
7376
};
7477

75-
class Client {
78+
class Client : public std::enable_shared_from_this<Client> {
7679
public:
7780
Client(boost::asio::io_context& ioctx,
7881
boost::intrusive_ptr<CephContext> cct,
@@ -97,19 +100,38 @@ class Client {
97100
virtual int get_instance_id() const = 0;
98101
};
99102

100-
class NeoClient : public Client {
103+
class NeoClient : public Client,
104+
public ceph::async::service_list_base_hook {
101105
public:
106+
102107
NeoClient(std::unique_ptr<RADOS>&& rados)
103108
: Client(rados->ioctx, rados->cct, rados->monclient,
104-
rados->objecter.get()),
109+
rados->objecter.get()),
110+
svc(boost::asio::use_service<ceph::async::service<NeoClient>>(
111+
boost::asio::query(ioctx.get_executor(),
112+
boost::asio::execution::context))),
105113
rados(std::move(rados)) {
114+
svc.add(*this);
115+
}
116+
117+
~NeoClient() {
118+
svc.remove(*this);
106119
}
107120

108121
int get_instance_id() const override {
109122
return rados->instance_id;
110123
}
111124

125+
void service_shutdown() {
126+
// In case the last owner of a reference is an op we're about to
127+
// cancel. (This can happen if the `RADOS` object
128+
auto service_ref = shared_from_this();
129+
rados->shutdown();
130+
}
131+
112132
private:
133+
friend ceph::async::service<RADOS>;
134+
async::service<NeoClient>& svc;
113135
std::unique_ptr<RADOS> rados;
114136
};
115137

‎src/neorados/cls/common.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
#pragma once
1313

1414
#include <concepts>
15-
#include <coroutine>
1615
#include <cstddef>
1716
#include <string>
1817
#include <type_traits>
@@ -35,6 +34,10 @@
3534
#include "include/buffer.h"
3635
#include "include/encoding.h"
3736

37+
#if !defined(__clang__) && defined(__GNUC__) && (__GNUC__ < 13)
38+
#define BROKEN_CO_COMPOSED
39+
#endif
40+
3841
/// \file neorados/cls/common.h
3942
///
4043
/// \brief Helpers for writing simple CLS clients

0 commit comments

Comments
 (0)