Skip to content

Commit 0167ddb

Browse files
athanatosathanatos
authored
Merge pull request ceph#55488 from cyx1231st/wip-crimson-load-obc
crimson/osd/osd_operations/client_request: make loading-obc concurrent Reviewed-by: Matan Breizman <mbreizma@redhat.com> Reviewed-by: Samuel Just <sjust@redhat.com>
2 parents 7528bf5 + 4a9ed13 commit 0167ddb

File tree

10 files changed

+266
-130
lines changed

10 files changed

+266
-130
lines changed

‎src/crimson/common/operation.h‎

Lines changed: 106 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -477,12 +477,9 @@ class PipelineExitBarrierI {
477477
/// Waits for exit barrier
478478
virtual std::optional<seastar::future<>> wait() = 0;
479479

480-
/// Releases pipeline resources, after or without waiting
481-
// FIXME: currently, exit() will discard the associated future even if it is
482-
// still unresolved, which is discouraged by seastar.
483-
virtual void exit() = 0;
484-
485-
/// Must ensure that resources are released, likely by calling exit()
480+
/// Releases pipeline resources.
481+
/// If wait() has been called,
482+
/// must release after the wait future is resolved.
486483
virtual ~PipelineExitBarrierI() {}
487484
};
488485

@@ -492,11 +489,6 @@ class PipelineStageIT : public BlockerT<T> {
492489
#ifndef NDEBUG
493490
const core_id_t core = seastar::this_shard_id();
494491
#endif
495-
496-
template <class... Args>
497-
decltype(auto) enter(Args&&... args) {
498-
return static_cast<T*>(this)->enter(std::forward<Args>(args)...);
499-
}
500492
};
501493

