From: Adam C. Emerson Date: Fri, 4 Sep 2020 00:43:47 +0000 (-0400) Subject: cls/fifo: Add 'exclusive' option to Trim X-Git-Tag: v17.0.0~1186^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=dbd2ee5819dc6fa291a9fface93a1edeec9b2345;p=ceph.git cls/fifo: Add 'exclusive' option to Trim To support RGW's MetadataLog, add a flag to FIFO::trim that when true trims up to but not including the given entry. Signed-off-by: Adam C. Emerson --- diff --git a/src/cls/fifo/cls_fifo.cc b/src/cls/fifo/cls_fifo.cc index baa94dc8eb830..4bf0c34e7eb15 100644 --- a/src/cls/fifo/cls_fifo.cc +++ b/src/cls/fifo/cls_fifo.cc @@ -776,6 +776,9 @@ int trim_part(cls_method_context_t hctx, if (op.ofs < part_header.min_ofs) { return 0; } + if (op.exclusive && op.ofs == part_header.min_ofs) { + return 0; + } if (op.ofs >= part_header.next_ofs) { if (full_part(part_header)) { @@ -803,15 +806,19 @@ int trim_part(cls_method_context_t hctx, return r; } - r = reader.get_next_entry(nullptr, nullptr, nullptr); - if (r < 0) { - CLS_ERR("ERROR: %s(): unexpected failure at get_next_entry(): r=%d", - __func__, r); - return r; + if (op.exclusive) { + part_header.min_index = pre_header.index; + } else { + r = reader.get_next_entry(nullptr, nullptr, nullptr); + if (r < 0) { + CLS_ERR("ERROR: %s(): unexpected failure at get_next_entry(): r=%d", + __func__, r); + return r; + } + part_header.min_index = pre_header.index + 1; } part_header.min_ofs = reader.get_ofs(); - part_header.min_index = pre_header.index + 1; } r = write_part_header(hctx, part_header); diff --git a/src/cls/fifo/cls_fifo_ops.h b/src/cls/fifo/cls_fifo_ops.h index 0adae1e5d39ae..a3f4ae237c9c5 100644 --- a/src/cls/fifo/cls_fifo_ops.h +++ b/src/cls/fifo/cls_fifo_ops.h @@ -194,17 +194,20 @@ struct trim_part { std::optional tag; std::uint64_t ofs{0}; + bool exclusive = false; void encode(ceph::buffer::list& bl) const { ENCODE_START(1, 1, bl); encode(tag, bl); encode(ofs, bl); + encode(exclusive, bl); ENCODE_FINISH(bl); } void decode(ceph::buffer::list::const_iterator& bl) { DECODE_START(1, bl); decode(tag, bl); decode(ofs, bl); + decode(exclusive, bl); DECODE_FINISH(bl); } }; diff --git a/src/neorados/cls/fifo.cc b/src/neorados/cls/fifo.cc index 84f20a50e79bb..fa99275b25f96 100644 --- a/src/neorados/cls/fifo.cc +++ b/src/neorados/cls/fifo.cc @@ -143,12 +143,13 @@ void push_part(WriteOp& op, std::string_view tag, void trim_part(WriteOp& op, std::optional tag, - std::uint64_t ofs) + std::uint64_t ofs, bool exclusive) { fifo::op::trim_part tp; tp.tag = tag; tp.ofs = ofs; + tp.exclusive = exclusive; bufferlist in; encode(tp, in); diff --git a/src/neorados/cls/fifo.h b/src/neorados/cls/fifo.h index 3bdf55a0af62a..05865dccace3c 100644 --- a/src/neorados/cls/fifo.h +++ b/src/neorados/cls/fifo.h @@ -112,7 +112,8 @@ void push_part(WriteOp& op, std::string_view tag, std::deque data_bufs, fu2::unique_function); void trim_part(WriteOp& op, std::optional tag, - std::uint64_t ofs); + std::uint64_t ofs, + bool exclusive); void list_part(ReadOp& op, std::optional tag, std::uint64_t ofs, @@ -567,6 +568,8 @@ public: /// Signature: (bs::error_code) template auto trim(std::string_view markstr, //< Position to which to trim, inclusive + bool exclusive, //< If true, trim markers up to but NOT INCLUDING + //< markstr, otherwise trim markstr as well. CT&& ct //< CompletionToken ) { auto m = to_marker(markstr); @@ -582,7 +585,7 @@ public: } else { using handler_type = decltype(init.completion_handler); auto t = ceph::allocate_unique>( - a, this, m->num, m->ofs, std::move(init.completion_handler)); + a, this, m->num, m->ofs, exclusive, std::move(init.completion_handler)); t.release()->trim(); } return init.result.get(); @@ -1088,9 +1091,10 @@ private: auto trim_part(int64_t part_num, uint64_t ofs, std::optional tag, + bool exclusive, CT&& ct) { WriteOp op; - cls::fifo::trim_part(op, tag, ofs); + cls::fifo::trim_part(op, tag, ofs, exclusive); return r->execute(info.part_oid(part_num), ioc, std::move(op), std::forward(ct)); } @@ -1362,6 +1366,7 @@ private: FIFO* f; std::int64_t part_num; std::uint64_t ofs; + bool exclusive; Handler handler; std::int64_t pn; int i = 0; @@ -1411,8 +1416,9 @@ private: public: Trimmer(FIFO* f, std::int64_t part_num, std::uint64_t ofs, - Handler&& handler) - : f(f), part_num(part_num), ofs(ofs), handler(std::move(handler)) { + bool exclusive, Handler&& handler) + : f(f), part_num(part_num), ofs(ofs), exclusive(exclusive), + handler(std::move(handler)) { std::unique_lock l(f->m); pn = f->info.tail_part_num; } @@ -1426,6 +1432,7 @@ private: l.unlock(); f->trim_part( pn, max_part_size, std::nullopt, + false, ca::bind_ea( e, a, [t = std::unique_ptr(this), @@ -1444,7 +1451,7 @@ private: return; } f->trim_part( - part_num, ofs, std::nullopt, + part_num, ofs, std::nullopt, exclusive, ca::bind_ea( e, a, [t = std::unique_ptr(this), diff --git a/src/rgw/cls_fifo_legacy.cc b/src/rgw/cls_fifo_legacy.cc index d44f52cf15e25..5b6015623a1f5 100644 --- a/src/rgw/cls_fifo_legacy.cc +++ b/src/rgw/cls_fifo_legacy.cc @@ -152,12 +152,13 @@ int push_part(lr::IoCtx& ioctx, const std::string& oid, std::string_view tag, void trim_part(lr::ObjectWriteOperation* op, std::optional tag, - std::uint64_t ofs) + std::uint64_t ofs, bool exclusive) { fifo::op::trim_part tp; tp.tag = tag; tp.ofs = ofs; + tp.exclusive = exclusive; cb::list in; encode(tp, in); @@ -621,25 +622,27 @@ int FIFO::push_entries(const std::deque& data_bufs, int FIFO::trim_part(int64_t part_num, uint64_t ofs, std::optional tag, + bool exclusive, optional_yield y) { lr::ObjectWriteOperation op; std::unique_lock l(m); const auto part_oid = info.part_oid(part_num); l.unlock(); - rgw::cls::fifo::trim_part(&op, tag, ofs); + rgw::cls::fifo::trim_part(&op, tag, ofs, exclusive); return rgw_rados_operate(ioctx, part_oid, &op, y); } int FIFO::trim_part(int64_t part_num, uint64_t ofs, std::optional tag, + bool exclusive, lr::AioCompletion* c) { lr::ObjectWriteOperation op; std::unique_lock l(m); const auto part_oid = info.part_oid(part_num); l.unlock(); - rgw::cls::fifo::trim_part(&op, tag, ofs); + rgw::cls::fifo::trim_part(&op, tag, ofs, exclusive); return ioctx.aio_operate(part_oid, c, &op); } @@ -924,7 +927,7 @@ int FIFO::list(int max_entries, return r; } -int FIFO::trim(std::string_view markstr, optional_yield y) +int FIFO::trim(std::string_view markstr, bool exclusive, optional_yield y) { auto marker = to_marker(markstr); if (!marker) { @@ -941,13 +944,13 @@ int FIFO::trim(std::string_view markstr, optional_yield y) std::unique_lock l(m); auto max_part_size = info.params.max_part_size; l.unlock(); - r = trim_part(pn, max_part_size, std::nullopt, y); + r = trim_part(pn, max_part_size, std::nullopt, false, y); if (r < 0 && r == -ENOENT) { return r; } ++pn; } - r = trim_part(part_num, ofs, std::nullopt, y); + r = trim_part(part_num, ofs, std::nullopt, exclusive, y); if (r < 0 && r != -ENOENT) { return r; } @@ -982,6 +985,7 @@ struct Trimmer { std::int64_t part_num; std::uint64_t ofs; std::int64_t pn; + bool exclusive; lr::AioCompletion* super; lr::AioCompletion* cur = lr::Rados::aio_create_completion( static_cast(this), &FIFO::trim_callback); @@ -990,8 +994,9 @@ struct Trimmer { int retries = 0; Trimmer(FIFO* fifo, std::int64_t part_num, std::uint64_t ofs, std::int64_t pn, - lr::AioCompletion* super) - : fifo(fifo), part_num(part_num), ofs(ofs), pn(pn), super(super) { + bool exclusive, lr::AioCompletion* super) + : fifo(fifo), part_num(part_num), ofs(ofs), pn(pn), exclusive(exclusive), + super(super) { super->pc->get(); } ~Trimmer() { @@ -1019,7 +1024,7 @@ void FIFO::trim_callback(lr::completion_t, void* arg) trimmer->cur->release(); trimmer->cur = lr::Rados::aio_create_completion(arg, &FIFO::trim_callback); r = trimmer->fifo->trim_part(trimmer->pn++, max_part_size, std::nullopt, - trimmer->cur); + false, trimmer->cur); if (r < 0) { complete(trimmer->super, r); delete trimmer; @@ -1033,7 +1038,7 @@ void FIFO::trim_callback(lr::completion_t, void* arg) trimmer->update = true; trimmer->canceled = tail_part_num < trimmer->part_num; r = trimmer->fifo->trim_part(trimmer->part_num, trimmer->ofs, - std::nullopt, trimmer->cur); + std::nullopt, trimmer->exclusive, trimmer->cur); if (r < 0) { complete(trimmer->super, r); delete trimmer; @@ -1069,7 +1074,7 @@ void FIFO::trim_callback(lr::completion_t, void* arg) } } -int FIFO::trim(std::string_view markstr, lr::AioCompletion* c) { +int FIFO::trim(std::string_view markstr, bool exclusive, lr::AioCompletion* c) { auto marker = to_marker(markstr); if (!marker) { return -EINVAL; @@ -1079,7 +1084,7 @@ int FIFO::trim(std::string_view markstr, lr::AioCompletion* c) { const auto pn = info.tail_part_num; const auto part_oid = info.part_oid(pn); l.unlock(); - auto trimmer = new Trimmer(this, marker->num, marker->ofs, pn, c); + auto trimmer = new Trimmer(this, marker->num, marker->ofs, pn, exclusive, c); ++trimmer->pn; auto ofs = marker->ofs; if (pn < marker->num) { @@ -1087,7 +1092,8 @@ int FIFO::trim(std::string_view markstr, lr::AioCompletion* c) { } else { trimmer->update = true; } - auto r = trimmer->fifo->trim_part(pn, ofs, std::nullopt, trimmer->cur); + auto r = trimmer->fifo->trim_part(pn, ofs, std::nullopt, exclusive, + trimmer->cur); if (r < 0) { complete(trimmer->super, r); delete trimmer; diff --git a/src/rgw/cls_fifo_legacy.h b/src/rgw/cls_fifo_legacy.h index 19c024719b018..734e05112ff24 100644 --- a/src/rgw/cls_fifo_legacy.h +++ b/src/rgw/cls_fifo_legacy.h @@ -62,7 +62,8 @@ void part_init(lr::ObjectWriteOperation* op, std::string_view tag, int push_part(lr::IoCtx& ioctx, const std::string& oid, std::string_view tag, std::deque data_bufs, optional_yield y); void trim_part(lr::ObjectWriteOperation* op, - std::optional tag, std::uint64_t ofs); + std::optional tag, std::uint64_t ofs, + bool exclusive); int list_part(lr::IoCtx& ioctx, const std::string& oid, std::optional tag, std::uint64_t ofs, std::uint64_t max_entries, @@ -148,9 +149,11 @@ class FIFO { int push_entries(const std::deque& data_bufs, optional_yield y); int trim_part(int64_t part_num, uint64_t ofs, - std::optional tag, optional_yield y); + std::optional tag, bool exclusive, + optional_yield y); int trim_part(int64_t part_num, uint64_t ofs, - std::optional tag, lr::AioCompletion* c); + std::optional tag, bool exclusive, + lr::AioCompletion* c); static void trim_callback(lr::completion_t, void* arg); static void update_callback(lr::completion_t, void* arg); @@ -214,10 +217,14 @@ public: ); /// Trim entries, coroutine/block style int trim(std::string_view markstr, //< Position to which to trim, inclusive + bool exclusive, //< If true, do not trim the target entry + //< itself, just all those before it. optional_yield y //< Optional yield ); /// Trim entries, librados AioCompletion style int trim(std::string_view markstr, //< Position to which to trim, inclusive + bool exclusive, //< If true, do not trim the target entry + //< itself, just all those before it. lr::AioCompletion* c //< librados AIO Completion ); /// Get part info diff --git a/src/rgw/rgw_datalog.cc b/src/rgw/rgw_datalog.cc index 33edaf7271b58..b494d53e9983c 100644 --- a/src/rgw/rgw_datalog.cc +++ b/src/rgw/rgw_datalog.cc @@ -430,7 +430,7 @@ public: return 0; } int trim(int index, std::string_view marker) override { - auto r = fifos[index]->trim(marker, null_yield); + auto r = fifos[index]->trim(marker, false, null_yield); if (r < 0) { lderr(cct) << __PRETTY_FUNCTION__ << ": unable to trim FIFO: " << get_oid(index) @@ -464,7 +464,7 @@ public: pc->cond.notify_all(); pc->put_unlock(); } else { - r = fifos[index]->trim(marker, c); + r = fifos[index]->trim(marker, false, c); if (r < 0) { lderr(cct) << __PRETTY_FUNCTION__ << ": unable to trim FIFO: " << get_oid(index) diff --git a/src/test/cls_fifo/bench_cls_fifo.cc b/src/test/cls_fifo/bench_cls_fifo.cc index b990fbedfc865..df390cd317125 100644 --- a/src/test/cls_fifo/bench_cls_fifo.cc +++ b/src/test/cls_fifo/bench_cls_fifo.cc @@ -105,7 +105,7 @@ benchmark pull(RCf::FIFO& f, const std::uint32_t count, break; got += result.size(); remaining -= result.size(); - f.trim(result.back().marker, y); + f.trim(result.back().marker, false, y); } auto finish = sc::steady_clock::now(); return benchmark(got, (finish - start)); @@ -140,7 +140,7 @@ void concurpull(const std::string& oid, const std::int64_t pool, got += result.size(); remaining -= result.size(); if (*exit_early) break; - f->trim(result.back().marker, y); + f->trim(result.back().marker, false, y); } auto finish = sc::steady_clock::now(); bench.entries = got; diff --git a/src/test/cls_fifo/test_cls_fifo.cc b/src/test/cls_fifo/test_cls_fifo.cc index 23f106c49caef..484248c0c81ca 100644 --- a/src/test/cls_fifo/test_cls_fifo.cc +++ b/src/test/cls_fifo/test_cls_fifo.cc @@ -297,7 +297,7 @@ TEST(FIFO, TestPushListTrim) { /* trim one entry */ - f->trim(markers[min_entry], y); + f->trim(markers[min_entry], false, y); ++min_entry; } @@ -444,7 +444,7 @@ TEST(FIFO, TestMultipleParts) { marker = result.front().marker; - f->trim(*marker, y); + f->trim(*marker, false, y); } /* check tail */ @@ -606,7 +606,7 @@ TEST(FIFO, TestTwoPushersTrim) { auto& entry = result[num - 1]; marker = entry.marker; - f1->trim(marker, y); + f1->trim(marker, false, y); /* list what's left by fifo2 */ @@ -686,3 +686,54 @@ TEST(FIFO, TestPushBatch) { }); c.run(); } + +TEST(FIFO, TestTrimExclusive) { + ba::io_context c; + auto fifo_id = "fifo"sv; + + s::spawn(c, [&](s::yield_context y) mutable { + auto r = R::RADOS::Builder{}.build(c, y); + auto pool = create_pool(r, get_temp_pool_name(), y); + auto sg = make_scope_guard( + [&] { + r.delete_pool(pool, y); + }); + R::IOContext ioc(pool); + auto f = RCf::FIFO::create(r, ioc, fifo_id, y); + static constexpr auto max_entries = 10u; + for (uint32_t i = 0; i < max_entries; ++i) { + cb::list bl; + encode(i, bl); + f->push(bl, y); + } + + { + auto [result, more] = f->list(1, std::nullopt, y); + auto [val, marker] = + decode_entry(result.front()); + ASSERT_EQ(0, val); + f->trim(marker, true, y); + } + { + auto [result, more] = f->list(max_entries, std::nullopt, y); + auto [val, marker] = decode_entry(result.front()); + ASSERT_EQ(0, val); + f->trim(result[4].marker, true, y); + } + { + auto [result, more] = f->list(max_entries, std::nullopt, y); + auto [val, marker] = + decode_entry(result.front()); + ASSERT_EQ(4, val); + f->trim(result.back().marker, true, y); + } + { + auto [result, more] = f->list(max_entries, std::nullopt, y); + auto [val, marker] = + decode_entry(result.front()); + ASSERT_EQ(result.size(), 1); + ASSERT_EQ(max_entries - 1, val); + } + }); + c.run(); +} diff --git a/src/test/rgw/test_cls_fifo_legacy.cc b/src/test/rgw/test_cls_fifo_legacy.cc index f8808b87de8e4..17e78e357f6e3 100644 --- a/src/test/rgw/test_cls_fifo_legacy.cc +++ b/src/test/rgw/test_cls_fifo_legacy.cc @@ -220,7 +220,7 @@ TEST_F(LegacyFIFO, TestPushListTrim) } /* trim one entry */ - r = f->trim(markers[min_entry], null_yield); + r = f->trim(markers[min_entry], false, null_yield); ASSERT_EQ(0, r); ++min_entry; @@ -333,7 +333,7 @@ TEST_F(LegacyFIFO, TestMultipleParts) ASSERT_EQ(expected_more, more); marker = result.front().marker; - r = f->trim(*marker, null_yield); + r = f->trim(*marker, false, null_yield); ASSERT_EQ(0, r); /* check tail */ @@ -459,7 +459,7 @@ TEST_F(LegacyFIFO, TestTwoPushersTrim) auto& entry = result[num - 1]; marker = entry.marker; - r = f1->trim(marker, null_yield); + r = f1->trim(marker, false, null_yield); /* list what's left by fifo2 */ const auto left = max_entries - num; @@ -577,7 +577,7 @@ TEST_F(LegacyFIFO, TestAioTrim) marker = result.front().marker; std::unique_ptr c(rados.aio_create_completion(nullptr, nullptr)); - r = f->trim(*marker, c.get()); + r = f->trim(*marker, false, c.get()); ASSERT_EQ(0, r); c->wait_for_complete(); r = c->get_return_value(); @@ -607,3 +607,41 @@ TEST_F(LegacyFIFO, TestAioTrim) r = f->get_part_info(info.tail_part_num, &partinfo, null_yield); ASSERT_EQ(0, r); } + +TEST_F(LegacyFIFO, TestTrimExclusive) { + std::unique_ptr f; + auto r = RCf::FIFO::create(ioctx, fifo_id, &f, null_yield); + ASSERT_EQ(0, r); + std::vector result; + bool more = false; + + static constexpr auto max_entries = 10u; + for (uint32_t i = 0; i < max_entries; ++i) { + cb::list bl; + encode(i, bl); + f->push(bl, null_yield); + } + + f->list(1, std::nullopt, &result, &more, null_yield); + auto [val, marker] = decode_entry(result.front()); + ASSERT_EQ(0, val); + f->trim(marker, true, null_yield); + + result.clear(); + f->list(max_entries, std::nullopt, &result, &more, null_yield); + std::tie(val, marker) = decode_entry(result.front()); + ASSERT_EQ(0, val); + f->trim(result[4].marker, true, null_yield); + + result.clear(); + f->list(max_entries, std::nullopt, &result, &more, null_yield); + std::tie(val, marker) = decode_entry(result.front()); + ASSERT_EQ(4, val); + f->trim(result.back().marker, true, null_yield); + + result.clear(); + f->list(max_entries, std::nullopt, &result, &more, null_yield); + std::tie(val, marker) = decode_entry(result.front()); + ASSERT_EQ(result.size(), 1); + ASSERT_EQ(max_entries - 1, val); +}