rgw: remove metadata backend from topic by cbodley · Pull Request #55152 · ceph/ceph

cbodley · 2024-01-11T18:59:01Z

based on #54868 - will rebase once that merges. also relates to #54777 which removes metadata backends, but this PR only contains the topic bits

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

src/rgw/driver/rados/topic.cc

kchheda3 · 2024-01-11T20:30:49Z

Looks great.
Thank you so much !

kchheda3 · 2024-01-12T18:29:25Z

src/rgw/driver/rados/topic.cc

+}
+
+
+class MetadataObject : public RGWMetadataObject {


nit:
all these class names should be prefixed with Topic

MetadataObject MetadataHandler MetadataLister

src/rgw/driver/rados/rgw_metadata_lister.h

kchheda3

the persistent queue was not added/deleted on secondary zone, so added the logic inside the metadata handler, as without queue the notifications will not work for persistent topics.
Ideally would have wanted to call addition/deletion function inside the topic:write n topic:remove and remove the call from all other places... this ensures its called from single place as is topic creation and deletion now called at single place for all zones, however add_persistent_topic n remove_persistent_topic need the notification manager to be initialized which is true for all s3 calls, however for radosgw-admin command to remove topics the notification-manager is not turned on and returns error. so added the call inside metadata-handler and not removed the code from other places to add/delete queue

src/rgw/driver/rados/topic.cc

cbodley · 2024-01-18T19:13:35Z

however add_persistent_topic n remove_persistent_topic need the notification manager to be initialized which is true for all s3 calls

it looks easy enough to remove that dependency on the Manager singleton. remove_persistent_topic() already has an overload without it. i'll give that a try

kchheda3 · 2024-01-18T19:20:11Z

however add_persistent_topic n remove_persistent_topic need the notification manager to be initialized which is true for all s3 calls

it looks easy enough to remove that dependency on the Manager singleton. remove_persistent_topic() already has an overload without it. i'll give that a try

yeah it does have a overload without the Manager, but that overload needs rados_ioctx which we do not have in topic::remove, we could create rados_ioctx pointing to notif_pool a and then delete the Manager::remove_persistent_topic . But i thought that was a bit overkill so i thought of adding it to MetadaHandler.

src/rgw/driver/rados/rgw_metadata_lister.h

kchheda3

Tested the code changes with both bucket notification test and multisite test for v2 and all works good.

src/rgw/rgw_admin.cc

src/rgw/rgw_pubsub.cc

github-actions · 2024-01-24T16:18:24Z

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

…n multisite config Signed-off-by: kchheda3 <kchheda3@bloomberg.net>

…ions in multisite config. Signed-off-by: kchheda3 <kchheda3@bloomberg.net>

…een bucket and topics. Signed-off-by: kchheda3 <kchheda3@bloomberg.net>

…ions in multisite config. Signed-off-by: kchheda3 <kchheda3@bloomberg.net>

both locally and in teuthology Signed-off-by: Yuval Lifshitz <ylifshit@redhat.com>

v1 could be enabled only in local tests. teuthology tests would run with v2 Signed-off-by: Yuval Lifshitz <ylifshit@redhat.com>

Signed-off-by: Casey Bodley <cbodley@redhat.com>

make the SiteConfig available to all of RGWRados via svc.site instead of storing it in sal::RadosStore Signed-off-by: Casey Bodley <cbodley@redhat.com>

Signed-off-by: Casey Bodley <cbodley@redhat.com>

RGWPubSub constructor takes SiteConfig instead of zonegroup map replace do_all_zonegroups_support_notification_v2() with a generic function rgw::all_zonegroups_support() that handles non-realm configurations too remove unused sal::ZoneGroup::supports_feature() Signed-off-by: Casey Bodley <cbodley@redhat.com>

Signed-off-by: Casey Bodley <cbodley@redhat.com>

metadata sync ignores entries other than MDLOG_STATUS_COMPLETE, so we don't need to write separate prepare/complete entries. metadata mutations can just call complete_entry() on success Signed-off-by: Casey Bodley <cbodley@redhat.com>

kchheda3 · 2024-02-06T00:53:58Z

src/rgw/rgw_admin.cc

  formatter->open_object_section("topic");
  topic.dump(formatter);
-  encode_json("subscribed_buckets", subscribed_buckets, formatter);
+  if (!subscribed_buckets.empty()) {


probably we should show empty subscribed_buckets [] ?

ok, i made this conditional so we could use the same helper function for v1 and v2 topics. should v1 output show the []?

ok, i made this conditional so we could use the same helper function for v1 and v2 topics. should v1 output show the []?
v1 does not even have concept of subscribed_buckets, so only for v2

reverted this back to show_topics_info_v2() instead of sharing with the v1 case

kchheda3 · 2024-02-06T00:56:19Z

looks good after SiteConfig changes

rename read_topics()/write_topics() to 'v1' and only call them from internal v1 call paths public get_topics() now calls read_topics_v1() for the v1 case, and does the paginated listing with driver->meta_list_keys_next() for v2 RGWPSListTopicsOp now uses the NextToken request/response params with the paginated get_topics(), limiting responses to 100 entries like AWS 'radosgw-admin topic list' also paginates the listing according to --max-entries to avoid reading everything into memory at once Signed-off-by: Casey Bodley <cbodley@redhat.com>

the format of topic metadata keys is agnostic to the backend, so the parsing/formatting functions should be in rgw_pubsub.h Signed-off-by: Casey Bodley <cbodley@redhat.com>

add a new interface for topic metadata that doesn't depend on metadata backends. this low-level interface is used by both RadosStore and the topic metadata handler remove Driver::delete_bucket_topic_mapping() from sal because the omap object is deleted internally by rgwrados::topic::remove() remove the RGWRados::topics_pool_ctx member Signed-off-by: Casey Bodley <cbodley@redhat.com>

Signed-off-by: Casey Bodley <cbodley@redhat.com>

cbodley · 2024-03-08T18:12:24Z

merged with #55657

github-actions bot added build/ops documentation rgw tests labels Jan 11, 2024

cbodley mentioned this pull request Jan 11, 2024

rgw/multisite-notification: Replicate Topic and Bucket notification in multisite config #54868

Closed

14 tasks

kchheda3 reviewed Jan 11, 2024

View reviewed changes

src/rgw/driver/rados/topic.cc Show resolved Hide resolved

kchheda3 reviewed Jan 11, 2024

View reviewed changes

src/rgw/driver/rados/topic.cc Show resolved Hide resolved

kchheda3 reviewed Jan 12, 2024

View reviewed changes

kchheda3 reviewed Jan 17, 2024

View reviewed changes

src/rgw/driver/rados/rgw_metadata_lister.h Show resolved Hide resolved

kchheda3 reviewed Jan 18, 2024

View reviewed changes

src/rgw/driver/rados/topic.cc Show resolved Hide resolved

src/rgw/driver/rados/topic.cc Outdated Show resolved Hide resolved

cbodley force-pushed the wip-rgw-meta-topic branch from ba248ea to 84f6227 Compare January 18, 2024 20:46

kchheda3 reviewed Jan 19, 2024

View reviewed changes

src/rgw/driver/rados/rgw_metadata_lister.h Show resolved Hide resolved

cbodley marked this pull request as ready for review January 19, 2024 16:38

cbodley requested a review from a team as a code owner January 19, 2024 16:38

kchheda3 approved these changes Jan 20, 2024

View reviewed changes

kchheda3 reviewed Jan 22, 2024

View reviewed changes

src/rgw/rgw_admin.cc Outdated Show resolved Hide resolved

kchheda3 reviewed Jan 22, 2024

View reviewed changes

src/rgw/rgw_pubsub.cc Show resolved Hide resolved

github-actions bot added the needs-rebase label Jan 24, 2024

kchheda3 and others added 7 commits February 1, 2024 15:54

rgw/multisite-notification: Add support to replicate topic creation i…

5c6c322

…n multisite config Signed-off-by: kchheda3 <kchheda3@bloomberg.net>

rgw/multisite-notification: Add support to replicate bucket notificat…

6bde5e3

…ions in multisite config. Signed-off-by: kchheda3 <kchheda3@bloomberg.net>

rgw/multisite-notification: Add omap object to store the mapping betw…

4d0dc17

…een bucket and topics. Signed-off-by: kchheda3 <kchheda3@bloomberg.net>

rgw/multisite-notification: Add integration test for bucket notificat…

37f56c7

…ions in multisite config. Signed-off-by: kchheda3 <kchheda3@bloomberg.net>

test/rgw/notifications: support running tests in multisite environment

99e7643

both locally and in teuthology Signed-off-by: Yuval Lifshitz <ylifshit@redhat.com>

rgw/multisite-notification: allow enabling v1/v2 in tests

dece9c0

v1 could be enabled only in local tests. teuthology tests would run with v2 Signed-off-by: Yuval Lifshitz <ylifshit@redhat.com>

radosgw-admin: make SiteConfig available to commands

f4bdcff

Signed-off-by: Casey Bodley <cbodley@redhat.com>

cbodley force-pushed the wip-rgw-meta-topic branch from 73e1929 to 3a092cc Compare February 4, 2024 23:46

github-actions bot removed the needs-rebase label Feb 4, 2024

cbodley added 5 commits February 5, 2024 14:26

rgw/rados: store SiteConfig with RGWServices

1935ec1

make the SiteConfig available to all of RGWRados via svc.site instead of storing it in sal::RadosStore Signed-off-by: Casey Bodley <cbodley@redhat.com>

rgw/rados: RGWTopicMetadataHandler doesn't depend on RGWPubSub

9e42235

Signed-off-by: Casey Bodley <cbodley@redhat.com>

rgw/metadata: add RGWMetadataLister abstraction

25dce85

Signed-off-by: Casey Bodley <cbodley@redhat.com>

cbodley force-pushed the wip-rgw-meta-topic branch from 3a092cc to f8f6b11 Compare February 5, 2024 20:20

kchheda3 reviewed Feb 6, 2024

View reviewed changes

cbodley mentioned this pull request Feb 6, 2024

RGW: Migrate topics to data path v2 #55214

Merged

14 tasks

cbodley added 5 commits February 6, 2024 13:02

rgw/topic: metadata key format in rgw_pubsub.h

9e00188

the format of topic metadata keys is agnostic to the backend, so the parsing/formatting functions should be in rgw_pubsub.h Signed-off-by: Casey Bodley <cbodley@redhat.com>

rgw/topic: remove unused RGWSI_Topic_RADOS

f606f00

Signed-off-by: Casey Bodley <cbodley@redhat.com>

rgw/topic: metadata handler adds/removes persistent queues

1cf5de1

Signed-off-by: Casey Bodley <cbodley@redhat.com>

cbodley force-pushed the wip-rgw-meta-topic branch from f8f6b11 to 1cf5de1 Compare February 6, 2024 18:13

This was referenced Feb 16, 2024

rgw: remove unused backend abstraction for metadata handlers #54777

Merged

rgw: replicate v2 topic/notification metadata #55657

Merged

cbodley closed this Mar 8, 2024

Conversation

cbodley commented Jan 11, 2024

Uh oh!

Uh oh!

Uh oh!

kchheda3 commented Jan 11, 2024

Uh oh!

kchheda3 Jan 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kchheda3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cbodley commented Jan 18, 2024

Uh oh!

kchheda3 commented Jan 18, 2024

Uh oh!

Uh oh!

kchheda3 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 24, 2024

Uh oh!

kchheda3 Feb 6, 2024

Choose a reason for hiding this comment

Uh oh!

cbodley Feb 6, 2024

Choose a reason for hiding this comment

Uh oh!

kchheda3 Feb 6, 2024

Choose a reason for hiding this comment

Uh oh!

cbodley Feb 6, 2024

Choose a reason for hiding this comment

Uh oh!

kchheda3 commented Feb 6, 2024

Uh oh!

cbodley commented Mar 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kchheda3 Jan 12, 2024 •

edited

Loading

kchheda3 left a comment •

edited

Loading