fix: 修复删除交易员后排行榜仍显示已删除交易员的问题

[songzhihui]

问题原因：

• 删除交易员时只从数据库删除，未从 TraderManager 内存 map 移除
• 导致竞赛接口从内存获取数据时仍包含已删除的交易员

修复方案：

• 新增 TraderManager.RemoveTrader 方法，从内存 map 移除交易员
• 删除时自动停止运行中的交易员并清空竞赛数据缓存
• 修改 handleDeleteTrader 调用 RemoveTrader 确保内存和数据库一致

[github-actions]

🤖 Advisory Check Results

These are advisory checks to help improve code quality. They won't block your PR from being merged.

📋 PR Information

Title Format: ✅ Good - Follows Conventional Commits
PR Size: 🟢 Small (138 lines: +112 -26)

🔧 Backend Checks

Go Formatting: ⚠️ Needs formatting

Files needing formatting

api/server.go
decision/engine.go
logger/telegram_sender.go
mcp/client.go
trader/aster_trader.go
trader/auto_trader.go
trader/binance_futures.go
trader/hyperliquid_trader.go

Go Vet: ✅ Good
Tests: ✅ Passed

Fix locally:

go fmt ./...      # Format code
go vet ./...      # Check for issues
go test ./...     # Run tests

⚛️ Frontend Checks

Build & Type Check: ✅ Success

Fix locally:

cd web
npm run build  # Test build (includes type checking)

---

📖 Resources

• Contributing Guidelines
• Migration Guide

Questions? Feel free to ask in the comments! 🙏

---

These checks are advisory and won't block your PR from being merged. This comment is automatically generated from pr-checks-run.yml.

[xqliu]

代码审查报告 - PR #622

✅ 审查结果: 通过

📋 基本信息

• PR标题: fix: 修复删除交易员后排行榜仍显示已删除交易员的问题
• 作者: @songzhihui
• 类型: 🐛 Bug fix
• 影响: 修复排行榜缓存同步问题

🎯 问题与解决方案

问题: 删除trader后排行榜仍显示已删除trader（缓存未清理）
解决: 删除trader时清理对应的排行榜缓存

✅ 评估

维度	评分
业务逻辑	⭐⭐⭐⭐⭐
技术实现	⭐⭐⭐⭐
用户体验	⭐⭐⭐⭐⭐

🔧 建议

• ✅ 修复缓存一致性问题
• ✅ 用户体验改进
• 建议: 考虑使用更细粒度的缓存key（只删除特定trader，而非清空整个缓存）

---

审查者: Claude AI Code Reviewer

[xqliu]

代码审查报告 - PR #622 (完整详细版)

📋 基本信息

• PR标题: fix: 修复删除交易员后排行榜仍显示已删除交易员的问题
• 作者: @songzhihui
• 类型: 🐛 Bug fix
• 修改文件: manager/trader_manager.go (68行新增, 7行删除), api/server.go (43行新增, 19行删除), web/src/components/AITradersPage.tsx (1行新增)

---

1️⃣ 业务逻辑审查

✅ 问题定义

核心Bug: 删除trader后，排行榜仍然显示已删除的trader

问题根源:

当前流程:
1. 用户点击删除trader
2. handleDeleteTrader() 只删除数据库记录
3. TraderManager内存map中仍保留trader实例
4. 竞赛接口从内存map获取数据 → 包含已删除trader

导致: 排行榜显示"幽灵trader"（数据库中不存在，但内存中仍有）

实际影响:

场景: 用户删除表现不佳的trader
结果: 
- 数据库已删除 ✅
- Web UI我的Trader列表已消失 ✅
- 排行榜仍然显示该trader ❌（用户困惑）
- 重启服务器后排行榜才正常 ❌（临时解决方案）

✅ 解决方案验证

修复策略: 内存和数据库双向同步删除

新增方法: TraderManager.RemoveTrader()
功能:
1. 停止运行中的trader
2. 从内存map删除trader实例
3. 清空竞赛数据缓存

调用链:
handleDeleteTrader() 
  → database.DeleteTrader() (删除数据库)
  → traderManager.RemoveTrader() (删除内存) ← 新增

业务逻辑正确性: ✅ 完全解决问题

