rgw: Avoid spamming OSD requests on startup by adamemerson · Pull Request #38230 · ceph/ceph

adamemerson · 2020-11-22T04:23:26Z

Currently we send multiple OSD requests on startup to initialize the datalog. This is too many. To reduce this we mark a log (the set of shards) as usable with an omap entry on shard zero and take this as true unless something conflicts with it.

Secondly, initialize FIFOs lazily, so if we aren't doing anything relevant to the data log, don't pay to set them up.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

Provide quick and lengthy checks for the backing store of sharded logs, including a marker on shard 0 to speed things up. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

Also don't use svc_cls. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

LazyFIFO opens the FIFO on first access. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

Because everybody's sick of not having a container for non-default-constructible, immovable objects. Or at least I am. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

That way we don't start sending ops to open a FIFO until we need it. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

To manually complete an asynchronous librados call. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

mattbenjamin

don't know if I can legitimately approve, but it looks sane

mattbenjamin · 2020-12-01T20:19:10Z

src/rgw/rgw_log_backing.cc

+  int r_out = 0;
+  op.omap_get_vals_by_keys({ logmark_omap_key }, &values, &r_out);
+
+  auto oid = get_oid(0);


get_oid(0)?

We keep the 'check' value on the zeroth shard, so after checking all the shards once, we only have to look at one shard after that.

mattbenjamin · 2020-12-01T20:22:07Z

src/rgw/rgw_log_backing.cc

+{
+  auto cct = static_cast<CephContext*>(ioctx.cct());
+  bool omap = false;
+  {


are you doing this just to clear this off the stack after the check?

mattbenjamin · 2020-12-01T20:37:41Z

src/rgw/rgw_datalog.cc


    cls_log_entry e;
-    cls.timelog.prepare_entry(e, ut, {}, key, entry);
+    cls_log_add_prepare_entry(e, utime_t(ut), {}, key, entry);


isn't ut already a utime_t (hence the name)?

No, it's a real_time.

(I could change the name to make that clearer.)

mattbenjamin · 2020-12-01T20:44:27Z

src/rgw/rgw_datalog.cc


 class RGWDataChangesOmap final : public RGWDataChangesBE {
  using centries = std::list<cls_log_entry>;
-  RGWSI_Cls& cls;


just to clarify, is this just to delayer things?

Not exactly. Since RGWDataChangesLog has to have an IoCtx to pass to log_acquire_backing and friends, it seemed silly for the backends to have to make their own instead of it just being passed into them. Especially since svc_cls just created and dropped an IoCtx with every call, just having one and using it seemed preferable.

mattbenjamin · 2020-12-01T21:07:15Z

src/rgw/rgw_datalog.cc

 class RGWDataChangesFIFO final : public RGWDataChangesBE {
  using centries = std::vector<ceph::buffer::list>;
-  std::vector<std::unique_ptr<rgw::cls::fifo::FIFO>> fifos;
+  dynarray<LazyFIFO> fifos;


nice improvement

Thank you. dynarray is something I've been wishing for for a long time to solve the 'dynamically sized array of things containing mutexes' problem.

adamemerson · 2020-12-02T16:26:40Z

http://pulpito.ceph.com/aemerson-2020-12-02_04:12:01-rgw-wip-mark-logbacking-distro-basic-smithi/

mattbenjamin

looks good, good run

smanjara · 2020-12-07T10:22:35Z

Looks good to me.

cbodley · 2021-01-12T16:15:45Z

src/common/dynarray.h

+
+namespace ceph {
+template<typename T, typename Allocator = std::allocator<T>>
+class dynarray {


have you tried using ceph::containers::tiny_vector here? it was added for a similar purpose in #28987

cbodley · 2021-01-12T16:41:38Z

src/rgw/rgw_tools.cc

  ext_mime_map = nullptr;
 }
+
+void rgw_complete_aio_completion(librados::AioCompletion* c, int r) {


it looks like this was mostly copied from CB_AioCompleteAndSafe in AioCompletionImpl.h. i worry that if librados changes, this copy could be missed and lead to bugs. could we use CB_AioCompleteAndSafe directly instead?

librados::CB_AioCompleteAndSafe cb(c->pc); cb(r);

github-actions · 2021-01-25T19:53:15Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

adamemerson added rgw performance labels Nov 22, 2020

adamemerson requested review from cbodley and smanjara November 22, 2020 04:23

github-actions bot added the common label Nov 22, 2020

adamemerson mentioned this pull request Nov 22, 2020

WIP/rgw: Support MetadataLog over FIFO and Omap backends #37251

Closed

adamemerson added the needs-review label Nov 22, 2020

adamemerson force-pushed the wip-mark-logbacking branch 4 times, most recently from 3dbf608 to 323fae2 Compare November 23, 2020 06:26

adamemerson requested a review from ivancich November 25, 2020 03:49

adamemerson force-pushed the wip-mark-logbacking branch 2 times, most recently from 6377844 to 3720f97 Compare November 30, 2020 06:27

github-actions bot added the tests label Nov 30, 2020

adamemerson added 11 commits November 30, 2020 13:59

rgw: Change subsys in cls_fifo_legacy to cut logspam

aa238b3

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

cls/log: Take const references of things you won't modify

6fb231c

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

rgw: Factor out tools to deal with different log backing

ea8a277

Provide quick and lengthy checks for the backing store of sharded logs, including a marker on shard 0 to speed things up. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

rgw: Use refactored log backing tools

7743552

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

rgw/datalog: Pass IoCtx in, don't have each backend make its own

778e72e

Also don't use svc_cls. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

rgw: Move get_oid back to RGWDataChangesLog

1e176e0

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

rgw: Add AioCompletion* versions for the rest of the FIFO methods

7250ca3

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

rgw: Add LazyFIFO to keep from blasting an op-per-shard on startup

62be23f

LazyFIFO opens the FIFO on first access. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

common: Add dynarray, a dynamic array of immovable objects

f3ddd55

Because everybody's sick of not having a container for non-default-constructible, immovable objects. Or at least I am. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

rgw: Use LazyFIFO in data changes log

8c452a5

That way we don't start sending ops to open a FIFO until we need it. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

rgw: Add rgw_complete_aio_completion()

fbb2819

To manually complete an asynchronous librados call. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

adamemerson force-pushed the wip-mark-logbacking branch from 3720f97 to fbb2819 Compare November 30, 2020 19:05

mattbenjamin reviewed Dec 1, 2020

View reviewed changes

mattbenjamin approved these changes Dec 4, 2020

View reviewed changes

cbodley reviewed Jan 12, 2021

View reviewed changes

github-actions bot added the needs-rebase label Jan 25, 2021

mattbenjamin mentioned this pull request Jan 25, 2021

rgw multisite: bucket reshard work in progress #39002

Merged

31 tasks

adamemerson closed this Jan 26, 2021

adamemerson deleted the wip-mark-logbacking branch January 26, 2021 19:55

Conversation

adamemerson commented Nov 22, 2020

Uh oh!

mattbenjamin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamemerson commented Dec 2, 2020

Uh oh!

mattbenjamin left a comment

Choose a reason for hiding this comment

Uh oh!

smanjara commented Dec 7, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants