Added `redpanda.virtual.cluster.id` topic property #16714

mmaslankaprv · 2024-02-26T18:29:47Z

Added redpanda.virtual.cluster.id topic property which affiliates the topic with virtual cluster. The property is only settable during topic creation and can not be modified.

Backports Required

Release Notes

none

mmaslankaprv · 2024-02-29T08:02:54Z

/dt

mmaslankaprv · 2024-02-29T11:16:51Z

/dt

vbotbuildovich · 2024-02-29T15:44:47Z

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45496#018df53e-a963-4925-85b8-7a6eeedc3101

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45685#018e0e0b-253c-4a0b-8ff1-1e67335ddf8a

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45685#018e0f1d-0fb3-472e-a100-372186bfc2f8

bharathv

lgtm, couple of nits.. not too familiar with the topic_manifest related changes, so someone else should take a look too.

bharathv · 2024-03-04T17:53:40Z

src/v/config/configuration.cc

+  , enable_mpx_extensions(
+      *this,
+      "enable_mpx_extensions",
+      "Enable Redpanda extensions for MPX.",


nit: a bit more verbose description

bharathv · 2024-03-04T17:58:04Z

src/v/kafka/server/handlers/topics/validators.h

+
+    static bool is_valid(const creatable_topic& c) {
+        if (!config::shard_local_cfg().enable_mpx_extensions()) {
+            return true;


nit: should this not be

return config_entries.find(topic_property_mpx_virtual_cluster_id) == config_entries.end();

or at least check for validity of property that it conforms to vcluster_id type (like in L326), otherwise I think there is a scope to sneak in garbage for this field.

if the extensions are not enabled the field is ignored, so we do not need to validate it, there is no risk of storing garbage as the property value. I did it like this as when unknown property is sent to create topics request it is silently ignored. This way the logic in validator is consistent with what we already have.

`vcluster_id` type is going to have limited scope. We are going to need it in the cloud storage module, hence moving the type to `model` submodule. Signed-off-by: Michal Maslanka <michal@redpanda.com>

Added topic property that is a label used to provide an information about topic virtual cluster affiliation. Signed-off-by: Michal Maslanka <michal@redpanda.com>

graphcareful

LGTM seems pretty complete !

michael-redpanda

looks good! just some suggestions

src/v/compat/cluster_compat.h

src/v/compat/cluster_json.h

Added property marking topic virtual cluster affiliation. The virtual cluster property is different than other topic properties as it has no default. By default it is an empty optional. Signed-off-by: Michal Maslanka <michal@redpanda.com>

Signed-off-by: Michal Maslanka <michal@redpanda.com>

github-actions bot added the area/redpanda label Feb 26, 2024

mmaslankaprv force-pushed the v-cluster-id-topic-property branch from 9a2d1dd to 13d0db5 Compare February 27, 2024 09:25

mmaslankaprv marked this pull request as ready for review February 27, 2024 11:34

mmaslankaprv marked this pull request as draft February 27, 2024 11:55

mmaslankaprv force-pushed the v-cluster-id-topic-property branch 2 times, most recently from 081d4d1 to f93278e Compare February 29, 2024 07:57

mmaslankaprv force-pushed the v-cluster-id-topic-property branch from f93278e to 237d2e7 Compare February 29, 2024 11:08

mmaslankaprv marked this pull request as ready for review February 29, 2024 13:09

mmaslankaprv requested review from michael-redpanda, bharathv, BenPope, oleiman and graphcareful February 29, 2024 13:10

redpanda-data deleted a comment from vbotbuildovich Mar 1, 2024

bharathv reviewed Mar 4, 2024

View reviewed changes

mmaslankaprv added 2 commits March 5, 2024 10:00

model: moved vcluster_id type to model

22a941b

`vcluster_id` type is going to have limited scope. We are going to need it in the cloud storage module, hence moving the type to `model` submodule. Signed-off-by: Michal Maslanka <michal@redpanda.com>

topics: added definition of redpanda.virtual.cluster.id topic property

52d7161

Added topic property that is a label used to provide an information about topic virtual cluster affiliation. Signed-off-by: Michal Maslanka <michal@redpanda.com>

mmaslankaprv force-pushed the v-cluster-id-topic-property branch from 98f2bf1 to a3ebcf3 Compare March 5, 2024 09:04

mmaslankaprv requested a review from bharathv March 5, 2024 16:50

graphcareful previously approved these changes Mar 6, 2024

View reviewed changes

michael-redpanda reviewed Mar 6, 2024

View reviewed changes

src/v/compat/cluster_compat.h Outdated Show resolved Hide resolved

src/v/compat/cluster_compat.h Outdated Show resolved Hide resolved

src/v/compat/cluster_json.h Outdated Show resolved Hide resolved

src/v/compat/cluster_json.h Outdated Show resolved Hide resolved

mmaslankaprv added 6 commits March 7, 2024 08:15

k/validators: added virtual cluster id topic property validator

9c7b6e0

Signed-off-by: Michal Maslanka <michal@redpanda.com>

k/create_topics: added handling for vcluster_id property

330b967

Signed-off-by: Michal Maslanka <michal@redpanda.com>

k/describe_configs: added handling for vcluster id property

4b3d6e3

Signed-off-by: Michal Maslanka <michal@redpanda.com>

tests: added vcluster id topic property definition

98f3100

Signed-off-by: Michal Maslanka <michal@redpanda.com>

tests: added virtual cluster id topic property test

6da88bf

Signed-off-by: Michal Maslanka <michal@redpanda.com>

mmaslankaprv added 3 commits March 7, 2024 08:15

cloud_storage: added vcluster_id property to topic manifest

b2e9b0b

Signed-off-by: Michal Maslanka <michal@redpanda.com>

tests: test topic recovery with vcluster_id property set

08164d7

Signed-off-by: Michal Maslanka <michal@redpanda.com>

tools/viewer: updated metadata viewer with new topic properties

5852ac0

Signed-off-by: Michal Maslanka <michal@redpanda.com>

mmaslankaprv dismissed graphcareful’s stale review via 5852ac0 March 7, 2024 07:15

mmaslankaprv force-pushed the v-cluster-id-topic-property branch from a3ebcf3 to 5852ac0 Compare March 7, 2024 07:15

mmaslankaprv requested review from graphcareful and michael-redpanda March 7, 2024 07:15

bharathv approved these changes Mar 7, 2024

View reviewed changes

mmaslankaprv merged commit 7dba0b9 into redpanda-data:dev Mar 7, 2024
17 checks passed

mmaslankaprv deleted the v-cluster-id-topic-property branch March 7, 2024 18:16

andrwng mentioned this pull request Mar 28, 2024

Feat/topic manifest serde #17405

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added `redpanda.virtual.cluster.id` topic property #16714

Added `redpanda.virtual.cluster.id` topic property #16714

mmaslankaprv commented Feb 26, 2024 •

edited

mmaslankaprv commented Feb 29, 2024

mmaslankaprv commented Feb 29, 2024

vbotbuildovich commented Feb 29, 2024 •

edited

bharathv left a comment

bharathv Mar 4, 2024

bharathv Mar 4, 2024

mmaslankaprv Mar 5, 2024

graphcareful left a comment

michael-redpanda left a comment

Added redpanda.virtual.cluster.id topic property #16714

Added redpanda.virtual.cluster.id topic property #16714

Conversation

mmaslankaprv commented Feb 26, 2024 • edited

Backports Required

Release Notes

mmaslankaprv commented Feb 29, 2024

mmaslankaprv commented Feb 29, 2024

vbotbuildovich commented Feb 29, 2024 • edited

bharathv left a comment

Choose a reason for hiding this comment

bharathv Mar 4, 2024

Choose a reason for hiding this comment

bharathv Mar 4, 2024

Choose a reason for hiding this comment

mmaslankaprv Mar 5, 2024

Choose a reason for hiding this comment

graphcareful left a comment

Choose a reason for hiding this comment

michael-redpanda left a comment

Choose a reason for hiding this comment

Added `redpanda.virtual.cluster.id` topic property #16714

Added `redpanda.virtual.cluster.id` topic property #16714

mmaslankaprv commented Feb 26, 2024 •

edited

vbotbuildovich commented Feb 29, 2024 •

edited