Skip to content

fix: honor --dstNodes in balance-segment (v1.1.x) - #509

Merged
sre-ci-robot merged 1 commit into
milvus-io:v1.1.xfrom
czs007:fix-honor-dstNode-1.1.x
Jul 27, 2026
Merged

fix: honor --dstNodes in balance-segment (v1.1.x)#509
sre-ci-robot merged 1 commit into
milvus-io:v1.1.xfrom
czs007:fix-honor-dstNode-1.1.x

Conversation

@czs007

@czs007 czs007 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Backport of #508 to v1.1.x.

BalanceSegmentCommand parsed the destination flag into p.DstNodes but never put it into the LoadBalanceRequest, so the flag was silently ignored and querycoord always fell back to picking a destination among all nodes of the replica (DstNodeIDs empty means "any RW node").

Populate DstNodeIDs from the flag. Per review feedback on #508, DstNodes is []int64 with flag --dstNodes, mirroring SourceNodes / --srcNodes and the repeated DstNodeIDs field in the proto; an empty value keeps the previous auto-select behaviour.

Cherry-picked from 1d01527.

@mergify

mergify Bot commented Jul 23, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

`BalanceSegmentCommand` parsed the destination flag into `p.DstNodes` but
never put it into the `LoadBalanceRequest`, so the flag was silently
ignored and querycoord always fell back to picking a destination among
all nodes of the replica (`DstNodeIDs` empty means "any RW node").

Verified against a local Milvus 2.6.19 cluster: running
`balance-segment --collection C --segment S --srcNodes 10 --dstNode 8`
returned success but the segment landed on node 11, not node 8.

Populate `DstNodeIDs` from the flag. `DstNodes` is now `[]int64` /
`--dstNodes`, mirroring `SourceNodes` / `--srcNodes` and the repeated
`DstNodeIDs` field in the proto; an empty value keeps the previous
auto-select behaviour.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 1d01527)
Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com>
@czs007
czs007 force-pushed the fix-honor-dstNode-1.1.x branch from 31e6db0 to 5da7486 Compare July 27, 2026 06:48
@czs007 czs007 changed the title fix: honor --dstNode in balance-segment (v1.1.x) fix: honor --dstNodes in balance-segment (v1.1.x) Jul 27, 2026

@congqixia congqixia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

/lgtm

@sre-ci-robot sre-ci-robot added the lgtm look good to me label Jul 27, 2026
@sre-ci-robot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: congqixia, czs007

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot
sre-ci-robot merged commit efc386e into milvus-io:v1.1.x Jul 27, 2026
5 checks passed
sre-ci-robot pushed a commit that referenced this pull request Jul 27, 2026
## What

`balance-segment` accepts a destination-node flag, but the value was
never sent to querycoord.

`BalanceSegmentCommand` parses the flag into `p.DstNodes` and then
builds the request without it:

```go
req := &querypb.LoadBalanceRequest{
    Base:             &commonpb.MsgBase{TargetID: s.session.ServerID},
    CollectionID:     p.CollectionID,
    SealedSegmentIDs: p.SegmentIDs,
    SourceNodeIDs:    p.SourceNodes,
}   // p.DstNodes is never used
```

On the server side, an empty `DstNodeIDs` means "any RW node of the
replica" — querycoord inserts `replica.GetRWNodes()` into the
destination set and lets the assign policy pick. So the flag is silently
ignored and the caller gets a destination they did not ask for, with no
error.

## Repro

Against a local Milvus 2.6.19 cluster (3 querynodes: 8, 10, 11):

```
$ balance-segment --collection 467844892567274894 \
      --segment 467844892568376490 --srcNodes 10 --dstNode 8
(success, no error)

$ show segment-loaded-grpc
ServerID 8     (segment not here)
ServerID 10    (segment not here)
ServerID 11
SegmentID: 467844892568376490 ... NumOfRows 120000     <- landed on 11, not the requested 8
```

## Fix

Populate `DstNodeIDs` from the flag.

Per review feedback, `DstNodes` is `[]int64` with flag `--dstNodes`,
mirroring `SourceNodes` / `--srcNodes` and the repeated `DstNodeIDs`
field in the proto. An empty value keeps the previous behaviour (let
querycoord pick), so existing usage is unaffected — and nothing could
have depended on the old `--dstNode` spelling, since the value was never
sent.

## Test

Rebuilt and re-ran the same command against the same cluster; the
segment now lands on the requested node. `go build ./...` and `gofmt`
clean.

Backport to `v1.1.x`: #509.

Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants