Skip to content

Commit 5d01518

Browse files
authored
Merge pull request ceph#62288 from athanatos/sjust/wip-crimson-recovery-69412-replica
crimson: take read lock on replica during final push commit Reviewed-by: Matan Breizman <[email protected]>
2 parents 5749b63 + 482036c commit 5d01518

File tree

8 files changed

+181
-103
lines changed

8 files changed

+181
-103
lines changed

src/crimson/osd/object_context.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,22 @@ class ObjectContext : public ceph::common::intrusive_lru_base<
107107
ceph_assert(is_head());
108108
obs = std::move(_obs);
109109
ssc = std::move(_ssc);
110+
// ObjectContextLoader::load_and_lock* rely on this to determine whether to
111+
// start loading from disk. As this method fills in the metadata, such
112+
// loading will not be necessary in the future. loading_started will already
113+
// be set if this is invoked from ObjectContextLoader::load_and_lock*
114+
loading_started = true;
110115
fully_loaded = true;
111116
}
112117

113118
void set_clone_state(ObjectState &&_obs) {
114119
ceph_assert(!is_head());
115120
obs = std::move(_obs);
121+
// ObjectContextLoader::load_and_lock* rely on this to determine whether to
122+
// start loading from disk. As this method fills in the metadata, such
123+
// loading will not be necessary in the future. loading_started will already
124+
// be set if this is invoked from ObjectContextLoader::load_and_lock*
125+
loading_started = true;
116126
fully_loaded = true;
117127
}
118128

src/crimson/osd/object_context_loader.cc

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ ObjectContextLoader::load_and_lock_head(Manager &manager, RWState::State lock_ty
2222
auto [obc, _] = obc_registry.get_cached_obc(manager.target);
2323
manager.set_state_obc(manager.head_state, obc);
2424
}
25-
ceph_assert(manager.target_state.is_empty());
26-
manager.set_state_obc(manager.target_state, manager.head_state.obc);
25+
if (manager.target_state.is_empty()) {
26+
manager.set_state_obc(manager.target_state, manager.head_state.obc);
27+
}
2728

