Skip to content

Commit f99746a

Browse files
committed
crimson: use make_interruptible, coroutine, and RAII releaser for recover_object_with_throttle
791772f used with_throttle here in a way which caused then_interruptible in PGRecovery::recover_object to be called outside of an interruptible context. Instead of using a wrapper taking a lambda, rephrase as an RAII releaser suitable for use in a coroutine. This avoids needing to structure with_throttle to deal correctly with both interruptible and non-interruptible contexts. Fixes: https://tracker.ceph.com/issues/70939 Signed-off-by: Samuel Just <[email protected]>
1 parent b3e55c4 commit f99746a

File tree

4 files changed

+39
-13
lines changed

4 files changed

+39
-13
lines changed

src/crimson/osd/osd_operation.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,30 @@ class OperationThrottler : public BlockerT<OperationThrottler>,
310310
return !max_in_progress || in_progress < max_in_progress;
311311
}
312312

313+
class ThrottleReleaser {
314+
OperationThrottler *parent = nullptr;
315+
public:
316+
ThrottleReleaser(OperationThrottler *parent) : parent(parent) {}
317+
ThrottleReleaser(const ThrottleReleaser &) = delete;
318+
ThrottleReleaser(ThrottleReleaser &&rhs) noexcept {
319+
std::swap(parent, rhs.parent);
320+
}
321+
322+
~ThrottleReleaser() {
323+
if (parent) {
324+
parent->release_throttle();
325+
}
326+
}
327+
};
328+
329+
auto get_throttle(crimson::osd::scheduler::params_t params) {
330+
return acquire_throttle(
331+
params
332+
).then([this] {
333+
return ThrottleReleaser{this};
334+
});
335+
}
336+
313337
template <typename F>
314338
auto with_throttle(
315339
crimson::osd::scheduler::params_t params,

src/crimson/osd/pg_recovery.cc

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <fmt/ostream.h>
66
#include <fmt/ranges.h>
77

8+
#include "crimson/common/coroutine.h"
89
#include "crimson/common/log.h"
910
#include "crimson/common/type_helpers.h"
1011
#include "crimson/osd/backfill_facades.h"
@@ -525,22 +526,19 @@ void PGRecovery::request_primary_scan(
525526

526527
PGRecovery::interruptible_future<>
527528
PGRecovery::recover_object_with_throttle(
528-
const hobject_t &soid,
529+
hobject_t soid,
529530
eversion_t need)
530531
{
531532
LOG_PREFIX(PGRecovery::recover_object_with_throttle);
532-
crimson::osd::scheduler::params_t params =
533-
{1, 0, crimson::osd::scheduler::scheduler_class_t::background_best_effort};
534-
auto &ss = pg->get_shard_services();
535533
DEBUGDPP("{} {}", pg->get_dpp(), soid, need);
536-
return ss.with_throttle(
537-
std::move(params),
538-
[FNAME, this, soid, need] {
539-
DEBUGDPP("got throttle: {} {}", pg->get_dpp(), soid, need);
540-
auto backend = pg->get_recovery_backend();
541-
assert(backend);
542-
return backend->recover_object(soid, need);
543-
});
534+
auto releaser = co_await interruptor::make_interruptible(
535+
pg->get_shard_services().get_throttle(
536+
crimson::osd::scheduler::params_t{
537+
1, 0, crimson::osd::scheduler::scheduler_class_t::background_best_effort
538+
}));
539+
DEBUGDPP("got throttle: {} {}", pg->get_dpp(), soid, need);
540+
co_await pg->get_recovery_backend()->recover_object(soid, need);
541+
co_return;
544542
}
545543

546544
void PGRecovery::enqueue_push(

src/crimson/osd/pg_recovery.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ class PGBackend;
2525

2626
class PGRecovery : public crimson::osd::BackfillState::BackfillListener {
2727
public:
28+
using interruptor =
29+
::crimson::interruptible::interruptor<
30+
::crimson::osd::IOInterruptCondition>;
2831
template <typename T = void>
2932
using interruptible_future = RecoveryBackend::interruptible_future<T>;
3033
PGRecovery(PGRecoveryListener* pg) : pg(pg) {}
@@ -100,7 +103,7 @@ class PGRecovery : public crimson::osd::BackfillState::BackfillListener {
100103
friend class crimson::osd::UrgentRecovery;
101104

102105
interruptible_future<> recover_object_with_throttle(
103-
const hobject_t &soid,
106+
hobject_t soid,
104107
eversion_t need);
105108

106109
interruptible_future<> recover_object(

src/crimson/osd/shard_services.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,7 @@ class ShardServices : public OSDMapService {
594594

595595
FORWARD_TO_OSD_SINGLETON(get_pool_info)
596596
FORWARD(with_throttle, with_throttle, local_state.throttler)
597+
FORWARD(get_throttle, get_throttle, local_state.throttler)
597598

598599
FORWARD_TO_OSD_SINGLETON(build_incremental_map_msg)
599600
FORWARD_TO_OSD_SINGLETON(send_incremental_map)

0 commit comments

Comments
 (0)