c/migrations: list mountable topics #23601

nvartolomei · 2024-10-02T15:17:49Z

Backports Required

Release Notes

none

src/v/cluster/data_migration_frontend.cc

src/v/redpanda/admin/api-doc/migration.def.json

tests/rptest/tests/data_migrations_api_test.py

nvartolomei · 2024-10-04T09:40:11Z

@bashtanov @andrwng this is ready for review.

micheleRP

looks good from docs

src/v/cluster/controller.h

dotnwat

b288d47, which appears to be the substantive commit of this PR, needs a commit message description.

bashtanov · 2024-10-09T11:31:09Z

and split this last commit perhaps, as it is 19 files and 500+ lines

vbotbuildovich · 2024-10-09T22:02:54Z

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/56169#019272fe-f7e4-44d4-97f1-c034ee5f4adb
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/57066#0192baaf-6afa-4d22-891c-dc91fdc99cbe

bashtanov · 2024-10-11T09:42:56Z

src/v/cloud_storage/topic_mount_handler.cc

+          .label = cloud_storage::remote_label(
+            path_parse_result->cluster_uuid()),


@nvartolomei Willem is adding revision to the mount manifest path in #23730. Once it is merged, can we return {uuid}/{rev} here? The plan is to support a hint from user when mounting, and the hint is a prefix for this path component of the topic manifest. If there are multiple unmounted topics with the same name and cluster UUID having a revision would be beneficial.

Should we keep them separate and ask user 3 fields (label (cluster uuid), topic, revision)? Because that's how things are represented in most parts. I'm afraid that putting together the label/cluster uuid with revision id will cause confusion long term... It is an accident for the most part that label and rev id are next to each other here (topic manifest). I.e. partition manifest, the path is <label>/meta/<ns>/<topic>/<partition>_<revision> and who knows what we'll come up with down the road.

bashtanov

LGTM, a few silly questions from me.
Also we may need to rename unmounted->mountable as discussed on slack.

src/v/cloud_storage/remote_path_provider.cc

src/v/cloud_storage/remote_label.h

src/v/cloud_storage/tests/topic_mount_handler_test.cc

src/v/cloud_storage/topic_mount_handler.h

src/v/redpanda/admin/api-doc/migration.def.json

vbotbuildovich · 2024-10-23T16:32:40Z

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/57053#0192b9fa-d87c-4777-90e8-c916b6719851:

"rptest.tests.data_migrations_api_test.DataMigrationsApiTest.test_list_mountable_topics"

vbotbuildovich · 2024-10-23T16:32:51Z

Retry command for Build#57053

please wait until all jobs are finished before running the slash command

/ci-repeat 1
tests/rptest/tests/data_migrations_api_test.py::DataMigrationsApiTest.test_list_mountable_topics

src/v/cloud_storage/topic_mount_manifest_path.h

src/v/cluster/data_migration_frontend.cc

src/v/redpanda/admin/topics.cc

src/v/cluster/data_migration_frontend.cc

src/v/redpanda/admin/api-doc/migration.def.json

We want to use parsing functionality for listing unmounted topics.

This will allow the same fixture to be used for multiple test suites

We will expose this method through the admin api. A topic for which a mount manifest exists in cloud storage is consider mountable. We list mountable topics by listing the prefix where these manifests are stored and parsing label, namespace, and topic from the object name.

I plan to reuse the same topic mount handler in the migration frontend as well to implement listing of unmounted topics. Lift it up in this commit. In subsequent commit it will be injected and used in the frontend.

All addressed. Happy to follow up.

github-actions bot added area/build area/redpanda labels Oct 2, 2024

nvartolomei force-pushed the nv/list-unmounted branch from be9dc1b to 8d24a46 Compare October 2, 2024 15:20

nvartolomei commented Oct 2, 2024

View reviewed changes

src/v/cluster/data_migration_frontend.cc Outdated Show resolved Hide resolved

src/v/redpanda/admin/api-doc/migration.def.json Outdated Show resolved Hide resolved

tests/rptest/tests/data_migrations_api_test.py Outdated Show resolved Hide resolved

nvartolomei force-pushed the nv/list-unmounted branch 11 times, most recently from 8e38ad1 to fe3561c Compare October 4, 2024 09:39

nvartolomei requested review from bashtanov and andrwng October 4, 2024 09:39

nvartolomei marked this pull request as ready for review October 4, 2024 09:39

nvartolomei requested a review from a team as a code owner October 4, 2024 09:39

redpanda-data deleted a comment from vbotbuildovich Oct 4, 2024

nvartolomei force-pushed the nv/list-unmounted branch from fe3561c to b288d47 Compare October 4, 2024 13:02

micheleRP previously approved these changes Oct 4, 2024

View reviewed changes

bashtanov reviewed Oct 4, 2024

View reviewed changes

src/v/cluster/controller.h Show resolved Hide resolved

dotnwat previously requested changes Oct 8, 2024

View reviewed changes

nvartolomei dismissed micheleRP’s stale review via 12f9924 October 9, 2024 18:55

