fix: auto 分组跨分组重试用优先级数量替代 RetryTimes 判断跳组 - #4226
Closed
zuiho-kai wants to merge 4 commits into
Closed
Conversation
Contributor
WalkthroughAdded channel-exclusion support and a group-model priority-count API to channel cache; reworked auto cross-group retry logic to use per-group priority counts and preserve global retry state; propagate excluded channel IDs through selection and use_channel tracking; added forced affinity update API and context-aware affinity recording. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Relay
participant Selector
participant Cache
participant Channels
Client->>Relay: Request(model, params, context)
Relay->>Selector: Build RetryParam (Include ExcludeChannelIDs from use_channel)
Selector->>Cache: GetGroupModelPriorityCount(group, model)
Cache-->>Selector: priorityCount
Selector->>Channels: GetRandomSatisfiedChannel(group, model, priorityRetry, excludeIDs...)
Channels-->>Selector: channel / nil
alt channel returned
Selector->>Relay: channel
Relay->>service: (on success) ForceUpdateChannelAffinity if needed
Relay->>Client: response
else nil returned
Selector->>Selector: update ContextKeyAutoGroupIndex / ContextKeyAutoGroupRetryIndex (per-group logic)
Selector->>Channels: try next group...
Selector->>Client: channel / error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
model/channel_cache.go (1)
201-215: Consider sharing this priority-resolution logic withGetRandomSatisfiedChannel.This helper now duplicates the same model fallback and unique-priority scan from
GetRandomSatisfiedChannel(Lines 105-142). Since the new cross-group switch threshold has to stay identical to the actual selection path, extracting a private shared helper would reduce the risk of those two behaviors drifting apart later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model/channel_cache.go` around lines 201 - 215, The priority-resolution code in this block duplicates the model-name fallback and unique-priority counting logic used by GetRandomSatisfiedChannel; extract that logic into a single private helper (e.g., computeUniquePriorityCount or getUniquePrioritiesForModel) that takes the group, modelName, group2model2channels, and channelsIDM, performs the normalized model fallback (using ratio_setting.FormatMatchingModelName), iterates channels to build the uniquePriorities map from channel.GetPriority(), and returns the count; replace the duplicated code in both this function and GetRandomSatisfiedChannel with calls to the new helper so both paths share identical behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@service/channel_select.go`:
- Around line 117-126: The code currently calls param.SetRetry(0) when switching
auto-groups which mutates the global Retry counter (RetryParam.Retry) used by
controller/relay.go and can exceed the global retry budget; instead, stop
resetting Retry, store the group's starting retry index in context via
common.SetContextKey(param.Ctx, constant.ContextKeyAutoGroupRetryIndex,
param.GetRetry()) and keep setting ContextKeyAutoGroupIndex as now; then compute
priorityRetry as param.GetRetry() - startRetryIndex when checking against
model.GetGroupModelPriorityCount(autoGroup, param.ModelName). Apply the same
change to the other group-transition branch (the channel == nil branch) so no
code resets the global retry counter and all group transitions preserve the
outer retry budget.
---
Nitpick comments:
In `@model/channel_cache.go`:
- Around line 201-215: The priority-resolution code in this block duplicates the
model-name fallback and unique-priority counting logic used by
GetRandomSatisfiedChannel; extract that logic into a single private helper
(e.g., computeUniquePriorityCount or getUniquePrioritiesForModel) that takes the
group, modelName, group2model2channels, and channelsIDM, performs the normalized
model fallback (using ratio_setting.FormatMatchingModelName), iterates channels
to build the uniquePriorities map from channel.GetPriority(), and returns the
count; replace the duplicated code in both this function and
GetRandomSatisfiedChannel with calls to the new helper so both paths share
identical behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 168b33ad-7fae-4062-8213-88d369053937
📒 Files selected for processing (2)
model/channel_cache.goservice/channel_select.go
按 coderabbitai review 建议修改： - 移除所有 param.SetRetry(0)，全局 Retry 始终递增 - 跳组时将当前 Retry 存入 ContextKeyAutoGroupRetryIndex - priorityRetry = param.GetRetry() - startRetryIndex - 移除不再需要的 ResetRetryNextTry() 调用
- GetRandomSatisfiedChannel 加 excludeIDs variadic 参数，retry 时跳过已失败渠道 - RetryParam 新增 ExcludeChannelIDs，relay retry loop 每次迭代传入已用渠道列表 - 新增 ForceUpdateChannelAffinity，retry 成功时主动把亲和缓存更新到成功渠道 - distributor c.Next() 后从 context 读最终 channel ID，避免用初始渠道覆盖亲和缓存
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controller/relay.go`:
- Line 192: Relay() correctly sets retryParam.ExcludeChannelIDs =
getUsedChannelIDs(c) but RelayTask() still uses an empty ExcludeChannelIDs when
retrying, so update RelayTask (or the retry loop that calls it) to populate
RetryParam.ExcludeChannelIDs with the same getUsedChannelIDs(c) value before
retrying; specifically locate the RelayTask function and the retry loop that
appends "use_channel" and ensure the RetryParam passed into or constructed
inside RelayTask carries the getUsedChannelIDs(c) exclusion list so retries
honor the same excluded channels.
In `@service/channel_select.go`:
- Around line 122-126: The retry/exhaustion logic uses the original priority
count and priorityRetry which can be wrong after ExcludeChannelIDs filtering;
update the code so that the exhaustion check and the index math use the
post-exclusion priority set produced by model.GetRandomSatisfiedChannel (the
uniquePriorities rebuilt after ExcludeChannelIDs) — i.e., obtain the filtered
priority list/count (or determine the target priority before applying
exclusions) and then compare priorityRetry against that filtered count and
compute the selected priority from that filtered list; apply the same change for
the other occurrences that use priorityRetry/GetGroupModelPriorityCount (the
blocks around the other matches you noted) so all checks use the post-filtered
uniquePriorities instead of the unfiltered GetGroupModelPriorityCount.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: abdfb03e-344c-4fef-ad36-83be04ef2b9a
📒 Files selected for processing (5)
controller/relay.gomiddleware/distributor.gomodel/channel_cache.goservice/channel_affinity.goservice/channel_select.go
🚧 Files skipped from review as they are similar to previous changes (1)
- model/channel_cache.go
|
|
||
| for ; retryParam.GetRetry() <= common.RetryTimes; retryParam.IncreaseRetry() { | ||
| relayInfo.RetryIndex = retryParam.GetRetry() | ||
| retryParam.ExcludeChannelIDs = getUsedChannelIDs(c) |
Contributor
There was a problem hiding this comment.
The exclusion list is only applied to one of the two retry loops.
Relay() now forwards getUsedChannelIDs(c), but RelayTask() below still retries with an empty RetryParam.ExcludeChannelIDs even though it also appends to "use_channel". That means task submissions can keep bouncing back to the same failed channel, so this retry fix is only partially wired in.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/relay.go` at line 192, Relay() correctly sets
retryParam.ExcludeChannelIDs = getUsedChannelIDs(c) but RelayTask() still uses
an empty ExcludeChannelIDs when retrying, so update RelayTask (or the retry loop
that calls it) to populate RetryParam.ExcludeChannelIDs with the same
getUsedChannelIDs(c) value before retrying; specifically locate the RelayTask
function and the retry loop that appends "use_channel" and ensure the RetryParam
passed into or constructed inside RelayTask carries the getUsedChannelIDs(c)
exclusion list so retries honor the same excluded channels.
Comment on lines
+122
to
+126
| // 跨分组重试时，检查当前分组的优先级是否已耗尽 | ||
| // 如果 priorityRetry >= 该分组的优先级数量，说明已经没有新的优先级可用，应跳到下一个分组 | ||
| if crossGroupRetry && priorityRetry > 0 { | ||
| priorityCount := model.GetGroupModelPriorityCount(autoGroup, param.ModelName) | ||
| if priorityCount > 0 && priorityRetry >= priorityCount { |
Contributor
There was a problem hiding this comment.
Excluded channels can shift retries onto the wrong priority.
model.GetRandomSatisfiedChannel rebuilds uniquePriorities after applying ExcludeChannelIDs (see model/channel_cache.go:118-151), but this code still uses the unadjusted priorityRetry and the unfiltered GetGroupModelPriorityCount(...). If the failed channel was the only member of a higher priority, the next retry can skip the next remaining priority entirely and the group-switch threshold no longer matches the actually selectable priorities. Please base both the retry index and the exhaustion check on the post-exclusion priority set, or determine the target priority before filtering excluded channel IDs.
Also applies to: 136-136, 149-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@service/channel_select.go` around lines 122 - 126, The retry/exhaustion logic
uses the original priority count and priorityRetry which can be wrong after
ExcludeChannelIDs filtering; update the code so that the exhaustion check and
the index math use the post-exclusion priority set produced by
model.GetRandomSatisfiedChannel (the uniquePriorities rebuilt after
ExcludeChannelIDs) — i.e., obtain the filtered priority list/count (or determine
the target priority before applying exclusions) and then compare priorityRetry
against that filtered count and compute the selected priority from that filtered
list; apply the same change for the other occurrences that use
priorityRetry/GetGroupModelPriorityCount (the blocks around the other matches
you noted) so all checks use the post-filtered uniquePriorities instead of the
unfiltered GetGroupModelPriorityCount.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 变更描述 / Description
修复 auto 分组「跨分组重试」不生效的问题。
原逻辑用全局
RetryTimes判断是否该跳组：但
GetRandomSatisfiedChannel在retry >= len(uniquePriorities)时会 clamp 到最后一个优先级，仍然返回 channel。当分组内该模型的优先级数量远小于RetryTimes时，所有重试都耗在同一个分组反复选同一个渠道，永远不会触发跳组。修复方案：
model/channel_cache.go— 新增GetGroupModelPriorityCount函数，查询分组内某模型的实际优先级数量service/channel_select.go— 用实际优先级数量替代RetryTimes判断跳组时机：每个优先级用完后立刻跳到下一个分组修复后行为：如果分组内只有 1 个优先级（如只有 1 个渠道有该模型），失败 1 次就跳到下一个分组；如果有 3 个优先级，用完 3 个优先级后跳组。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
📸 运行证明 / Proof of Work
场景验证（分组内只有 1 个渠道有该模型，priorityCount=1）：
修复前：
3->3->3->3->3->3->3->3（7 次重试全打在同一个渠道）修复后：
3->下一分组渠道->...（失败 1 次即跳组）Summary by CodeRabbit
New Features
Refactor
Bug Fixes