• 数据一致性: 数据库和内存同步删除
• 缓存失效: 清空竞赛缓存，强制重新查询
• 资源释放: 停止运行中的trader，释放goroutine

---

2️⃣ 技术实现审查

✅ 核心修改1: 新增RemoveTrader方法

// manager/trader_manager.go:428
func (tm *TraderManager) RemoveTrader(traderID string) error {
    tm.mu.Lock()
    defer tm.mu.Unlock()

    // 1. 检查trader是否存在
    trader, exists := tm.traders[traderID]
    if !exists {
        return fmt.Errorf("交易员不存在: %s", traderID)
    }

    // 2. 停止运行中的trader
    status := trader.GetStatus()
    if isRunning, ok := status["is_running"].(bool); ok && isRunning {
        trader.Stop()
        log.Printf("⏹  已停止运行中的交易员: %s", traderID)
    }

    // 3. 从map中删除
    delete(tm.traders, traderID)
    log.Printf("✓ 已从管理器中移除交易员: %s", traderID)

    // 4. 清空竞赛数据缓存
    tm.competitionCache.mu.Lock()
    tm.competitionCache.data = make(map[string]interface{})
    tm.competitionCache.timestamp = time.Time{}
    tm.competitionCache.mu.Unlock()
    log.Printf("🗑️  已清空竞赛数据缓存")

    return nil
}

技术亮点:

1. 线程安全: 使用tm.mu.Lock()保护map操作
2. 优雅停止: 先调用trader.Stop()，再删除实例
3. 缓存失效: 清空竞赛缓存，确保下次查询重新获取
4. 错误处理: 检查trader存在性，返回明确错误

评价: ✅ 实现正确，逻辑清晰

⚠️ 潜在问题: 缓存策略过于激进

当前实现:

// 清空整个竞赛数据缓存
tm.competitionCache.data = make(map[string]interface{})
tm.competitionCache.timestamp = time.Time{}

问题分析:

• 删除1个trader时，清空所有trader的竞赛缓存
• 影响所有用户（包括未删除trader的用户）
• 下次查询排行榜时，需要重新计算所有trader数据（性能开销）

建议优化:

// 只删除特定trader的缓存数据（如果缓存支持）
if data, ok := tm.competitionCache.data.(map[string]interface{}); ok {
    delete(data, traderID)  // 只删除这个trader
    log.Printf("🗑️  已清除trader %s 的竞赛缓存", traderID)
}

取舍: 当前实现简单粗暴但有效，如果删除操作不频繁，性能影响可接受

✅ 核心修改2: 更新handleDeleteTrader

修复前代码（不完整）:

// api/server.go:725
// 如果交易员正在运行，先停止它
if trader, err := s.traderManager.GetTrader(traderID); err == nil {
    status := trader.GetStatus()
    if isRunning, ok := status["is_running"].(bool); ok && isRunning {
        trader.Stop()  // ❌ 停止了，但没有从map删除
        log.Printf("⏹  已停止运行中的交易员: %s", traderID)
    }
}

// 只删除数据库，不删除内存
err := s.database.DeleteTrader(userID, traderID)

修复后代码（完整）:

// api/server.go:754
// 从 TraderManager 中移除交易员（会自动停止运行中的交易员并清空竞赛缓存）
if err := s.traderManager.RemoveTrader(traderID); err != nil {
    log.Printf("⚠️ 从管理器中移除交易员失败: %v", err)
    // 不影响返回结果，因为数据库删除已成功
}

改进点:

1. 逻辑封装: 所有清理逻辑封装在RemoveTrader中
2. 错误容错: 即使内存删除失败，也不影响API返回（数据库已删除）
3. 代码简化: 从14行减少到5行

✅ 附加修改: LoadUserTraders重载逻辑

新增功能: 支持重新加载已存在的trader（热更新配置）