502494
class PipelineHandle {
@@ -506,6 +498,57 @@ class PipelineHandle {
506498
return barrier ? barrier->wait() : std::nullopt;
507499
}
508500

501+
template <typename OpT, typename T>
502+
std::optional<seastar::future<>>
503+
do_enter_maybe_sync(
504+
T &stage,
505+
typename T::BlockingEvent::template Trigger<OpT>&& t,
506+
PipelineExitBarrierI::Ref&& moved_barrier) {
507+
assert(!barrier);
508+
if constexpr (!T::is_enter_sync) {
509+
auto fut = t.maybe_record_blocking(stage.enter(t), stage);
510+
return std::move(fut
511+
).then([this, t=std::move(t),
512+
moved_barrier=std::move(moved_barrier)](auto &&barrier_ref) {
513+
// destruct moved_barrier and unlock after entered
514+
assert(!barrier);
515+
barrier = std::move(barrier_ref);
516+
return seastar::now();
517+
});
518+
} else {
519+
auto barrier_ref = stage.enter(t);
520+
// destruct moved_barrier and unlock after entered
521+
barrier = std::move(barrier_ref);
522+
return std::nullopt;
523+
}
524+
}
525+
526+
template <typename OpT, typename T>
527+
std::optional<seastar::future<>>
528+
enter_maybe_sync(T &stage, typename T::BlockingEvent::template Trigger<OpT>&& t) {
529+
assert(stage.core == seastar::this_shard_id());
530+
auto wait_fut = wait_barrier();
531+
auto moved_barrier = std::move(barrier);
532+
barrier.reset();
533+
if (wait_fut.has_value()) {
534+
return wait_fut.value(
535+
).then([this, &stage, t=std::move(t),
536+
moved_barrier=std::move(moved_barrier)]() mutable {
537+
auto ret = do_enter_maybe_sync<OpT, T>(
538+
stage, std::move(t), std::move(moved_barrier));
539+
if constexpr (!T::is_enter_sync) {
540+
return std::move(ret.value());
541+
} else {
542+
assert(ret == std::nullopt);
543+
return seastar::now();
544+
}
545+
});
546+
} else {
547+
return do_enter_maybe_sync<OpT, T>(
548+
stage, std::move(t), std::move(moved_barrier));
549+
}
550+
}
551+
509552
public:
510553
PipelineHandle() = default;
511554

@@ -523,36 +566,44 @@ class PipelineHandle {
523566
template <typename OpT, typename T>
524567
seastar::future<>
525568
enter(T &stage, typename T::BlockingEvent::template Trigger<OpT>&& t) {
526-
assert(stage.core == seastar::this_shard_id());
527-
auto wait_fut = wait_barrier();
528-
if (wait_fut.has_value()) {
529-
return wait_fut.value().then([this, &stage, t=std::move(t)] () mutable {
530-
auto fut = t.maybe_record_blocking(stage.enter(t), stage);
531-
return std::move(fut).then(
532-
[this, t=std::move(t)](auto &&barrier_ref) mutable {
533-
exit();
534-
barrier = std::move(barrier_ref);
535-
return seastar::now();
536-
});
537-
});
569+
auto ret = enter_maybe_sync<OpT, T>(stage, std::move(t));
570+
if (ret.has_value()) {
571+
return std::move(ret.value());
538572
} else {
539-
auto fut = t.maybe_record_blocking(stage.enter(t), stage);
540-
return std::move(fut).then(
541-
[this, t=std::move(t)](auto &&barrier_ref) mutable {
542-
exit();
543-
barrier = std::move(barrier_ref);
544-
return seastar::now();
545-
});
573+
return seastar::now();
546574
}
547575
}
548576

577+
/**
578+
* Synchronously leaves the previous stage and enters the next stage.
579+
* Required for the use case which needs ordering upon entering an
580+
* ordered concurrent phase.
581+
*/
582+
template <typename OpT, typename T>
583+
void
584+
enter_sync(T &stage, typename T::BlockingEvent::template Trigger<OpT>&& t) {
585+
static_assert(T::is_enter_sync);
586+
auto ret = enter_maybe_sync<OpT, T>(stage, std::move(t));
587+
// Expect that barrier->wait() (leaving the previous stage)
588+
// also returns nullopt, see enter_maybe_sync() above
589+
ceph_assert(!ret.has_value());
590+
}
591+
549592
/**
550593
* Completes pending exit barrier without entering a new one.
551594
*/
552595
seastar::future<> complete() {
553596
auto ret = wait_barrier();
597+
auto moved_barrier = std::move(barrier);
554598
barrier.reset();
555-
return ret ? std::move(ret.value()) : seastar::now();
599+
if (ret) {
600+
return std::move(ret.value()
601+
).then([moved_barrier=std::move(moved_barrier)] {
602+
// destruct moved_barrier and unlock after wait()
603+
});
604+
} else {
605+
return seastar::now();
606+
}
556607
}
557608

558609
/**
@@ -592,16 +643,10 @@ class OrderedExclusivePhaseT : public PipelineStageIT<T> {
592643
return std::nullopt;
593644
}
594645

595-
void exit() final {
596-
if (phase) {
597-
assert(phase->core == seastar::this_shard_id());
598-
phase->exit(op_id);
599-
phase = nullptr;
600-
}
601-
}
602-
603646
~ExitBarrier() final {
604-
exit();
647+
assert(phase);
648+
assert(phase->core == seastar::this_shard_id());
649+
phase->exit(op_id);
605650
}
606651
};
607652

@@ -611,6 +656,8 @@ class OrderedExclusivePhaseT : public PipelineStageIT<T> {
611656
}
612657

613658
public:
659+
static constexpr bool is_enter_sync = false;
660+
614661
template <class TriggerT>
615662
seastar::future<PipelineExitBarrierI::Ref> enter(TriggerT& t) {
616663
waiting++;
@@ -690,33 +737,32 @@ class OrderedConcurrentPhaseT : public PipelineStageIT<T> {
690737
return trigger.maybe_record_exit_barrier(std::move(ret));
691738
}
692739

693-
void exit() final {
740+
~ExitBarrier() final {
741+
assert(phase);
742+
assert(phase->core == seastar::this_shard_id());
694743
if (barrier) {
695-
assert(phase);
696-
assert(phase->core == seastar::this_shard_id());
744+
// wait() hasn't been called
745+
746+
// FIXME: should not discard future,
747+
// it's discouraged by seastar and may cause shutdown issues.
697748
std::ignore = std::move(*barrier
698749
).then([phase=this->phase] {
699750
phase->mutex.unlock();
700751
});
701-
barrier = std::nullopt;
702-
phase = nullptr;
703-
} else if (phase) {
704-
assert(phase->core == seastar::this_shard_id());
752+
} else {
753+
// wait() has been called, must unlock
754+
// after the wait() future is resolved.
705755
phase->mutex.unlock();
706-
phase = nullptr;
707756
}
708757
}
709-
710-
~ExitBarrier() final {
711-
exit();
712-
}
713758
};
714759

715760
public:
761+
static constexpr bool is_enter_sync = true;
762+
716763
template <class TriggerT>
717-
seastar::future<PipelineExitBarrierI::Ref> enter(TriggerT& t) {
718-
return seastar::make_ready_future<PipelineExitBarrierI::Ref>(
719-
new ExitBarrier<TriggerT>{*this, mutex.lock(), t});
764+
PipelineExitBarrierI::Ref enter(TriggerT& t) {
765+
return std::make_unique<ExitBarrier<TriggerT>>(*this, mutex.lock(), t);
720766
}
721767

722768
private:
@@ -740,16 +786,15 @@ class UnorderedStageT : public PipelineStageIT<T> {
740786
return std::nullopt;
741787
}
742788

743-
void exit() final {}
744-
745789
~ExitBarrier() final {}
746790
};
747791

748792
public:
749-
template <class... IgnoreArgs>
750-
seastar::future<PipelineExitBarrierI::Ref> enter(IgnoreArgs&&...) {
751-
return seastar::make_ready_future<PipelineExitBarrierI::Ref>(
752-
new ExitBarrier);
793+
static constexpr bool is_enter_sync = true;
794+
795+
template <class TriggerT>
796+
PipelineExitBarrierI::Ref enter(TriggerT&) {
797+
return std::make_unique<ExitBarrier>();
753798
}
754799
};
755800

‎src/crimson/osd/osd_operation.h‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,11 +153,15 @@ class TrackableOperationT : public OperationT<T> {
153153
get_event<EventT>().trigger(*that(), std::forward<Args>(args)...);
154154
}
155155

156+
template <class BlockingEventT>
157+
typename BlockingEventT::template Trigger<T>
158+
get_trigger() {
159+
return {get_event<BlockingEventT>(), *that()};
160+
}
161+
156162
template <class BlockingEventT, class InterruptorT=void, class F>
157163
auto with_blocking_event(F&& f) {
158-
auto ret = std::forward<F>(f)(typename BlockingEventT::template Trigger<T>{
159-
get_event<BlockingEventT>(), *that()
160-
});
164+
auto ret = std::forward<F>(f)(get_trigger<BlockingEventT>());
161165
if constexpr (std::is_same_v<InterruptorT, void>) {
162166
return ret;
163167
} else {
@@ -196,6 +200,12 @@ class PhasedOperationT : public TrackableOperationT<T> {
196200
});
197201
}
198202

203+
template <class StageT>
204+
void enter_stage_sync(StageT& stage) {
205+
that()->get_handle().template enter_sync<T>(
206+
stage, this->template get_trigger<typename StageT::BlockingEvent>());
207+
}
208+
199209
template <class OpT>
200210
friend class crimson::os::seastore::OperationProxyT;
201211

‎src/crimson/osd/osd_operation_external_tracking.h‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,13 @@ struct LttngBackend
3333
PGActivationBlocker::BlockingEvent::Backend,
3434
scrub::PGScrubber::BlockingEvent::Backend,
3535
ClientRequest::PGPipeline::RecoverMissing::BlockingEvent::Backend,
36+
ClientRequest::PGPipeline::RecoverMissingLockOBC::BlockingEvent::Backend,
37+
ClientRequest::PGPipeline::RecoverMissingLockOBC::
38+
BlockingEvent::ExitBarrierEvent::Backend,
39+
ClientRequest::PGPipeline::RecoverMissingSnaps::BlockingEvent::Backend,
3640
ClientRequest::PGPipeline::GetOBC::BlockingEvent::Backend,
41+
ClientRequest::PGPipeline::LockOBC::BlockingEvent::Backend,
42+
ClientRequest::PGPipeline::LockOBC::BlockingEvent::ExitBarrierEvent::Backend,
3743
ClientRequest::PGPipeline::Process::BlockingEvent::Backend,
3844
ClientRequest::PGPipeline::WaitRepop::BlockingEvent::Backend,
3945
ClientRequest::PGPipeline::WaitRepop::BlockingEvent::ExitBarrierEvent::Backend,
@@ -103,11 +109,35 @@ struct LttngBackend
103109
const ClientRequest::PGPipeline::RecoverMissing& blocker) override {
104110
}
105111

112+
void handle(ClientRequest::PGPipeline::RecoverMissingLockOBC::BlockingEvent& ev,
113+
const Operation& op,
114+
const ClientRequest::PGPipeline::RecoverMissingLockOBC& blocker) override {
115+
}
116+
117+
void handle(ClientRequest::PGPipeline::RecoverMissingLockOBC::
118+
BlockingEvent::ExitBarrierEvent& ev,
119+
const Operation& op) override {
120+
}
121+
122+
void handle(ClientRequest::PGPipeline::RecoverMissingSnaps::BlockingEvent& ev,
123+
const Operation& op,
124+
const ClientRequest::PGPipeline::RecoverMissingSnaps& blocker) override {
125+
}
126+
106127
void handle(ClientRequest::PGPipeline::GetOBC::BlockingEvent& ev,
107128
const Operation& op,
108129
const ClientRequest::PGPipeline::GetOBC& blocker) override {
109130
}
110131

132+
void handle(ClientRequest::PGPipeline::LockOBC::BlockingEvent& ev,
133+
const Operation& op,
134+
const ClientRequest::PGPipeline::LockOBC& blocker) override {
135+
}
136+
137+
void handle(ClientRequest::PGPipeline::LockOBC::BlockingEvent::ExitBarrierEvent& ev,
138+
const Operation& op) override {
139+
}
140+
111141
void handle(ClientRequest::PGPipeline::Process::BlockingEvent& ev,
112142
const Operation& op,
113143
const ClientRequest::PGPipeline::Process& blocker) override {
@@ -145,7 +175,13 @@ struct HistoricBackend
145175
PGActivationBlocker::BlockingEvent::Backend,
146176
scrub::PGScrubber::BlockingEvent::Backend,
147177
ClientRequest::PGPipeline::RecoverMissing::BlockingEvent::Backend,
178+
ClientRequest::PGPipeline::RecoverMissingLockOBC::BlockingEvent::Backend,
179+
ClientRequest::PGPipeline::RecoverMissingLockOBC::
180+
BlockingEvent::ExitBarrierEvent::Backend,
181+
ClientRequest::PGPipeline::RecoverMissingSnaps::BlockingEvent::Backend,
148182
ClientRequest::PGPipeline::GetOBC::BlockingEvent::Backend,
183+
ClientRequest::PGPipeline::LockOBC::BlockingEvent::Backend,
184+
ClientRequest::PGPipeline::LockOBC::BlockingEvent::ExitBarrierEvent::Backend,
149185
ClientRequest::PGPipeline::Process::BlockingEvent::Backend,
150186
ClientRequest::PGPipeline::WaitRepop::BlockingEvent::Backend,
151187
ClientRequest::PGPipeline::WaitRepop::BlockingEvent::ExitBarrierEvent::Backend,
@@ -215,11 +251,35 @@ struct HistoricBackend
215251
const ClientRequest::PGPipeline::RecoverMissing& blocker) override {
216252
}
217253

254+
void handle(ClientRequest::PGPipeline::RecoverMissingLockOBC::BlockingEvent& ev,
255+
const Operation& op,
256+
const ClientRequest::PGPipeline::RecoverMissingLockOBC& blocker) override {
257+
}
258+
259+
void handle(ClientRequest::PGPipeline::RecoverMissingLockOBC::
260+
BlockingEvent::ExitBarrierEvent& ev,
261+
const Operation& op) override {
262+
}
263+
264+
void handle(ClientRequest::PGPipeline::RecoverMissingSnaps::BlockingEvent& ev,
265+
const Operation& op,
266+
const ClientRequest::PGPipeline::RecoverMissingSnaps& blocker) override {
267+
}
268+
218269
void handle(ClientRequest::PGPipeline::GetOBC::BlockingEvent& ev,
219270
const Operation& op,
220271
const ClientRequest::PGPipeline::GetOBC& blocker) override {
221272
}
222273

274+
void handle(ClientRequest::PGPipeline::LockOBC::BlockingEvent& ev,
275+
const Operation& op,
276+
const ClientRequest::PGPipeline::LockOBC& blocker) override {
277+
}
278+
279+
void handle(ClientRequest::PGPipeline::LockOBC::BlockingEvent::ExitBarrierEvent& ev,
280+
const Operation& op) override {
281+
}
282+
223283
void handle(ClientRequest::PGPipeline::Process::BlockingEvent& ev,
224284
const Operation& op,
225285
const ClientRequest::PGPipeline::Process& blocker) override {

0 commit comments

Comments
 (0)