fix: ignore topology changes that predate a replication task by czs007 · Pull Request #52728 · milvus-io/milvus

czs007 · 2026-08-20T16:54:28Z

Problem

A replicator does not start reading at the live position. It resumes from the checkpoint
the target cluster reports, and after milvus-backup restore secondary that checkpoint is
the position the backup was taken at. The replicator therefore replays the source WAL from
far behind, walking over every topology change made since — including ones that removed this
very edge before it was re-created.

IsReplicationRemovedByAlterReplicateConfigMessage decides purely on the message contents,
with no ordering check, so a historical detach is executed as if it had just been issued:

the replicator deletes its own replicating-pchannel/ key and stops
(replicate_stream_client_impl.go:337-370), and the same predicate ends the consume loop
(channel_replicator.go:191-199);
the same message is forwarded to the secondary, where overwriteAlterReplicateConfigMessage
finds the secondary absent from the configuration and turns it back into a standalone
primary.

In the field this stopped replication 0.2 s after it started, and left a state nothing
recovers from: the topology query still reports the edge, because only the per-pchannel task
keys are gone and the configuration key is untouched, and re-applying the same configuration
is short-circuited as unchanged and returns success.

Ordinary CDC catch-up never exposes this — the checkpoint stays close to the live position
and only new messages are read.

Change

ReplicatePChannelMeta.initialized_checkpoint already carries the time tick of the
AlterReplicateConfig that created the task, so no new field or protocol change is needed.
A message at or before it predates the task and cannot be an instruction for it.

util.IsStaleTopologyChange implements that comparison.
IsReplicationRemovedByAlterReplicateConfigMessage returns false for such a message, so
the replicator neither deletes its metadata nor ends its consume loop.
The consume loop drops such a message instead of forwarding it, so the secondary cannot be
flipped back to a standalone primary by a configuration from the past.
A task with no initialized time tick — written by an older version — enforces no ordering
and keeps the previous behaviour.

The pchannel-increasing path is unaffected: it deliberately sets the initialized time tick to
one below the creating message, so that message is still delivered, and it carries the edge,
which takes the existing skip path.

Tests

internal/cdc/util had no test file at all. Added:

TestIsStaleTopologyChange — older than the task, exactly the creating message, newer than
the task, and the no-initialized-time-tick case.
TestIsReplicationRemovedByAlterReplicateConfigMessage — a current detach still removes the
replication; a current configuration that keeps the edge does not; a detach that predates
the task does not remove it (the regression); the no-initialized-time-tick case still
removes; an ignored message never removes.
TestChannelReplicatorConsumeLoopSkipsStaleTopologyChange — a stale topology change is
neither forwarded (the stream client mock has no Replicate expectation, so forwarding
fails the test) nor ends the consume loop.

The existing TestChannelReplicatorConsumeLoopDeletesLagSeriesOnRemoval covers the opposite
direction and still passes: its task carries no initialized checkpoint, so the removal path
is unchanged for it.

sre-ci-robot · 2026-08-20T16:56:08Z

[ci-v2-notice]
Notice: New ci-v2 system is enabled for this PR.

To rerun ci-v2 checks, comment with:

/ci-rerun-code-check // for ci-v2/code-check
/ci-rerun-code-check-macos // for Code Checker MacOS (GitHub Actions)
/ci-rerun-build // for ci-v2/build
/ci-rerun-build-all // for ci-v2/build-all (multi-arch builds)
/ci-rerun-buildenv // for ci-v2/build-env (build milvus-env builder images; update .env after the new tag is ready)
/ci-rerun-ut-integration // for ci-v2/ut-integration, will rerun ci-v2/build
/ci-rerun-ut-go // for ci-v2/ut-go, will rerun ci-v2/build
/ci-rerun-ut-cpp // for ci-v2/ut-cpp
/ci-rerun-ut // for all ci-v2/ut-integration, ci-v2/ut-go, ci-v2/ut-cpp, will rerun ci-v2/build
/ci-rerun-e2e-default // for ci-v2/e2e-default
/ci-rerun-e2e-amd // for ci-v2/e2e-amd (e2e pool dispatcher)
/ci-rerun-e2e-dist-wp // for ci-v2/e2e-dist-wp (Tencent distributed woodpecker-service boundary)
/ci-rerun-build-ut-cov // for ci-v2/build-ut-cov (build + unit tests in one pipeline)
/ci-rerun-gosdk // for ci-v2/go-sdk (Go SDK E2E tests, ARM)
/ci-rerun-gosdk-std // for ci-v2/go-sdk-std (Go SDK E2E, standalone)
/ci-rerun-gosdk-dist-wp // for ci-v2/go-sdk-dist-wp (Go SDK E2E, distributed + Woodpecker service)