// manager/trader_manager.go:862
if existingTrader, exists := tm.traders[traderCfg.ID]; exists {
    // 交易员已存在，需要更新配置
    log.Printf("🔄 交易员 %s 已加载，将重新加载新配置", traderCfg.Name)

    // 1. 检查是否正在运行
    status := existingTrader.GetStatus()
    wasRunning := false
    if isRunning, ok := status["is_running"].(bool); ok && isRunning {
        wasRunning = true
        existingTrader.Stop()
    }

    // 2. 移除旧实例
    delete(tm.traders, traderCfg.ID)

    // 3. 加载新配置
    err = tm.loadSingleTrader(...)
    if err != nil {
        log.Printf("⚠️ 重新加载交易员 %s 失败: %v", traderCfg.Name, err)
        continue
    }

    // 4. 如果之前在运行，重新启动
    if wasRunning {
        if newTrader, ok := tm.traders[traderCfg.ID]; ok {
            go newTrader.Run()
            log.Printf("▶️  已重新启动交易员: %s", traderCfg.Name)
        }
    }
}

用途: 配置更新时不需要重启服务器（热重载）

评价: ✅ 优秀的附加功能，提升用户体验

---

3️⃣ 端到端验证

✅ 核心场景测试

场景1: 删除停止状态的trader

初始状态:
- Trader A (停止状态)
- Trader B (运行中)
- 排行榜显示: [A, B]

操作: 删除Trader A
1. handleDeleteTrader调用
2. database.DeleteTrader() → DB删除 ✅
3. traderManager.RemoveTrader(A) 
   → trader.Stop()（已停止，跳过）
   → delete(tm.traders, A) ✅
   → 清空竞赛缓存 ✅

结果:
- 数据库: 只有B ✅
- 内存map: 只有B ✅
- 排行榜: 只显示B ✅

场景2: 删除运行中的trader

初始状态:
- Trader A (运行中)
- Trader B (运行中)

操作: 删除Trader A
1. handleDeleteTrader调用
2. database.DeleteTrader() → DB删除 ✅
3. traderManager.RemoveTrader(A)
   → trader.Stop() → 停止goroutine ✅
   → delete(tm.traders, A) ✅
   → 清空竞赛缓存 ✅

结果:
- Trader A的goroutine已退出（无资源泄漏）✅
- 内存map中无Trader A ✅
- 排行榜只显示B ✅

场景3: 删除不存在的trader

操作: 删除不存在的Trader X
1. database.DeleteTrader(X) → 返回错误"trader不存在"
2. handleDeleteTrader返回404 ✅
3. RemoveTrader未被调用（因为数据库删除失败）

结果: 正确处理，不会崩溃 ✅

场景4: 排行榜缓存失效

初始状态:
- 竞赛缓存有效期内（5分钟）
- 缓存包含Trader A和B的数据

操作: 删除Trader A
1. RemoveTrader清空缓存
2. 用户查询排行榜
3. 缓存已失效 → 重新从数据库查询
4. 查询结果只有B（A已从DB删除）

结果: 排行榜立即更新 ✅

✅ 边界条件

边界1: 并发删除

场景: 两个用户同时删除同一个trader

线程1: RemoveTrader(A)
  → tm.mu.Lock() → 获取锁
  → delete(tm.traders, A)
  → tm.mu.Unlock()

线程2: RemoveTrader(A)
  → tm.mu.Lock() → 等待锁
  → trader不存在 → 返回错误 ✅

结果: 第二个请求返回错误，但不影响系统稳定性 ✅

边界2: 删除后立即查询

场景: 删除trader后立即查询排行榜

操作顺序:
1. DELETE /api/traders/A
2. 立即GET /api/competition

潜在问题: 竞赛接口缓存未失效？
✅ 已解决: RemoveTrader会清空缓存

---

4️⃣ 安全审查

✅ 并发安全

• 读写锁: ✅ tm.mu.Lock()保护map操作
• 竞赛缓存锁: ✅ tm.competitionCache.mu.Lock()保护缓存清空
• 双重锁定: ⚠️ 注意死锁风险（但当前实现无嵌套锁，安全）

✅ 资源释放

• Goroutine泄漏: ✅ trader.Stop()确保goroutine退出
• 内存泄漏: ✅ delete(tm.traders, traderID)释放map引用
• 数据库连接: ✅ trader停止时会关闭相关连接

---

5️⃣ 代码质量

✅ 优点

1. 问题定位准确: 清楚识别内存和数据库不一致问题
2. 解决方案完整: 同时处理数据库、内存、缓存三个层面
3. 代码封装: 逻辑封装在RemoveTrader方法中，易于复用
4. 日志完善: 每个关键步骤都有日志输出

