Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpk: add 'partitions move' command #13684

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

daisukebe
Copy link
Contributor

@daisukebe daisukebe commented Sep 26, 2023

In this PR, we add 'rpk cluster partitions move' that reassigns given partition replicas across brokers and cores. The command wraps the /v1/partitions/{ns}/{topic}/{partition}/replicas endpoint. Pasting the help text here:

This command changes replica assignments for given partitions. By default, it
assumes the "kafka" namespace, but you can specify an internal namespace using
the "{namespace}/" prefix.

To move replicas, use the following syntax:

    rpk cluster partitions move foo --partition 0:1,2,3 -p 1:2,3,4

Here, the command assigns new replicas for partition 0 to brokers [1, 2, 3] and
for partition 1 to brokers [2, 3, 4] for the topic "foo".

You can also specify the core id with "-{core_id}" where the new replicas
should be placed:

    rpk cluster partitions move foo -p 0:1-0,2-0,3-0

Here all new replicas [1, 2, 3] will be assigned on core 0 on the nodes.

The command does not change a "core" assignment unless it is explicitly
specified. When a core is not specified for a new node, the command randomly
picks a core and assign a replica on the core.

Topic arguments are optional. For more control, you can specify the topic name
in the "--partition" flag:

    rpk cluster partitions move -p foo/0:1,2,3 -p kafka_internal/tx/0:1-0,2-0,3-0

Example output:

$ rpk cluster partitions move -p dice/0:1-1,2-1,4 -p dice/1:1,2,3-10 -p dice/2:1,2,5-10 -p foo/0:1,2,3
NAMESPACE  TOPIC  PARTITION  OLD-REPLICAS     NEW-REPLICAS      ERROR
kafka      dice   0          [1-1, 2-1, 4-0]  [1-1, 2-1, 4-0]
kafka      dice   1          [1-0, 2-1, 3-0]  [1-0, 2-1, 3-10]  Replica set refers to non-existent node/shard (node 3 shard 10)
kafka      dice   2          [1-1, 2-1, 4-1]  [1-1, 2-1, 5-10]  Replica set refers to non-existent node/shard (node 5 shard 10)
kafka      foo    0          [1-0, 2-0, 3-1]  [1-0, 2-0, 3-1]

Successfully began 2 partition movement(s).

Check the movement status with 'rpk cluster partitions move-status' or see new assignments with 'rpk topic describe -p TOPIC'.

Fixes #9205 (1/3 of it)

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.2.x
  • v23.1.x
  • v22.3.x

Release Notes

Features

  • rpk: add 'partitions move' to reassign replicas

Copy link
Contributor

@r-vasquez r-vasquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, some small comments

src/go/rpk/pkg/cli/cluster/partitions/move.go Show resolved Hide resolved
src/go/rpk/pkg/cli/cluster/partitions/move.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cluster/partitions/move.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cluster/partitions/move.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cluster/partitions/move.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cluster/partitions/move.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cluster/partitions/move.go Outdated Show resolved Hide resolved
@daisukebe
Copy link
Contributor Author

Force pushed 1595272. Changes are below:

  • Added license
  • Revised the whole logic on parsing NTP and replicas / cores. Regex is used entirely
    • Parse ntp first
    • Fetch current replica assignments
    • Parse new replicas. Compare with current ones to decide the new core assignments
  • Made the for loop concurrent
  • Revised the long help text

r-vasquez
r-vasquez previously approved these changes Oct 9, 2023
@twmb
Copy link
Contributor

twmb commented Oct 10, 2023

I'd like to take a look again before this is merged, there are some things I'm worried about being serial that instead can be concurrent (or cached)

@daisukebe
Copy link
Contributor Author

daisukebe commented Oct 11, 2023

Thanks, Travis. Just one confirmation; you meant sequential is more serious (concerned) than concurrent?

@daisukebe
Copy link
Contributor Author

@twmb, thanks for raising #14267 ensure the operation reliable. In the mean time, given hitting the raw admin endpoint in the wild is painful, is it a good idea to change the code to allow one partition movement at a time before redpanda core supports batch processing?

src/go/rpk/pkg/cli/cluster/partitions/move.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cluster/partitions/move.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cluster/partitions/move.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cluster/partitions/move.go Outdated Show resolved Hide resolved
src/go/rpk/pkg/cli/cluster/partitions/move.go Outdated Show resolved Hide resolved
@vbotbuildovich
Copy link
Collaborator

@daisukebe
Copy link
Contributor Author

The latest force push contains:

  • help update
  • var name update
  • regex updates
  • make Broker call concurrent with goroutines

@daisukebe
Copy link
Contributor Author

In the latest force push:

  • supported the --format option
  • output to the table along with the API error column
  • supported to show oldReplicas in the result table

Thanks for much @twmb

@vbotbuildovich
Copy link
Collaborator

twmb
twmb previously approved these changes Nov 16, 2023
Copy link
Contributor

@twmb twmb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, can you resolve the conflict, rebase on main, and we can merge?

src/go/rpk/pkg/cli/cluster/partitions/move.go Show resolved Hide resolved
This commit adds a new 'rpk cluster partitions move' command to
reassign given partition replicas. The command wraps the
'/v1/partitions/{ns}/{topic}/{partition}/replicas' endpoint.
@daisukebe
Copy link
Contributor Author

/ci-repeat 2

@twmb twmb merged commit 3d159e5 into redpanda-data:dev Nov 21, 2023
24 checks passed
@vbotbuildovich
Copy link
Collaborator

/backport v23.2.x

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.2.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-13684-v23.2.x-718 remotes/upstream/v23.2.x
git cherry-pick -x b135171fb54380163fbf89e42ac7f24b71cf1ad3

Workflow run logs.

@daisukebe
Copy link
Contributor Author

#14862 should be backported first.

@r-vasquez
Copy link
Contributor

@daisukebe I'm waiting for #15098 before backporting. Probably EOD today.

@daisukebe
Copy link
Contributor Author

@r-vasquez , are we ready to backport this to 23.2.x?

@daisukebe
Copy link
Contributor Author

/backport v23.2.x

@r-vasquez
Copy link
Contributor

@daisukebe We are now, 👍 I see that you started the backport but the linter is failing. Let me check what's going on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rpk: Enhance partition management commands
4 participants