fix: address load_factor code review findings
- Fix bulk edit: send 0 instead of null/NaN to clear load_factor - Fix edit modal: explicit NaN check instead of implicit falsy - Fix create modal: use ?? instead of || for load_factor - Add load_factor upper limit validation (max 10000) - Add //go:build unit tag and self-contained intPtrHelper in test - Add design intent comments on WaitPlan.MaxConcurrency
This commit is contained in:
@@ -1,3 +1,5 @@
|
|||||||
|
//go:build unit
|
||||||
|
|
||||||
package service
|
package service
|
||||||
|
|
||||||
import (
|
import (
|
||||||
@@ -6,6 +8,8 @@ import (
|
|||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
func intPtrHelper(v int) *int { return &v }
|
||||||
|
|
||||||
func TestEffectiveLoadFactor_NilAccount(t *testing.T) {
|
func TestEffectiveLoadFactor_NilAccount(t *testing.T) {
|
||||||
var a *Account
|
var a *Account
|
||||||
require.Equal(t, 1, a.EffectiveLoadFactor())
|
require.Equal(t, 1, a.EffectiveLoadFactor())
|
||||||
@@ -22,21 +26,21 @@ func TestEffectiveLoadFactor_NilLoadFactor_ZeroConcurrency(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestEffectiveLoadFactor_PositiveLoadFactor(t *testing.T) {
|
func TestEffectiveLoadFactor_PositiveLoadFactor(t *testing.T) {
|
||||||
a := &Account{Concurrency: 5, LoadFactor: intPtr(20)}
|
a := &Account{Concurrency: 5, LoadFactor: intPtrHelper(20)}
|
||||||
require.Equal(t, 20, a.EffectiveLoadFactor())
|
require.Equal(t, 20, a.EffectiveLoadFactor())
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestEffectiveLoadFactor_ZeroLoadFactor_FallbackToConcurrency(t *testing.T) {
|
func TestEffectiveLoadFactor_ZeroLoadFactor_FallbackToConcurrency(t *testing.T) {
|
||||||
a := &Account{Concurrency: 5, LoadFactor: intPtr(0)}
|
a := &Account{Concurrency: 5, LoadFactor: intPtrHelper(0)}
|
||||||
require.Equal(t, 5, a.EffectiveLoadFactor())
|
require.Equal(t, 5, a.EffectiveLoadFactor())
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestEffectiveLoadFactor_NegativeLoadFactor_FallbackToConcurrency(t *testing.T) {
|
func TestEffectiveLoadFactor_NegativeLoadFactor_FallbackToConcurrency(t *testing.T) {
|
||||||
a := &Account{Concurrency: 3, LoadFactor: intPtr(-1)}
|
a := &Account{Concurrency: 3, LoadFactor: intPtrHelper(-1)}
|
||||||
require.Equal(t, 3, a.EffectiveLoadFactor())
|
require.Equal(t, 3, a.EffectiveLoadFactor())
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestEffectiveLoadFactor_ZeroLoadFactor_ZeroConcurrency(t *testing.T) {
|
func TestEffectiveLoadFactor_ZeroLoadFactor_ZeroConcurrency(t *testing.T) {
|
||||||
a := &Account{Concurrency: 0, LoadFactor: intPtr(0)}
|
a := &Account{Concurrency: 0, LoadFactor: intPtrHelper(0)}
|
||||||
require.Equal(t, 1, a.EffectiveLoadFactor())
|
require.Equal(t, 1, a.EffectiveLoadFactor())
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1417,6 +1417,9 @@ func (s *adminServiceImpl) CreateAccount(ctx context.Context, input *CreateAccou
|
|||||||
account.RateMultiplier = input.RateMultiplier
|
account.RateMultiplier = input.RateMultiplier
|
||||||
}
|
}
|
||||||
if input.LoadFactor != nil && *input.LoadFactor > 0 {
|
if input.LoadFactor != nil && *input.LoadFactor > 0 {
|
||||||
|
if *input.LoadFactor > 10000 {
|
||||||
|
return nil, errors.New("load_factor must be <= 10000")
|
||||||
|
}
|
||||||
account.LoadFactor = input.LoadFactor
|
account.LoadFactor = input.LoadFactor
|
||||||
}
|
}
|
||||||
if err := s.accountRepo.Create(ctx, account); err != nil {
|
if err := s.accountRepo.Create(ctx, account); err != nil {
|
||||||
@@ -1492,6 +1495,8 @@ func (s *adminServiceImpl) UpdateAccount(ctx context.Context, id int64, input *U
|
|||||||
if input.LoadFactor != nil {
|
if input.LoadFactor != nil {
|
||||||
if *input.LoadFactor <= 0 {
|
if *input.LoadFactor <= 0 {
|
||||||
account.LoadFactor = nil // 0 或负数表示清除
|
account.LoadFactor = nil // 0 或负数表示清除
|
||||||
|
} else if *input.LoadFactor > 10000 {
|
||||||
|
return nil, errors.New("load_factor must be <= 10000")
|
||||||
} else {
|
} else {
|
||||||
account.LoadFactor = input.LoadFactor
|
account.LoadFactor = input.LoadFactor
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -342,6 +342,7 @@ func (s *defaultOpenAIAccountScheduler) selectBySessionHash(
|
|||||||
}
|
}
|
||||||
|
|
||||||
cfg := s.service.schedulingConfig()
|
cfg := s.service.schedulingConfig()
|
||||||
|
// WaitPlan.MaxConcurrency 使用 Concurrency(非 EffectiveLoadFactor),因为 WaitPlan 控制的是 Redis 实际并发槽位等待。
|
||||||
if s.service.concurrencyService != nil {
|
if s.service.concurrencyService != nil {
|
||||||
return &AccountSelectionResult{
|
return &AccountSelectionResult{
|
||||||
Account: account,
|
Account: account,
|
||||||
@@ -703,6 +704,7 @@ func (s *defaultOpenAIAccountScheduler) selectByLoadBalance(
|
|||||||
}
|
}
|
||||||
|
|
||||||
cfg := s.service.schedulingConfig()
|
cfg := s.service.schedulingConfig()
|
||||||
|
// WaitPlan.MaxConcurrency 使用 Concurrency(非 EffectiveLoadFactor),因为 WaitPlan 控制的是 Redis 实际并发槽位等待。
|
||||||
candidate := selectionOrder[0]
|
candidate := selectionOrder[0]
|
||||||
return &AccountSelectionResult{
|
return &AccountSelectionResult{
|
||||||
Account: candidate.account,
|
Account: candidate.account,
|
||||||
|
|||||||
@@ -1227,7 +1227,9 @@ const buildUpdatePayload = (): Record<string, unknown> | null => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (enableLoadFactor.value) {
|
if (enableLoadFactor.value) {
|
||||||
updates.load_factor = loadFactor.value
|
// 空值/NaN/0 时发送 0(后端约定 <= 0 表示清除)
|
||||||
|
const lf = loadFactor.value
|
||||||
|
updates.load_factor = (lf != null && !Number.isNaN(lf) && lf > 0) ? lf : 0
|
||||||
}
|
}
|
||||||
|
|
||||||
if (enablePriority.value) {
|
if (enablePriority.value) {
|
||||||
|
|||||||
@@ -3491,7 +3491,7 @@ const handleImportAccessToken = async (accessTokenInput: string) => {
|
|||||||
extra: soraExtra,
|
extra: soraExtra,
|
||||||
proxy_id: form.proxy_id,
|
proxy_id: form.proxy_id,
|
||||||
concurrency: form.concurrency,
|
concurrency: form.concurrency,
|
||||||
load_factor: form.load_factor || undefined,
|
load_factor: form.load_factor ?? undefined,
|
||||||
priority: form.priority,
|
priority: form.priority,
|
||||||
rate_multiplier: form.rate_multiplier,
|
rate_multiplier: form.rate_multiplier,
|
||||||
group_ids: form.group_ids,
|
group_ids: form.group_ids,
|
||||||
@@ -3551,7 +3551,7 @@ const createAccountAndFinish = async (
|
|||||||
extra,
|
extra,
|
||||||
proxy_id: form.proxy_id,
|
proxy_id: form.proxy_id,
|
||||||
concurrency: form.concurrency,
|
concurrency: form.concurrency,
|
||||||
load_factor: form.load_factor || undefined,
|
load_factor: form.load_factor ?? undefined,
|
||||||
priority: form.priority,
|
priority: form.priority,
|
||||||
rate_multiplier: form.rate_multiplier,
|
rate_multiplier: form.rate_multiplier,
|
||||||
group_ids: form.group_ids,
|
group_ids: form.group_ids,
|
||||||
@@ -3607,7 +3607,7 @@ const handleOpenAIExchange = async (authCode: string) => {
|
|||||||
extra,
|
extra,
|
||||||
proxy_id: form.proxy_id,
|
proxy_id: form.proxy_id,
|
||||||
concurrency: form.concurrency,
|
concurrency: form.concurrency,
|
||||||
load_factor: form.load_factor || undefined,
|
load_factor: form.load_factor ?? undefined,
|
||||||
priority: form.priority,
|
priority: form.priority,
|
||||||
rate_multiplier: form.rate_multiplier,
|
rate_multiplier: form.rate_multiplier,
|
||||||
group_ids: form.group_ids,
|
group_ids: form.group_ids,
|
||||||
@@ -3637,7 +3637,7 @@ const handleOpenAIExchange = async (authCode: string) => {
|
|||||||
extra: soraExtra,
|
extra: soraExtra,
|
||||||
proxy_id: form.proxy_id,
|
proxy_id: form.proxy_id,
|
||||||
concurrency: form.concurrency,
|
concurrency: form.concurrency,
|
||||||
load_factor: form.load_factor || undefined,
|
load_factor: form.load_factor ?? undefined,
|
||||||
priority: form.priority,
|
priority: form.priority,
|
||||||
rate_multiplier: form.rate_multiplier,
|
rate_multiplier: form.rate_multiplier,
|
||||||
group_ids: form.group_ids,
|
group_ids: form.group_ids,
|
||||||
@@ -3715,7 +3715,7 @@ const handleOpenAIValidateRT = async (refreshTokenInput: string) => {
|
|||||||
extra,
|
extra,
|
||||||
proxy_id: form.proxy_id,
|
proxy_id: form.proxy_id,
|
||||||
concurrency: form.concurrency,
|
concurrency: form.concurrency,
|
||||||
load_factor: form.load_factor || undefined,
|
load_factor: form.load_factor ?? undefined,
|
||||||
priority: form.priority,
|
priority: form.priority,
|
||||||
rate_multiplier: form.rate_multiplier,
|
rate_multiplier: form.rate_multiplier,
|
||||||
group_ids: form.group_ids,
|
group_ids: form.group_ids,
|
||||||
@@ -3743,7 +3743,7 @@ const handleOpenAIValidateRT = async (refreshTokenInput: string) => {
|
|||||||
extra: soraExtra,
|
extra: soraExtra,
|
||||||
proxy_id: form.proxy_id,
|
proxy_id: form.proxy_id,
|
||||||
concurrency: form.concurrency,
|
concurrency: form.concurrency,
|
||||||
load_factor: form.load_factor || undefined,
|
load_factor: form.load_factor ?? undefined,
|
||||||
priority: form.priority,
|
priority: form.priority,
|
||||||
rate_multiplier: form.rate_multiplier,
|
rate_multiplier: form.rate_multiplier,
|
||||||
group_ids: form.group_ids,
|
group_ids: form.group_ids,
|
||||||
@@ -3832,7 +3832,7 @@ const handleSoraValidateST = async (sessionTokenInput: string) => {
|
|||||||
extra: soraExtra,
|
extra: soraExtra,
|
||||||
proxy_id: form.proxy_id,
|
proxy_id: form.proxy_id,
|
||||||
concurrency: form.concurrency,
|
concurrency: form.concurrency,
|
||||||
load_factor: form.load_factor || undefined,
|
load_factor: form.load_factor ?? undefined,
|
||||||
priority: form.priority,
|
priority: form.priority,
|
||||||
rate_multiplier: form.rate_multiplier,
|
rate_multiplier: form.rate_multiplier,
|
||||||
group_ids: form.group_ids,
|
group_ids: form.group_ids,
|
||||||
@@ -3921,7 +3921,7 @@ const handleAntigravityValidateRT = async (refreshTokenInput: string) => {
|
|||||||
extra: {},
|
extra: {},
|
||||||
proxy_id: form.proxy_id,
|
proxy_id: form.proxy_id,
|
||||||
concurrency: form.concurrency,
|
concurrency: form.concurrency,
|
||||||
load_factor: form.load_factor || undefined,
|
load_factor: form.load_factor ?? undefined,
|
||||||
priority: form.priority,
|
priority: form.priority,
|
||||||
rate_multiplier: form.rate_multiplier,
|
rate_multiplier: form.rate_multiplier,
|
||||||
group_ids: form.group_ids,
|
group_ids: form.group_ids,
|
||||||
@@ -4245,7 +4245,7 @@ const handleCookieAuth = async (sessionKey: string) => {
|
|||||||
extra,
|
extra,
|
||||||
proxy_id: form.proxy_id,
|
proxy_id: form.proxy_id,
|
||||||
concurrency: form.concurrency,
|
concurrency: form.concurrency,
|
||||||
load_factor: form.load_factor || undefined,
|
load_factor: form.load_factor ?? undefined,
|
||||||
priority: form.priority,
|
priority: form.priority,
|
||||||
rate_multiplier: form.rate_multiplier,
|
rate_multiplier: form.rate_multiplier,
|
||||||
group_ids: form.group_ids,
|
group_ids: form.group_ids,
|
||||||
|
|||||||
@@ -2064,8 +2064,9 @@ const handleSubmit = async () => {
|
|||||||
if (form.expires_at === null) {
|
if (form.expires_at === null) {
|
||||||
updatePayload.expires_at = 0
|
updatePayload.expires_at = 0
|
||||||
}
|
}
|
||||||
// load_factor: 空值/0/NaN 时发送 0(后端约定 0 = 清除)
|
// load_factor: 空值/NaN/0/负数 时发送 0(后端约定 <= 0 = 清除)
|
||||||
if (!form.load_factor || form.load_factor <= 0) {
|
const lf = form.load_factor
|
||||||
|
if (lf == null || Number.isNaN(lf) || lf <= 0) {
|
||||||
updatePayload.load_factor = 0
|
updatePayload.load_factor = 0
|
||||||
}
|
}
|
||||||
updatePayload.auto_pause_on_expired = autoPauseOnExpired.value
|
updatePayload.auto_pause_on_expired = autoPauseOnExpired.value
|
||||||
|
|||||||
Reference in New Issue
Block a user