⚠️ 建议改进

1. 优化缓存失效策略（性能优化）

// 当前: 清空所有缓存（简单但低效）
tm.competitionCache.data = make(map[string]interface{})

// 建议: 只删除特定trader（如果缓存结构支持）
// 方案A: 使缓存过期但不清空
tm.competitionCache.timestamp = time.Time{}

// 方案B: 细粒度删除（需要修改缓存结构）
if cacheData, ok := tm.competitionCache.data.([]TraderStats); ok {
    // 过滤掉删除的trader
    newData := make([]TraderStats, 0)
    for _, stats := range cacheData {
        if stats.TraderID != traderID {
            newData = append(newData, stats)
        }
    }
    tm.competitionCache.data = newData
}

2. 添加事务处理（数据一致性增强）

// api/server.go:handleDeleteTrader（建议增强）
func (s *Server) handleDeleteTrader(c *gin.Context) {
    // ... 前置检查 ...

    // 1. 先从内存删除（可回滚）
    err := s.traderManager.RemoveTrader(traderID)
    if err != nil {
        c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("移除交易员失败: %v", err)})
        return
    }

    // 2. 再删除数据库
    err = s.database.DeleteTrader(userID, traderID)
    if err != nil {
        // 数据库删除失败，回滚内存操作
        log.Printf("⚠️ 数据库删除失败，尝试回滚: %v", err)
        // TODO: 重新加载trader到内存（回滚）
        c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("删除交易员失败: %v", err)})
        return
    }

    c.JSON(http.StatusOK, gin.H{"message": "交易员已删除"})
}

注意: 当前实现是"先DB后内存"，失败不回滚（问题不大，因为重启会自动同步）

3. 添加单元测试

// manager/trader_manager_test.go（建议新增）
func TestRemoveTrader(t *testing.T) {
    tm := NewTraderManager()
    
    // 添加测试trader
    trader := &MockTrader{id: "test123"}
    tm.traders["test123"] = trader
    
    // 测试删除
    err := tm.RemoveTrader("test123")
    assert.NoError(t, err)
    
    // 验证已删除
    _, exists := tm.traders["test123"]
    assert.False(t, exists)
    
    // 验证缓存已清空
    assert.Empty(t, tm.competitionCache.data)
}

func TestRemoveTrader_NotExists(t *testing.T) {
    tm := NewTraderManager()
    
    err := tm.RemoveTrader("nonexistent")
    assert.Error(t, err)
    assert.Contains(t, err.Error(), "交易员不存在")
}

---

📊 总结

✅ 审查结果: 通过（建议优化缓存策略）

维度	评分	说明
业务逻辑	⭐⭐⭐⭐⭐	完全解决排行榜显示"幽灵trader"问题
技术实现	⭐⭐⭐⭐	逻辑正确，但缓存清空过于激进
并发安全	⭐⭐⭐⭐⭐	使用锁保护，无并发问题
代码质量	⭐⭐⭐⭐	封装良好，建议添加测试

🎯 核心价值

1. 数据一致性: 数据库和内存双向同步删除
2. 用户体验: 删除后排行榜立即更新（无需重启）
3. 资源管理: 优雅停止trader，释放goroutine

🔧 行动建议

必须修复: 无
强烈建议:

1. 优化缓存失效策略（只失效而非清空，或细粒度删除）
2. 添加单元测试验证RemoveTrader逻辑

可选优化:

1. 添加事务处理（先删内存，失败可回滚）
2. 添加Prometheus指标监控trader删除频率
3. 考虑软删除（标记为deleted而非物理删除）

---

🌟 特别表扬

问题定位: 准确识别内存和数据库不一致的根本原因
解决方案: 完整覆盖数据库、内存、缓存三个层面
代码封装: RemoveTrader方法设计合理，逻辑清晰

附加功能: LoadUserTraders支持热重载（优秀的附加价值）

---

总评: 这是一个高质量的Bug修复，彻底解决排行榜数据不一致问题。代码实现正确、线程安全、资源管理到位。缓存清空策略可以优化，但当前实现简单有效。建议合并后添加单元测试，并考虑优化缓存策略以提升性能。

---

审查时间: 2025-11-08
审查者: Claude AI Code Reviewer