mirror of
https://github.com/milvus-io/milvus.git
synced 2026-10-28 17:05:53 +08:00
cai.zhangandClaude Fable 5
2c97f79b12
pr: #52716 issue: #52713 3.0 counterpart of #52716, carrying that PR's final state (master head `c46a81b82a`) -- the response-tag design plus every review-round fix -- replacing the earlier request-filter revision this branch previously held. The design doc lives on master: docs/design-docs/design_docs/20260824-per-resource-group-load-and-shard-leader-readiness.md. 3.0 adaptations: the delete fast path passes its streaming exec closure through the shared workload, and `query_coord.pb.go` is regenerated against 3.0's proto tree. ## What this adds Two resource groups on the same collection can legitimately sit at very different load progress -- one at 100%, another at 0% -- and the collection-wide figures average that away. This PR adds the scoped answers: - **`utils.LoadPercentageByResourceGroup`** -- how loaded a collection is, restricted to the replicas in one resource group. Five outcomes a caller must act on differently: `-1` (the group holds no replica -- terminal once the load has been registered; see the doc for the load-startup window), `0` (a replica is there but carries nothing yet, or the target set itself is empty after an ungraceful restart -- wait either way; `0` is never truncation, any progress reads at least `1`), `-1` with `ErrServiceNotReady` (the coordinator's own read stores are not wired up yet -- nothing is known, as opposed to something being known to be absent, and retriable), `-1` with `ErrCollectionNotLoaded` (the load failed terminally, cause in the message -- stop), and `-1` with `ErrResourceGroupNotFound` (the group does not exist -- an `InputError`, checked before anything about the collection, so a typo is not reported as the terminal "-1, nothing lives here"). `ErrServiceNotReady` and `ErrCollectionNotLoaded` must not be conflated: only the first is worth retrying. An empty resource group means every replica, reporting the furthest-behind one (the minimum, deliberately, rather than the collection-wide average -- a caller gating on this figure wants the laggard). It is a **live target-coverage figure**: it agrees with the collection-wide `ShowLoadCollections` number at the endpoints (-1 and 100, and on a single-partition, single-replica collection everywhere progress is at or above 1% -- below that this figure rounds up to 1 where the observer truncates to 0) but deliberately diverges in between -- it pools targets across partitions where the observer averages per partition, and it re-arms below 100 when new work (a fresh flush or compaction output) lands in the target, where the observer's persisted number stays at 100. That re-arming is why the figure is **progress, not a gate**: readiness (below) is the servability gate, `Ready` already implies every current-target segment is carried, and a caller must not AND it with `== 100` -- under ingestion `== 100` is a transient and the AND-gate may never open on a group that has been servable throughout. A recorded load failure is surfaced even after the failed load's replica records have been cleaned up. - **`utils.ShardLeaderReadinessByResourceGroup`** -- whether the replicas in one resource group can serve every shard right now, as a structured verdict (`Ready`, `Reason`, `TotalShards`, `UnreadyShards`). It does not run the collection-wide full-load gate, for any resource group -- being independent of the collection's aggregate status is the point -- so a group that has finished is not vetoed by a lagging sibling. Only query-visible replicas count as able to serve, matching the filter the routing path applies. An unknown group name is refused with `ErrResourceGroupNotFound` (`Reason = ShardLeadersReasonResourceGroupNotFound`) rather than folded into `NoReplicaInResourceGroup`; with no group named, the no-replica verdict reads `ShardLeadersReasonNoReplica`, since the reason strings are compared by callers and "in this resource group" would be false. - **`ShardLeadersList.resource_groups`** -- each leader in the `GetShardLeaders` response is tagged with the resource group of the **replica** it leads, parallel to `node_ids` / `node_addrs` / `serviceable`. - **`shardclient.FilterByResourceGroup` + `ChannelWorkload.ResourceGroup`** -- the one correct way to consume the tag, shipped with it. `selectNode` filters the candidates of **one channel** (never the channel map -- `Execute` does not cross-check the channel count against the shard number, so a dropped channel is a silently partial answer), and an empty scoped set is `ErrCollectionNotFullyLoaded` (103, retriable) after one uncached refresh, not `ErrChannelNotAvailable` (503, non-retriable): "this group's delegator for this channel is not serviceable yet" heals in seconds to minutes and the caller polls through it. `CollectionWorkLoad.ForChannel` is the only way a per-channel workload is derived -- `Execute`, `ExecuteOneChannel` and the three namespace single-shard fast paths in `task_search.go` / `task_query.go` / `task_delete.go` all go through it, so the scope reaches every path or none. Nothing on the request path sets the field yet; with it empty, routing is byte-for-byte unchanged, and the unscoped `utils` surfaces never consult the `ResourceManager` (a `Meta` without one still answers `rgName == ""`). ### Why the tag is on the response, not a filter on the request The filter cannot be applied by the caller afterwards: the response flattens every replica into one list per channel, and a replica may borrow nodes from another resource group (`num_outgoing_node` / `num_incoming_node` model exactly that), so node-set membership is not replica membership -- and the two diverge precisely during the rebalance windows this question is asked in. Only the coordinator still holds the mapping, so it has to put it on the wire. Given that, a request-side filter was implemented first and then replaced. The response tag wins on the caller this was built for: | | request filter | response tag | |---|---|---| | proxy cache | needs a resource-group dimension: `map[collID]` → `map[collID]map[rg]`, N fetches and N entries per collection | **unchanged**, single copy; the group is a per-leader attribute filtered in memory | | `InvalidateShardLeaderCache([]int64)` | semantics must be rethought | untouched | | information | one group per call | every group in one call | | old coordinator | silently ignores the field and answers **unfiltered**, in a shape the caller cannot distinguish | leaves the field **empty**, which is detectable | The proxy already consumes a per-leader boolean (`serviceable`) to build a two-tier candidate set; a per-leader string is the same shape of thing. ### What the tag does not answer It says which group a leader belongs to, not whether that group can serve. A not-yet-query-visible replica -- what `UpdateLoadConfig` spawns for a newly added group -- is filtered out before the list is built, so its group simply does not appear, and absence must not be read as "this group holds no replica". `ShardLeaderReadinessByResourceGroup` is what separates `ShardsWithoutLeader` from `NoReplicaInResourceGroup`. Likewise an empty string is **unknown**, not "no resource group", and an unknown tag never matches a named group in `FilterByResourceGroup`. Both computations live in `querycoordv2/utils` so the observers, which hold the same read-only stores and cannot import the querycoordv2 root package, can reach them; `Server` methods expose them as stable entry points and gate on `merr.CheckHealthy(s.State())` like the rest of `Server`'s surface. ## Compatibility - The proto change is additive and on the response. `utils/util.go` differs from its pre-PR state only by filling the new array; `GetShardLeaders` gains no request parameter and its gating is unchanged. - An old coordinator fills the other three arrays and leaves `resource_groups` empty. The proxy reads that as "unknown" for every leader instead of panicking on every cache refresh, and a scoped workload against unknown tags gets the retriable refusal rather than a node from another group. A non-empty but **misaligned** array (a coordinator bug) is neutralized to all-unknown for that channel with a rate-limited warning -- strictly safer than trusting tags of unknown alignment. - Nothing on the proxy's request path sets `ChannelWorkload.ResourceGroup` yet, so deployed traffic routes exactly as before; the filter and its error contract land here so the first consumer inherits them. ## Testing - `resource_group_load_percentage_test.go` -- the five outcomes including a failed load seeded with a *retriable* sentinel asserting the normalized result is neither retriable nor `ErrServiceNotReady`, an unknown group being an `InputError` while an existing-but-empty group still reads `-1`, `0` never being truncation (1 of 200 reads 1, 199 of 200 reads 99), an invisible replica counting toward the percentage while readiness says not ready, partial progress, min-across-replicas, single-partition equivalence with the collection-wide figure plus both deliberate divergences with concrete numbers, stability across target promotion, failed-load surfacing after replica cleanup, and the health gate / nil-store split. - `shard_leader_readiness_test.go` -- per-group readiness across serviceable/unserviceable leaders and unknown nodes, reason strings including the unfiltered `NoReplica` wording and the unknown-group refusal, query-invisible replicas not counting until promoted, the collection-wide gate not being inherited, failed-load surfacing after replica cleanup. - `shard_leader_rg_tag_test.go` -- the tag matching the replica rather than anything node-derived; index alignment surviving a leader dropped at the NodeManager check (how a misplaced `append` shifts every later tag); and an invisible replica's group being absent from the response while readiness still reports `ShardsWithoutLeader`. - `internal/proxy/shardclient/manager_test.go` -- the tag reaching the cache index-aligned, an old coordinator's empty `resource_groups` reading as unknown instead of panicking, and a misaligned array being neutralized rather than trusted partially. - `internal/proxy/shardclient/shard_client_test.go`, `lb_policy_test.go` -- `FilterByResourceGroup`'s contract; a scoped `selectNode` registering and choosing among that group's leaders only; an empty scoped set being `ErrCollectionNotFullyLoaded` (retriable) after one uncached refresh, also against untagged leaders; `Execute` still visiting every shard under a scope, failing the one the group cannot serve instead of skipping it; `ForChannel` carrying the scope; the exclusion recovery firing once the group's own leaders are exhausted; a fresh retriable refusal not masked by a stale non-retriable exec error (scoped only -- the unscoped request still ends on the exec error, pinned); and a scoped `ExecuteOneChannel` moving past a channel the group cannot serve, via a pre-pass on one fresh read of the whole leader table (`ShardClientMgr.GetShardLeaders`, one RPC) so a group that serves no shard is refused immediately instead of after shards x budget. Signed-off-by: cai.zhang <cai.zhang@zilliz.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>