Skip to content

Commit f90af12

Browse files
committed
crimson/osd/osd_operations/client_request: check already complete in the
"check_already_complete_get_obc" phase Otherwise, the replies can go out-of-order Signed-off-by: Xuehan Xu <[email protected]>
1 parent 943e902 commit f90af12

File tree

4 files changed

+27
-6
lines changed

4 files changed

+27
-6
lines changed

src/crimson/osd/osd_operation_external_tracking.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ struct LttngBackend
3535
ClientRequest::PGPipeline::RecoverMissing::BlockingEvent::Backend,
3636
ClientRequest::PGPipeline::RecoverMissing::
3737
BlockingEvent::ExitBarrierEvent::Backend,
38+
ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent::Backend,
3839
ClientRequest::PGPipeline::GetOBC::BlockingEvent::Backend,
3940
ClientRequest::PGPipeline::LockOBC::BlockingEvent::Backend,
4041
ClientRequest::PGPipeline::LockOBC::BlockingEvent::ExitBarrierEvent::Backend,
@@ -111,6 +112,11 @@ struct LttngBackend
111112
const Operation& op) override {
112113
}
113114

115+
void handle(ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent& ev,
116+
const Operation& op,
117+
const ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc& blocker) override {
118+
}
119+
114120
void handle(ClientRequest::PGPipeline::GetOBC::BlockingEvent& ev,
115121
const Operation& op,
116122
const ClientRequest::PGPipeline::GetOBC& blocker) override {
@@ -164,6 +170,7 @@ struct HistoricBackend
164170
ClientRequest::PGPipeline::RecoverMissing::BlockingEvent::Backend,
165171
ClientRequest::PGPipeline::RecoverMissing::
166172
BlockingEvent::ExitBarrierEvent::Backend,
173+
ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent::Backend,
167174
ClientRequest::PGPipeline::GetOBC::BlockingEvent::Backend,
168175
ClientRequest::PGPipeline::LockOBC::BlockingEvent::Backend,
169176
ClientRequest::PGPipeline::LockOBC::BlockingEvent::ExitBarrierEvent::Backend,
@@ -240,6 +247,11 @@ struct HistoricBackend
240247
const Operation& op) override {
241248
}
242249

250+
void handle(ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent& ev,
251+
const Operation& op,
252+
const ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc& blocker) override {
253+
}
254+
243255
void handle(ClientRequest::PGPipeline::GetOBC::BlockingEvent& ev,
244256
const Operation& op,
245257
const ClientRequest::PGPipeline::GetOBC& blocker) override {

src/crimson/osd/osd_operations/client_request.cc

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,6 @@ ClientRequest::process_op(
337337

338338
std::set<snapid_t> snaps = snaps_need_to_recover();
339339
if (!snaps.empty()) {
340-
// call with_obc() in order, but wait concurrently for loading.
341340
auto with_obc = pg->obc_loader.with_obc<RWState::RWREAD>(
342341
m->get_hobj().get_head(),
343342
[&snaps, &ihref, pg, this](auto head, auto) {
@@ -350,6 +349,16 @@ ClientRequest::process_op(
350349
}
351350
}
352351

352+
/**
353+
* The previous stage of recover_missing is a concurrent phase.
354+
* Checking for already_complete requests must done exclusively.
355+
* Since get_obc is also an exclusive stage, we can merge both stages into
356+
* a single stage and avoid stage switching overhead.
357+
*/
358+
DEBUGDPP("{}.{}: entering check_already_complete_get_obc",
359+
*pg, *this, this_instance_id);
360+
co_await ihref.enter_stage<interruptor>(
361+
client_pp(*pg).check_already_complete_get_obc, *this);
353362
DEBUGDPP("{}.{}: checking already_complete",
354363
*pg, *this, this_instance_id);
355364
auto completed = co_await pg->already_complete(m->get_reqid());
@@ -368,11 +377,7 @@ ClientRequest::process_op(
368377
co_return;
369378
}
370379

371-
DEBUGDPP("{}.{}: not completed, entering get_obc stage",
372-
*pg, *this, this_instance_id);
373-
co_await ihref.enter_stage<interruptor>(client_pp(*pg).get_obc, *this);
374-
375-
DEBUGDPP("{}.{}: entered get_obc stage, about to wait_scrub",
380+
DEBUGDPP("{}.{}: not completed, about to wait_scrub",
376381
*pg, *this, this_instance_id);
377382
co_await ihref.enter_blocker(
378383
*this, pg->scrubber, &decltype(pg->scrubber)::wait_scrub,

src/crimson/osd/osd_operations/client_request.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ class ClientRequest final : public PhasedOperationT<ClientRequest>,
103103
PGActivationBlocker::BlockingEvent,
104104
PGPipeline::RecoverMissing::BlockingEvent,
105105
scrub::PGScrubber::BlockingEvent,
106+
PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent,
106107
PGPipeline::GetOBC::BlockingEvent,
107108
PGPipeline::LockOBC::BlockingEvent,
108109
PGPipeline::Process::BlockingEvent,

src/crimson/osd/osd_operations/common/pg_pipeline.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ class CommonPGPipeline {
2020
struct RecoverMissing : OrderedConcurrentPhaseT<RecoverMissing> {
2121
static constexpr auto type_name = "CommonPGPipeline::recover_missing";
2222
} recover_missing;
23+
struct CheckAlreadyCompleteGetObc : OrderedExclusivePhaseT<CheckAlreadyCompleteGetObc> {
24+
static constexpr auto type_name = "CommonPGPipeline::check_already_complete_get_obc";
25+
} check_already_complete_get_obc;
2326
struct GetOBC : OrderedExclusivePhaseT<GetOBC> {
2427
static constexpr auto type_name = "CommonPGPipeline::get_obc";
2528
} get_obc;

0 commit comments

Comments
 (0)