nvartolomei force-pushed the nv/list-unmounted branch 2 times, most recently from 12f9924 to 61420e9 Compare October 9, 2024 18:56

bashtanov reviewed Oct 11, 2024

View reviewed changes

nvartolomei force-pushed the nv/list-unmounted branch from 64e6791 to c30743f Compare October 21, 2024 14:51

nvartolomei requested a review from bashtanov October 21, 2024 14:52

nvartolomei force-pushed the nv/list-unmounted branch 2 times, most recently from c1c3bf1 to 0ea313d Compare October 21, 2024 15:49

bashtanov reviewed Oct 22, 2024

View reviewed changes

nvartolomei changed the title ~~c/migrations: list unmounted topics~~ c/migrations: list mountable topics Oct 23, 2024

nvartolomei force-pushed the nv/list-unmounted branch from 0ea313d to e225143 Compare October 23, 2024 13:32

nvartolomei requested a review from bashtanov October 23, 2024 13:46

nvartolomei force-pushed the nv/list-unmounted branch from e225143 to 5c9a2a0 Compare October 23, 2024 13:49

Feediver1 reviewed Oct 23, 2024

View reviewed changes

src/v/redpanda/admin/api-doc/migration.def.json Outdated Show resolved Hide resolved

Feediver1 reviewed Oct 23, 2024

View reviewed changes

src/v/redpanda/admin/api-doc/migration.def.json Outdated Show resolved Hide resolved

nvartolomei force-pushed the nv/list-unmounted branch from 5c9a2a0 to a702401 Compare October 23, 2024 17:04

dotnwat reviewed Oct 23, 2024

View reviewed changes

src/v/cloud_storage/topic_mount_manifest_path.h Outdated Show resolved Hide resolved

src/v/cluster/data_migration_frontend.cc Outdated Show resolved Hide resolved

src/v/redpanda/admin/topics.cc Show resolved Hide resolved

src/v/cluster/data_migration_frontend.cc Show resolved Hide resolved

kbatuigas reviewed Oct 23, 2024

View reviewed changes

src/v/redpanda/admin/api-doc/migration.def.json Show resolved Hide resolved

nvartolomei added 2 commits October 24, 2024 10:15

cloud_storage: extract mount topic path and allow to parse it

d678012

We want to use parsing functionality for listing unmounted topics.

cloud_storage/tests: split mount handler fixture and suite

da0543b

This will allow the same fixture to be used for multiple test suites

nvartolomei force-pushed the nv/list-unmounted branch from a702401 to 64e436b Compare October 24, 2024 09:21

nvartolomei added 3 commits October 24, 2024 10:21

cluster: lift mount handler from migration backend to controller

d2fbb3c

I plan to reuse the same topic mount handler in the migration frontend as well to implement listing of unmounted topics. Lift it up in this commit. In subsequent commit it will be injected and used in the frontend.

c/migrations: list mountable admin endpoint

64e436b

bashtanov approved these changes Oct 24, 2024

View reviewed changes

nvartolomei requested a review from dotnwat October 24, 2024 09:40

nvartolomei merged commit 5cf22c0 into redpanda-data:dev Oct 24, 2024
18 checks passed

nvartolomei deleted the nv/list-unmounted branch October 24, 2024 12:58

gene-redpanda mentioned this pull request Oct 24, 2024

feat: add support for mountable redpanda-data/common-go#29

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

c/migrations: list mountable topics #23601

c/migrations: list mountable topics #23601

nvartolomei commented Oct 2, 2024

nvartolomei commented Oct 4, 2024

micheleRP left a comment

dotnwat left a comment

bashtanov commented Oct 9, 2024 •

edited

Loading

vbotbuildovich commented Oct 9, 2024 •

edited

Loading

bashtanov Oct 11, 2024

nvartolomei Oct 11, 2024

bashtanov left a comment

vbotbuildovich commented Oct 23, 2024

vbotbuildovich commented Oct 23, 2024

		.label = cloud_storage::remote_label(
		path_parse_result->cluster_uuid()),

c/migrations: list mountable topics #23601

c/migrations: list mountable topics #23601

Conversation

nvartolomei commented Oct 2, 2024

Backports Required

Release Notes

nvartolomei commented Oct 4, 2024

micheleRP left a comment

Choose a reason for hiding this comment

dotnwat left a comment

Choose a reason for hiding this comment

bashtanov commented Oct 9, 2024 • edited Loading

vbotbuildovich commented Oct 9, 2024 • edited Loading

bashtanov Oct 11, 2024

Choose a reason for hiding this comment

nvartolomei Oct 11, 2024

Choose a reason for hiding this comment

bashtanov left a comment

Choose a reason for hiding this comment

vbotbuildovich commented Oct 23, 2024

vbotbuildovich commented Oct 23, 2024

Retry command for Build#57053

bashtanov commented Oct 9, 2024 •

edited

Loading

vbotbuildovich commented Oct 9, 2024 •

edited

Loading