Skip to content

Conversation

@fly602
Copy link
Contributor

@fly602 fly602 commented Sep 24, 2025

The resource control module (cgroup.go) has been deleted as it was deprecated and no longer in use. This module previously handled CPU and memory resource allocation through cgroups but has been superseded by newer implementations. The removal includes:

  1. Cgroup path management functions
  2. Configuration loading from JSON
  3. Task management and cgroup setup logic
  4. Periodic cleanup of unused cgroups

The deletion reflects ongoing system cleanup and modernization efforts, removing legacy code that could potentially cause confusion or maintenance overhead. No replacement functionality is needed as this was handled at a different system level.

Influence:

  1. Verify system resource management still functions as expected
  2. Check for any processes that might have relied on this cgroup management
  3. Ensure no configuration files at /etc/deepin/daemon/resource- control.json are still being read
  4. Monitor system performance to confirm no unexpected behavior

chore: 移除废弃的资源控制模块

资源控制模块(cgroup.go)已被删除,因为该模块已废弃不再使用。该模块之前通
过cgroups处理CPU和内存资源分配,但已被新的实现所取代。删除内容包括:

  1. Cgroup路径管理功能
  2. 从JSON加载配置
  3. 任务管理和cgroup设置逻辑
  4. 定期清理未使用的cgroups

此次删除反映了系统持续清理和现代化的努力,移除了可能导致混淆或维护负担的
遗留代码。不需要替代功能,因为这已在不同的系统层面处理。

Influence:

  1. 验证系统资源管理是否仍按预期工作
  2. 检查是否有进程可能依赖于此cgroup管理
  3. 确保/etc/deepin/daemon/resource-control.json的配置文件不再被读取
  4. 监控系统性能以确认没有意外行为

Summary by Sourcery

Chores:

  • Delete cgroup.go and all related cgroup path management, JSON configuration loading, task management, and cleanup logic

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602

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

Needs approval from an approver in each of these files:

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

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 24, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR deletes the deprecated cgroup-based resource control module, removing all associated path management, configuration loading, task setup, and cleanup logic as part of legacy code cleanup.

File-Level Changes

Change Details Files
Remove deprecated resource control module
  • Deleted Cgroup path management functions
  • Removed JSON configuration loading logic
  • Removed task management and cgroup setup routines
  • Removed periodic cleanup of unused cgroups
system/resource_ctl/cgroup.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

The resource control module (cgroup.go) has been deleted as it was
deprecated and no longer in use. This module previously handled CPU and
memory resource allocation through cgroups but has been superseded by
newer implementations. The removal includes:
1. Cgroup path management functions
2. Configuration loading from JSON
3. Task management and cgroup setup logic
4. Periodic cleanup of unused cgroups

The deletion reflects ongoing system cleanup and modernization
efforts, removing legacy code that could potentially cause confusion or
maintenance overhead. No replacement functionality is needed as this was
handled at a different system level.

Influence:
1. Verify system resource management still functions as expected
2. Check for any processes that might have relied on this cgroup
management
3. Ensure no configuration files at /etc/deepin/daemon/resource-
control.json are still being read
4. Monitor system performance to confirm no unexpected behavior

chore: 移除废弃的资源控制模块

资源控制模块(cgroup.go)已被删除,因为该模块已废弃不再使用。该模块之前通
过cgroups处理CPU和内存资源分配,但已被新的实现所取代。删除内容包括:
1. Cgroup路径管理功能
2. 从JSON加载配置
3. 任务管理和cgroup设置逻辑
4. 定期清理未使用的cgroups

此次删除反映了系统持续清理和现代化的努力,移除了可能导致混淆或维护负担的
遗留代码。不需要替代功能,因为这已在不同的系统层面处理。

Influence:
1. 验证系统资源管理是否仍按预期工作
2. 检查是否有进程可能依赖于此cgroup管理
3. 确保/etc/deepin/daemon/resource-control.json的配置文件不再被读取
4. 监控系统性能以确认没有意外行为
@deepin-ci-robot
Copy link

deepin pr auto review

根据提供的git diff和代码文件,我发现这是对resource_ctl模块的移除操作。我将从代码质量、性能和安全性等方面对被删除的代码进行分析和评价:

1. 代码质量

优点:

  • 代码结构清晰,有合理的模块划分和职责分离
  • 使用了defer进行错误处理,提高了代码的健壮性
  • 有适当的日志记录,便于问题追踪

不足:

  • 错误处理不够完善,部分函数仅记录错误但未采取恢复措施
  • 缺乏详细的注释,特别是对cgroup操作的解释
  • 配置文件路径硬编码,缺乏灵活性

2. 性能

优点:

  • 使用了定时器进行定期检查,避免频繁的文件系统操作
  • 使用了map来跟踪需要管理的cgroup,避免重复处理

不足:

  • 定时器间隔固定为3秒,可能不够灵活
  • 文件系统操作较为频繁,可能影响性能
  • 每次扫描都进行完整的目录遍历,效率较低

3. 安全性

优点:

  • 文件操作使用了适当的权限设置
  • 对输入参数进行了基本的验证

不足:

  • 缺乏对配置文件内容的校验
  • 文件操作没有进行充分的权限检查
  • cgroup操作缺乏错误恢复机制
  • 正则表达式可能存在安全风险,没有进行充分的输入验证

改进建议

  1. 代码质量改进:

    • 增加更详细的错误处理机制
    • 添加更多注释说明cgroup操作的目的
    • 将配置路径改为可配置的,提高灵活性
    • 添加单元测试,确保代码质量
  2. 性能优化:

    • 使用更灵活的定时机制,根据系统负载动态调整检查频率
    • 实现增量更新机制,避免每次都完整遍历目录
    • 考虑使用缓存机制减少重复的文件系统操作
  3. 安全性增强:

    • 增加配置文件的格式和内容验证
    • 添加更严格的权限检查
    • 实现错误恢复机制
    • 对正则表达式进行安全加固
    • 添加资源使用的监控和限制,防止资源耗尽
  4. 其他改进:

    • 实现优雅的停止机制,确保资源正确释放
    • 添加性能监控指标,便于优化
    • 考虑使用更现代的cgroup API,如v2版本

总的来说,这个resource_ctl模块虽然实现了基本的功能,但在代码质量、性能和安全性方面还有较大的改进空间。如果需要重新实现这个功能,建议参考上述改进建议进行优化。

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Sep 25, 2025

TAG Bot

New tag: 6.1.57
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #920

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Oct 10, 2025

TAG Bot

New tag: 6.1.58
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #924

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Oct 16, 2025

TAG Bot

New tag: 6.1.59
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #934

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Oct 30, 2025

TAG Bot

New tag: 6.1.62
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #951

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants