fix: Readjust the timer trigger logic for peak and off-peak time switching.

[qiuzhiqian]

Task: https://pms.uniontech.com/task-view-388291.html

[github-actions]

CLA Assistant Lite bot:
提交邮箱中包含我们的合作伙伴，但您似乎并非合作伙伴的成员或对接人，请联系相关对接人将您添加至组织之中，或由其重新发起 Pull Request。
The commit email domain belongs to one of our partners, but it seems you are not yet a member of the current organization, please contact the contact person to add you to the organization or let them submit the Pull Request.

xml seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You can retrigger this bot by commenting recheck in this Pull Request}

[deepin-ci-robot]

deepin pr auto review

代码审查报告：忙闲时切换监控流程实现

总体评价

该代码实现了对下载任务中的忙闲时状态进行监控，并在状态变化时更新限速配置的功能。代码整体结构清晰，将监控逻辑封装到了独立的 PeakOffPeakMonitor 结构体中，提高了代码的可维护性。但在一些细节上还有改进空间。

具体审查意见

1. 语法逻辑

1.1 applyOnlineRateLimit 函数

func (m *Manager) applyOnlineRateLimit(downloadSpeed *downloadSpeedLimitConfig,
    onlineRateLimit *OnlineRateLimit) {
    // ...
}

• 问题：函数签名中缺少对 onlineRateLimit 参数的注释说明
• 建议：添加函数和参数的注释说明其用途

1.2 getDownloadSpeedLimitConfigByTimeState 函数

func getDownloadSpeedLimitConfigByTimeState(m *Manager, timeState int) downloadSpeedLimitConfig {
    switch timeState {
    case timeStatePeak:
        return downloadSpeedLimitConfig{
            LimitSpeed:                strconv.Itoa(m.updatePlatform.OnlineRateLimit.PeakTimeRateLimit.Bps),
            IsOnlineSpeedLimit:        true,
            DownloadSpeedLimitEnabled: true,
        }
    // ...
    }
}

• 问题：函数没有对 m 参数为 nil 的情况进行检查
• 建议：添加 nil 检查或确保调用方不会传入 nil 值

1.3 getCurrentTimeState 函数

func (m *Manager) getCurrentTimeState(nowTime string) int {
    onlineRateLimit := m.updatePlatform.OnlineRateLimit
    if onlineRateLimit.AllDayRateLimit.Enable {
        return timeStateUnknown
    } else if isTimeInRange(nowTime, onlineRateLimit.PeakTimeRateLimit.StartTime, onlineRateLimit.PeakTimeRateLimit.EndTime) &&
        onlineRateLimit.PeakTimeRateLimit.Enable {
        return timeStatePeak
    } else if isTimeInRange(nowTime, onlineRateLimit.OffPeakTimeRateLimit.StartTime, onlineRateLimit.OffPeakTimeRateLimit.EndTime) &&
        onlineRateLimit.OffPeakTimeRateLimit.Enable {
        return timeStateOffPeak
    }
    return timeStateUnknown
}

• 问题：函数没有对 m 或 m.updatePlatform 为 nil 的情况进行检查
• 建议：添加 nil 检查或确保调用方不会传入 nil 值

2. 代码质量

2.1 常量定义

const (
    timeStateUnknown = iota
    timeStatePeak
    timeStateOffPeak
)

• 问题：常量定义缺少注释说明
• 建议：为每个常量添加注释说明其用途

2.2 PeakOffPeakMonitor 结构体

type PeakOffPeakMonitor struct {
    manager       *Manager
    done          chan struct{}
    wg            sync.WaitGroup
    startTime     time.Time
    serverTime    string
    checkInterval time.Duration
    stopped       bool
    stopMu        sync.Mutex
    lastTimeState int
}

• 问题：结构体字段缺少注释说明
• 建议：为每个字段添加注释说明其用途

2.3 错误处理

func (m *PeakOffPeakMonitor) getCurrentTime() string {
    layout := "15:04:05"
    downloadStartServiceTime, err := time.ParseInLocation(layout, m.serverTime, time.Local)
    if err != nil {
        logger.Warningf("format server time failed: %v", err)
        return time.Now().Format(layout)
    }
    now := time.Now()
    nowTime := downloadStartServiceTime.Add(now.Sub(m.startTime))
    return nowTime.Format(layout)
}

• 问题：当解析服务器时间失败时，直接返回本地时间，可能导致限速时间计算不准确
• 建议：考虑更健壮的错误处理方式，例如重试机制或使用默认值

2.4 代码注释

func (m *PeakOffPeakMonitor) refresh() {
    nowTime := m.getCurrentTime()
    currentState := m.manager.getCurrentTimeState(nowTime)

    if currentState != m.lastTimeState {
        logger.Infof("time state changed: %d -> %d, refreshing rate limit", m.lastTimeState, currentState)
        m.manager.setEffectiveOnlineRateLimit(nowTime)
        m.lastTimeState = currentState
    }
}

• 问题：缺少函数注释
• 建议：添加函数注释说明其功能和实现逻辑

3. 代码性能

3.1 字符串转换

func getDownloadSpeedLimitConfigByTimeState(m *Manager, timeState int) downloadSpeedLimitConfig {
    switch timeState {
    case timeStatePeak:
        return downloadSpeedLimitConfig{
            LimitSpeed:                strconv.Itoa(m.updatePlatform.OnlineRateLimit.PeakTimeRateLimit.Bps),
            IsOnlineSpeedLimit:        true,
            DownloadSpeedLimitEnabled: true,
        }
    // ...
    }
}