If you have any questions or requests, please contact @zhikunyao.

tinswzy · 2026-08-20T17:21:20Z

Must-fix issues introduced by this PR: 0
Merge recommendation: Mergeable as-is — this review found no must-fix issue introduced by this PR.

Adversarial review found no issues requiring changes.

Verified:

internal/streamingcoord/server/balancer/channel/manager.go:686-707 — InitializedCheckpoint.TimeTick has exactly one writer and it stores the creating AlterReplicateConfig's append result on that same pchannel, so the new comparison is against the right clock.
internal/cdc/replication/replicatemanager/channel_replicator.go:151 — the scanner filter is DeliverFilterTimeTickGT(cp.TimeTick), so the creating message is already excluded on the normal path and treating == as stale reproduces existing behaviour rather than changing it.
internal/streamingcoord/server/balancer/channel/manager.go:687-693 — skipGetReplicateCheckpoint tasks persist init-1, so their creating message (tick init+1) is still forwarded on every pchannel and the broadcast can still complete.
internal/cdc/replication/replicatemanager/channel_replicator.go:194 — streamClient.Replicate( has a single call site repo-wide and the new guard sits immediately above it, so no other outbound path bypasses the filter.
internal/cdc/util/util.go:40-50 — the stale check is placed after header.Ignore and before MustNewConfigHelper, which also avoids running a historical configuration through a helper that panics on ErrCurrentClusterNotFound.
internal/cdc/util/util.go:25-26 — GetInitializedCheckpoint().GetTimeTick() is nil-safe on both hops and the initTimeTick != 0 guard preserves prior behaviour for tasks persisted before the field existed.

Low — non-blocking

internal/cdc/replication/replicatemanager/channel_replicator_test.go:113 — At the consume-loop level, no test pins the forward-and-stop direction for a task that carries a non-zero initialized checkpoint. The only consume-loop removal test, TestChannelReplicatorConsumeLoopDeletesLagSeriesOnRemoval, builds its ReplicatePChannelMeta without an InitializedCheckpoint (:113-117) and sends a message with WithTimeTick(1) (:137), so util.IsStaleTopologyChange short-circuits on initTimeTick == 0 (internal/cdc/util/util.go:26) and the timestamp comparison is never reached; the new test covers only the dropped direction. This gap is demonstrably reachable by mutation: replacing the call at channel_replicator.go:188 with r.channel.Value.GetInitializedCheckpoint().GetTimeTick() != 0 — which unconditionally drops every AlterReplicateConfig for any task with an init checkpoint, so genuine detaches are no longer forwarded, the loop no longer stops, and replicating-pchannel/<key> is never cleaned up — leaves go test ./internal/cdc/... fully green. (For completeness: flipping <= to >= in util.go:26 is caught today, failing both internal/cdc/util and replicatemanager.) Suggestion: raise the message tick at :137 from 1 to e.g. 100 and give the task at :113 an InitializedCheckpoint{TimeTick: 50}, so the existing case exercises "non-zero init tick + newer message → must forward and stop the loop". (raised by tinswzy)

sre-ci-robot · 2026-08-20T17:58:06Z

❌ CI Loop Results `f30715f`

Stage	Result	Duration	Tests
✅ Build	SUCCESS	10.8min	-
❌ Code-Check	FAILURE	5.5min	-
❌ UT-Integration	SKIPPED	-	-
❌ UT-GO	SKIPPED	-	-
✅ UT-CPP-Cov	SUCCESS	39.8min	8755 total, 8755 passed, 0 failed

Total: 54min | Pipeline | Artifacts

Failed Test Logs:

Code-Check: view log

…er topology queries on unconfigured clusters Cherry-pick of two upstream fixes onto poc_2.6.17. milvus-io#52728 — a replicator resumes from the checkpoint the target reports, which after `restore secondary` is the position the backup was taken at. Replaying from there walks over topology changes that removed this edge before it was re-created, and those were acted on as if current: the replicator deleted its own replicate pchannel metadata and stopped moments after starting, and the same messages, forwarded to the secondary, turned it back into a standalone primary. The task's initialized checkpoint carries the time tick of the AlterReplicateConfig that created it, so anything at or before it predates the task and is now disregarded, both when deciding to remove the task and when deciding to forward the message. milvus-io#52687 — GetReplicateConfiguration never returned on a cluster that had never been given a replicate configuration, blocking until the caller's deadline. Such a cluster reports a nil configuration, and the watcher treated the resulting nil helper as "no assignment received yet". Wait only until an assignment arrives, return the possibly-nil helper, make the ConfigHelper read accessors nil-receiver safe, and reject replicate messages explicitly on a cluster with no configuration. Includes the unit tests from both upstream PRs; internal/cdc/util had no test file before. Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com>

…opology queries on unconfigured clusters (#52729) Cherry-picks onto `poc_2.6.17` for a customer running this branch, so a single image carries everything they need. Two fixes are open upstream against master (#52728, #52687); the other two are already on 2.6 and are brought over because they bear on the same area. ### #52728 — CDC stops itself on a topology change that predates the task A replicator resumes from the checkpoint the target cluster reports, which after `milvus-backup restore secondary` is the position the backup was taken at. Replaying from there walks over every topology change made since — including ones that removed this edge before it was re-created — and those were acted on as if they were current: - the replicator deleted its own `replicating-pchannel/` key and stopped 0.2 s after starting; - the same message, forwarded to the secondary, made it a standalone primary again, dropping its replicate checkpoint. Nothing recovered from it: the topology query still reported the edge (only the per-pchannel task keys were gone), and re-applying the same configuration was short-circuited as unchanged and returned success. `ReplicatePChannelMeta.initialized_checkpoint` already carries the time tick of the `AlterReplicateConfig` that created the task, so a message at or before it predates the task and cannot be an instruction for it. It is now disregarded both when deciding to remove the task and when deciding to forward the message. A task with no initialized time tick keeps the previous behaviour. ### #52687 — topology query on a cluster with no replication blocks `GetReplicateConfiguration` never returned on a cluster that had never been given a replicate configuration — the state of every freshly installed cluster. Such a cluster reports a nil configuration, and the watcher's wait condition treated the resulting nil helper as "no assignment received yet", so the call ran to the caller's deadline and surfaced as `DEADLINE_EXCEEDED`, indistinguishable from an unhealthy cluster. Wait only until an assignment has been received and return the possibly-nil helper; make the `ConfigHelper` read accessors nil-receiver safe; reject replicate messages explicitly on a cluster with no configuration, which previously could not be reached because the lookup blocked first. ### Tests Both upstream PRs' unit tests are included. `internal/cdc/util` had no test file before: - ordering check in both directions, including the no-initialized-time-tick case that keeps the old behaviour; - a current detach still removes the replication, a detach that predates the task does not; - the consume loop neither forwards a stale topology change nor ends on it. ### Also picked from 2.6 - **#51144 — recover pchannels from collection metadata** (`51749e6f8d`). streamingcoord recovery adds WAL topics missing after stats initialization by reading them back from RootCoord collection metadata. Directly relevant here: the customer reinstalled the secondary, and recovery on a rebuilt cluster is exactly this path. - **#51365 — correct the CDC replication lag metric** (`e6b5d9308d`). The replication lag series was wrong, so the one signal an operator would watch to notice replication had stopped could not be trusted. Given the failure this branch fixes is otherwise silent, the metric being correct matters. Both apply cleanly; the only conflict was two tests appended to the same file, resolved by keeping both. `#50849 — ignore stale replicated txn body` was checked and is already present on `poc_2.6.17`. `#51454 — skip unreplicable replicated ddl` is on master only and depends on message-property changes that are not on 2.6; it is left for a regular cherry-pick to 2.6 rather than being adapted here. --------- Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com> Signed-off-by: chyezh <chyezh@outlook.com> Signed-off-by: bigsheeper <yihao.dai@zilliz.com> Co-authored-by: chyezh <chyezh@outlook.com> Co-authored-by: bigsheeper <yihao.dai@zilliz.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

bigsheeper · 2026-09-07T09:42:32Z

/lgtm

liliu-z

/lgtm
/approve

sre-ci-robot · 2026-09-07T19:31:00Z

[approval-status] effective-owner-approvals=1 [liliu-z(review-comment)]; do-not-merge/disable-approve-self=not-required; do-not-merge/doc-need-two-approve=not-required; ignored=[none]

czs007 · 2026-09-08T02:46:31Z

/ci-rerun-code-check

czs007 · 2026-09-08T02:48:13Z

/ci-rerun-build-ut-cov

sre-ci-robot · 2026-09-08T02:53:07Z

❌ CI Loop Results `f30715f`

Stage	Result	Duration	Tests
❌ Build	FAILURE	0.5min	-
❌ Code-Check	SKIPPED	-	-
❌ UT-Integration	SKIPPED	-	-
❌ UT-GO	SKIPPED	-	-
❌ UT-CPP-Cov	SKIPPED	-	-

Total: 3min | Pipeline | Artifacts

Failed Test Logs:

Build: view log

A replicator resumes from the checkpoint reported by the target cluster. After `milvus-backup restore secondary` that checkpoint is the position the backup was taken at, so the replicator replays every topology change made since — including ones that removed this very edge before it was re-created. Those messages were acted on as if they were current. The replicator deleted its own replicate pchannel metadata and stopped 0.2s after starting, and the same messages were forwarded to the secondary, where a configuration that no longer lists it turns it back into a standalone primary. Replication then stopped with no error anywhere: the topology still reported the edge, because the configuration key is untouched and only the per-pchannel task keys are gone, and re-applying the same configuration is short-circuited as unchanged. Ordinary CDC catch-up never exposes this, since the checkpoint stays close to the live position and only new messages are read. The task's initialized checkpoint carries the time tick of the AlterReplicateConfig that created it, so anything at or before it predates the task itself and cannot be an instruction for it. Disregard such messages in both places they are consumed: the removal decision, and the forwarding decision in the consume loop. Tasks written by an older version carry no initialized time tick and keep the previous behaviour. Signed-off-by: Zack <noreply@zilliz.com>

czs007 · 2026-09-08T17:25:36Z

Rebased onto current master (no code change, same 4 files / +277 lines). The branch was cut on Aug 20 and its pinned conan recipes (e.g. libavrocpp, fixed on master by #52704 the next day) no longer resolve, which is why the Build stage failed in the reruns. @bigsheeper @liliu-z could you re-apply /lgtm once CI is green? Thanks.

sre-ci-robot · 2026-09-08T18:45:09Z

❌ CI Loop Results `e6f6dbc`

Stage	Result	Duration	Tests
✅ Build	SUCCESS	14.7min	-
❌ Code-Check	FAILURE	5.0min	-
❌ UT-Integration	SKIPPED	-	-
❌ UT-GO	SKIPPED	-	-
✅ UT-CPP-Cov	SUCCESS	57.6min	8827 total, 8827 passed, 0 failed

Total: 77min | Pipeline | Artifacts

Failed Test Logs:

Code-Check: view log

internal/cdc/util had no test file; the predicate that decides whether an AlterReplicateConfig removes a replication task was uncovered. The existing consume-loop test only asserts that a removal is acted on, never that a message which predates the task is left alone. Add unit tests for both directions of the ordering check, including the case where the task carries no initialized time tick and the previous behaviour is kept, and a consume-loop test asserting a stale topology change is neither forwarded to the secondary nor ends the loop. Signed-off-by: Zack <noreply@zilliz.com> Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com>

sre-ci-robot · 2026-09-08T22:37:10Z

✅ CI Loop Results `d68d4b1`

Stage	Result	Duration	Tests
✅ Build	SUCCESS	10.4min	-
✅ Code-Check	SUCCESS	5.2min	-
✅ UT-Integration	SUCCESS	26.8min	-
✅ UT-GO	SUCCESS	23.1min	-
✅ UT-CPP-Cov	SUCCESS	39.8min	8827 total, 8827 passed, 0 failed

Total: 77min | Pipeline | Artifacts

Overall Coverage: 75.2%
Diff Coverage: Go 100.0% (17 hit, 0 miss, 17 measurable lines, 22 unmeasured)
Diff Coverage HTML: view changed lines
Go Patch Warning: WARNING: Go patch coverage is partial; 22 changed lines were unmeasured.
Total Patch Coverage: 100.0% (17/17 measurable lines, 22 unmeasured)

xiaofan-luan · 2026-09-09T15:50:49Z

/lgtm
/approve

sre-ci-robot · 2026-09-09T15:51:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liliu-z, xiaofan-luan

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:

~~OWNERS~~ [liliu-z,xiaofan-luan]

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 · 2026-09-09T15:51:41Z

[approval-status] effective-owner-approvals=2 [xiaofan-luan(comment),liliu-z(review-comment)]; do-not-merge/disable-approve-self=not-required; do-not-merge/doc-need-two-approve=not-required; ignored=[none]

sre-ci-robot requested review from yanliang567 and zwd1208 August 20, 2026 16:54

sre-ci-robot added the size/L Denotes a PR that changes 100-499 lines. label Aug 20, 2026

mergify Bot added dco-passed DCO check passed. kind/bug Issues or changes related a bug labels Aug 20, 2026

czs007 mentioned this pull request Aug 20, 2026

fix: [poc_2.6.17] stop CDC acting on stale topology changes; answer topology queries on unconfigured clusters #52729

Merged

sre-ci-robot assigned bigsheeper Sep 7, 2026

sre-ci-robot added the lgtm label Sep 7, 2026

liliu-z reviewed Sep 7, 2026

View reviewed changes

sre-ci-robot assigned liliu-z Sep 7, 2026

sre-ci-robot added the approved label Sep 7, 2026

czs007 added this to the 3.0.1 milestone Sep 7, 2026

czs007 force-pushed the fix-cdc-stale-alter-replay branch from f30715f to e6f6dbc Compare September 8, 2026 17:25

sre-ci-robot removed the lgtm label Sep 8, 2026

czs007 force-pushed the fix-cdc-stale-alter-replay branch from e6f6dbc to d68d4b1 Compare September 8, 2026 21:17

mergify Bot added the ci-passed label Sep 9, 2026

yanliang567 modified the milestones: 3.0.1, 3.0.2 Sep 9, 2026

sre-ci-robot assigned xiaofan-luan Sep 9, 2026

sre-ci-robot added the lgtm label Sep 9, 2026

sre-ci-robot merged commit 063c538 into milvus-io:master Sep 9, 2026
7 of 10 checks passed

Uh oh!

Conversation

czs007 commented Aug 20, 2026

Problem

Change

Tests

Uh oh!

sre-ci-robot commented Aug 20, 2026

Uh oh!

tinswzy commented Aug 20, 2026

Low — non-blocking

Uh oh!

sre-ci-robot commented Aug 20, 2026

❌ CI Loop Results f30715f

Uh oh!

bigsheeper commented Sep 7, 2026

Uh oh!

liliu-z left a comment

Choose a reason for hiding this comment

Uh oh!

sre-ci-robot commented Sep 7, 2026

Uh oh!

czs007 commented Sep 8, 2026

Uh oh!

czs007 commented Sep 8, 2026

Uh oh!

sre-ci-robot commented Sep 8, 2026

❌ CI Loop Results f30715f

Uh oh!

czs007 commented Sep 8, 2026

Uh oh!

sre-ci-robot commented Sep 8, 2026

❌ CI Loop Results e6f6dbc

Uh oh!

sre-ci-robot commented Sep 8, 2026

✅ CI Loop Results d68d4b1

Uh oh!

xiaofan-luan commented Sep 9, 2026

Uh oh!

sre-ci-robot commented Sep 9, 2026

Uh oh!

Uh oh!

sre-ci-robot commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

❌ CI Loop Results `f30715f`

❌ CI Loop Results `f30715f`

❌ CI Loop Results `e6f6dbc`

✅ CI Loop Results `d68d4b1`