2829
if (manager.target_state.obc->loading_started) {
2930
co_await manager.target_state.lock_to(lock_type);
@@ -45,7 +46,6 @@ ObjectContextLoader::load_and_lock_clone(
4546
auto releaser = manager.get_releaser();
4647

4748
ceph_assert(!manager.target.is_head());
48-
ceph_assert(manager.target_state.is_empty());
4949

5050
if (manager.head_state.is_empty()) {
5151
auto [obc, _] = obc_registry.get_cached_obc(manager.target.get_head());
@@ -65,6 +65,9 @@ ObjectContextLoader::load_and_lock_clone(
6565
}
6666

6767
if (manager.options.resolve_clone) {
68+
// target_state must be empty because we won't know which object to load
69+
// until now
70+
ceph_assert(manager.target_state.is_empty());
6871
auto resolved_oid = resolve_oid(
6972
manager.head_state.obc->get_head_ss(),
7073
manager.target);
@@ -91,6 +94,8 @@ ObjectContextLoader::load_and_lock_clone(
9194
* actually can mutate a clone do not set resolve_clone, so target will not
9295
* become head here.
9396
*/
97+
ceph_assert(manager.options.resolve_clone);
98+
ceph_assert(manager.target_state.is_empty());
9499
manager.set_state_obc(manager.target_state, manager.head_state.obc);
95100
if (lock_type != manager.head_state.state) {
96101
// This case isn't actually possible at the moment for the above reason.
@@ -101,11 +106,20 @@ ObjectContextLoader::load_and_lock_clone(
101106
manager.head_state.state = RWState::RWNONE;
102107
}
103108
} else {
104-
auto [obc, _] = obc_registry.get_cached_obc(manager.target);
105-
manager.set_state_obc(manager.target_state, obc);
109+
// caller may have already populated this if !resolve_clone
110+
if (manager.target_state.is_empty()) {
111+
auto [obc, _] = obc_registry.get_cached_obc(manager.target);
112+
manager.set_state_obc(manager.target_state, obc);
113+
}
106114

107115
if (manager.target_state.obc->loading_started) {
108116
co_await manager.target_state.lock_to(RWState::RWREAD);
117+
if (!manager.target_state.obc->ssc) {
118+
// A cached clone obc may have a null ssc if created via
119+
// create_cached_obc_from_push_data. This interface
120+
// is responsible for fixing that if found.
121+
manager.target_state.obc->ssc = manager.head_state.obc->ssc;
122+
}
109123
} else {
110124
manager.target_state.lock_excl_sync();
111125
manager.target_state.obc->loading_started = true;
@@ -115,6 +129,8 @@ ObjectContextLoader::load_and_lock_clone(
115129
}
116130
}
117131

132+
ceph_assert(manager.target_state.obc->ssc);
133+
ceph_assert(manager.head_state.obc->ssc);
118134
releaser.cancel();
119135
}
120136

src/crimson/osd/object_context_loader.h

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ class ObjectContextLoader {
136136
friend ObjectContextLoader;
137137

138138
void set_state_obc(state_t &s, ObjectContextRef _obc) {
139+
ceph_assert(s.is_empty());
139140
s.obc = std::move(_obc);
140141
s.obc->append_to(loader.obc_set_accessing);
141142
}
@@ -210,6 +211,37 @@ class ObjectContextLoader {
210211
}
211212
};
212213

214+
/**
215+
* create_cached_obc_from_push_data
216+
*
217+
* Creates a fresh cached obc from passed oi and ssc.
218+
* Overwrites any obc already in cache for this object.
219+
*
220+
* Note, this interface may be used to create a clone obc
221+
* with a null ssc. The capability is useful when handling
222+
* a clone push on a replica -- we don't necessarily have
223+
* a valid local copy of the head since the primary may not
224+
* push the head first. That obc with a null ssc may
225+
* remain in the cache. ObjectContextLoader::load_and_lock_clone
226+
* will fix the ssc member if null, but users of any other
227+
* access mechanism must be aware that ssc on a clone obc may be
228+
* null.
229+
*/
230+
ObjectContextRef create_cached_obc_from_push_data(
231+
const object_info_t &oi,
232+
SnapSetContextRef ssc) {
233+
auto obc = obc_registry.get_cached_obc(oi.soid).first;
234+
if (oi.soid.is_head()) {
235+
ceph_assert(ssc); // head, ssc may not be null
236+
obc->set_head_state(ObjectState(oi, true), SnapSetContextRef(ssc));
237+
} else {
238+
// clone, ssc may be null
239+
obc->set_clone_state(ObjectState(oi, true));
240+
obc->set_clone_ssc(SnapSetContextRef(ssc));
241+
}
242+
return obc;
243+
}
244+
213245
Orderer get_obc_orderer(const hobject_t &oid) {
214246
Orderer ret;
215247
std::tie(ret.orderer_obc, std::ignore) =

src/crimson/osd/pg.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1315,9 +1315,10 @@ void PG::log_operation(
13151315
void PG::replica_clear_repop_obc(
13161316
const std::vector<pg_log_entry_t> &logv) {
13171317
LOG_PREFIX(PG::replica_clear_repop_obc);
1318-
DEBUGDPP("clearing obc for {} log entries", logv.size());
1318+
DEBUGDPP("clearing obc for {} log entries", *this, logv.size());
13191319
for (auto &&e: logv) {
13201320
DEBUGDPP("clearing entry for {} from: {} to: {}",
1321+
*this,
13211322
e.soid,
13221323
e.soid.get_object_boundary(),
13231324
e.soid.get_head());

src/crimson/osd/recovery_backend.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ void RecoveryBackend::clean_up(ceph::os::Transaction& t,
5050
});
5151
clear_temp_objs();
5252

53+
replica_push_targets.clear();
54+
5355
for (auto& [soid, recovery_waiter] : recovering) {
5456
if ((recovery_waiter->pull_info
5557
&& recovery_waiter->pull_info->is_complete())

src/crimson/osd/recovery_backend.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,14 @@ class RecoveryBackend {
272272
virtual interruptible_future<> handle_backfill_op(
273273
Ref<MOSDFastDispatchOp> m,
274274
crimson::net::ConnectionXcoreRef conn);
275+
276+
/**
277+
* replica_push_targets
278+
*
279+
* Holds obc on replica for in-progress pushes, see
280+
* ReplicatedRecoveryBackend::handle_push
281+
*/
282+
std::map<hobject_t, crimson::osd::ObjectContextRef> replica_push_targets;
275283
private:
276284
void handle_backfill_finish(
277285
MOSDPGBackfill& m,

0 commit comments

Comments
 (0)