• 问题：每次调用都会进行字符串转换，可能影响性能
• 建议：考虑缓存转换结果，或者在结构体中直接存储字符串类型的限速值

3.2 时间解析

func (m *PeakOffPeakMonitor) getCurrentTime() string {
    layout := "15:04:05"
    downloadStartServiceTime, err := time.ParseInLocation(layout, m.serverTime, time.Local)
    if err != nil {
        logger.Warningf("format server time failed: %v", err)
        return time.Now().Format(layout)
    }
    now := time.Now()
    nowTime := downloadStartServiceTime.Add(now.Sub(m.startTime))
    return nowTime.Format(layout)
}

• 问题：每次调用都会解析服务器时间字符串，可能影响性能
• 建议：在初始化时解析一次服务器时间，后续只进行时间计算

4. 代码安全

4.1 并发安全

type PeakOffPeakMonitor struct {
    manager       *Manager
    done          chan struct{}
    wg            sync.WaitGroup
    startTime     time.Time
    serverTime    string
    checkInterval time.Duration
    stopped       bool
    stopMu        sync.Mutex
    lastTimeState int
}

• 问题：lastTimeState 字段在 refresh() 方法中被修改，但没有使用互斥锁保护
• 建议：使用 stopMu 或添加新的互斥锁来保护 lastTimeState 字段的访问

4.2 资源泄漏

func (m *PeakOffPeakMonitor) Stop() {
    m.stopMu.Lock()
    defer m.stopMu.Unlock()
    if m.stopped {
        return
    }
    m.stopped = true
    close(m.done)
    m.wg.Wait()
    logger.Info("peak/off-peak monitor stopped")
}

• 问题：如果 Stop() 被多次调用，可能会导致 panic（重复关闭 channel）
• 建议：虽然代码中已经有 m.stopped 检查，但可以考虑使用 sync.Once 来确保只执行一次

4.3 时间计算

func (m *PeakOffPeakMonitor) getCurrentTime() string {
    layout := "15:04:05"
    downloadStartServiceTime, err := time.ParseInLocation(layout, m.serverTime, time.Local)
    if err != nil {
        logger.Warningf("format server time failed: %v", err)
        return time.Now().Format(layout)
    }
    now := time.Now()
    nowTime := downloadStartServiceTime.Add(now.Sub(m.startTime))
    return nowTime.Format(layout)
}

• 问题：时间计算依赖于本地系统时间，如果系统时间被修改，可能导致限速时间计算不准确
• 建议：考虑使用更可靠的时间源，例如 NTP 时间或服务器时间戳

改进建议总结

1. 添加函数、结构体和常量的注释说明
2. 添加 nil 检查，确保代码健壮性
3. 优化字符串转换和时间解析的性能
4. 保护并发访问的字段，确保线程安全
5. 使用更可靠的时间源，提高时间计算的准确性
6. 考虑使用 sync.Once 来确保 Stop() 只执行一次
7. 改进错误处理，例如添加重试机制或使用默认值

代码示例

改进后的 PeakOffPeakMonitor 结构体

// PeakOffPeakMonitor 监控忙闲时状态变化，并在状态变化时更新限速配置
type PeakOffPeakMonitor struct {
    manager       *Manager       // 管理器实例
    done          chan struct{}  // 用于通知停止的通道
    wg            sync.WaitGroup // 等待 goroutine 完成的 WaitGroup
    startTime     time.Time      // 监控开始时间
    serverTime    time.Time      // 服务器时间
    checkInterval time.Duration  // 检查间隔
    stopped       bool           // 是否已停止
    stopMu        sync.Mutex     // 保护 stopped 字段的互斥锁
    lastTimeState int            // 上次的时间状态
    stateMu       sync.Mutex     // 保护 lastTimeState 字段的互斥锁
}

改进后的 getCurrentTime 方法

// getCurrentTime 获取当前服务器时间
func (m *PeakOffPeakMonitor) getCurrentTime() string {
    layout := "15:04:05"
    now := time.Now()
    // 使用已解析的服务器时间进行计算
    nowTime := m.serverTime.Add(now.Sub(m.startTime))
    return nowTime.Format(layout)
}

改进后的 refresh 方法

// refresh 检查时间状态是否变化，如果变化则更新限速配置
func (m *PeakOffPeakMonitor) refresh() {
    nowTime := m.getCurrentTime()
    currentState := m.manager.getCurrentTimeState(nowTime)

    m.stateMu.Lock()
    defer m.stateMu.Unlock()
    
    if currentState != m.lastTimeState {
        logger.Infof("time state changed: %d -> %d, refreshing rate limit", m.lastTimeState, currentState)
        m.manager.setEffectiveOnlineRateLimit(nowTime)
        m.lastTimeState = currentState
    }
}

改进后的 Stop 方法

var stopOnce sync.Once

// Stop 停止监控
func (m *PeakOffPeakMonitor) Stop() {
    stopOnce.Do(func() {
        m.stopMu.Lock()
        defer m.stopMu.Unlock()
        if m.stopped {
            return
        }
        m.stopped = true
        close(m.done)
        m.wg.Wait()
        logger.Info("peak/off-peak monitor stopped")
    })
}

这些改进可以提高代码的可读性、健壮性、性能和安全性，同时保持原有功能不变。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: qiuzhiqian, zhaohuiw42

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment