rpk: support node-local core assignment API #21573
Conversation
r-vasquez
force-pushed
the
partition-move-changes
branch
2 times, most recently
from
d1e65a9
to
66a94ee
Compare
r-vasquez
changed the title
r-vasquez
requested review from
twmb,
gene-redpanda and
Deflaimun
as code owners
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/51945#0190e263-413a-4d8d-a0f4-9477db6f2867 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/51945#0190e262-4be3-48c9-9777-9230cf02f7b2 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/52042#0190eac6-692b-4760-941e-a25bb0b63bd8 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/52056#0190ebd1-45dc-4cfd-88a6-f0cb8e77bb64 |
| for i, newa := range newAssignmentList { | ||
| i := i | ||
| newa := newa | ||
| i, newa := i, newa |
There was a problem hiding this comment.
is there a reason for the reassignment here?
There was a problem hiding this comment.
or rather, is there a reason to reassign to a local with the same name?
There was a problem hiding this comment.
An old habit of avoiding mistakingly sharing the loop variable across different routines: https://go.dev/blog/loopvar-preview (also, this original code was written before Go 1.22)
This is fixed in Go 1.22 though so we should be fine removing it from the codebase (we have other instances of this across rpk), but if it's ok for you, I will leave this one here until we have proper ducktape tests for this particularly dangerous command.
There was a problem hiding this comment.
That I'm familiar with, I guess I'm just used to renaming the loop variables when I do it.
r-vasquez
force-pushed
the
partition-move-changes
branch
from
66a94ee
to
ab995af
Compare
|
|
new failures in https://buildkite.com/redpanda/redpanda/builds/52042#0190eadc-2bed-47ed-8292-a3c570c1e8f9: |
Redpanda clusters are switching to a node-local core assignments, which means that the API for dispatching cross-core partition moves will be different. This is guarded behind the new feature: node_local_core_assignment. Those clusters with the feature enabled will issue the core movements independently from the node changed. If it's not enabled/present, we still do the old API calls.
r-vasquez
force-pushed
the
partition-move-changes
branch
from
ab995af
to
1950bff
Compare
|
|
Unrelated rp unit test failures: https://buildkite.com/redpanda/redpanda/builds/52056#0190eb8c-c461-4b40-b532-5a47dc4bf553/6-11124 |
|
/backport v24.2.x |
Redpanda clusters are switching to node-local core assignments, which means that the API for dispatching cross-core partition moves will be different.
This is guarded behind the new feature: node_local_core_assignment. Those clusters with the feature enabled will issue the core movements independently from the node change.
If it's not enabled/present, we still do the old API calls.
Fixes #21562
Backports Required
Release Notes