refactor: improve server mutex implementation and configuration #1019
Conversation
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
- Change server mutex prefix from APP_ENV to APP_NAME for better identification - Remove readonly properties from Consumer class to allow dependency injection flexibility - Improve RedisServerMutex constructor with better name construction and default owner handling - Add getName() method to ServerMutexInterface for consistency - Create ServerMutexCommand for mutex management operations This refactor improves the server mutex functionality by: - Using application name instead of environment for mutex prefix - Making Consumer properties mutable for better DI container compatibility - Enhancing RedisServerMutex with proper name formatting and owner handling - Providing interface consistency with getName() method
|
Warning Rate limit exceeded@huangdijia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 2 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Walkthrough新增服务器互斥相关命令与接口方法，调整互斥锁命名来源与 Redis 实现构造逻辑，并将 Consumer 的若干只读属性改为可变以延迟初始化健康监控与服务器互斥实例。 Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (ServerMutexCommand)
participant Container as Container
participant Config as Config
participant Redis as Redis
participant Consumer as Consumer
participant Mutex as RedisServerMutex
CLI->>Container: resolve Command
CLI->>Config: 读取 connections 配置
CLI->>Redis: 请求锁信息（list） / 发送释放请求（release）
Redis-->>CLI: 返回锁条目 / 确认释放
note right of Consumer#Mutex: Consumer 构造时（配置驱动）
Consumer->>Config: 检查 health_monitor.enable
alt enabled
Consumer->>Consumer: 创建 HealthMonitor 实例
else
Consumer->>Consumer: healthMonitor = null
end
Consumer->>Config: 检查 server_mutex.enable
alt enabled
Consumer->>Mutex: new RedisServerMutex(prefix, connection, owner, options, logger)
Mutex-->>Consumer: 返回实例 (提供 getName())
else
Consumer->>Consumer: serverMutex = null
end
Estimated code review effort🎯 3 (中等) | ⏱️ ~25 分钟 需额外关注：
Possibly related PRs
Suggested reviewers
诗
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/trigger/src/Mutex/ServerMutexInterface.php (1)
14-21: 接口新增抽象方法构成明确的 BC 变更，建议补充文档或调整方案已验证以下事实：
接口性质：
ServerMutexInterface无@internal注解，是公开 API，设计用于第三方扩展内部实现：唯一的框架内实现
RedisServerMutex已包含getName(): string方法，内部不存在破坏BC 影响：任何在用户代码中自定义实现该接口的类升级后都将触发致命错误，这确实是一个 BC 变更
版本信息：项目采用 ~3.1.0 版本（patch/minor），无 major 版本跳跃；CHANGELOG-3.1.md 最新条目（v3.1.73）未记录此变更
建议采取以下任一方案：
- 在 CHANGELOG 中明确标记此为 BC 变更，并提供迁移指引
- 或创建
NamedServerMutexInterface extends ServerMutexInterface，仅在内部使用新接口- 或给
ServerMutexInterface添加@internal注解，明确其非公开 API- 或考虑发布 major 版本以承载此 BC 变更
🧹 Nitpick comments (4)
src/trigger/publish/trigger.php (1)
32-38:server_mutex.prefix从 APP_ENV 切到 APP_NAME，Redis key 命名语义有明显变化结合新的
RedisServerMutex::__construct()实现（$this->name = ($options['prefix'] ?? 'trigger:mutex:') . $this->connection;）：
- 当前配置下，完整的 mutex key 形如：
{APP_NAME}:{connection}（例如myapp:default）。- 如果不配置
prefix，则退化为默认：trigger:mutex:{connection}。这相对旧版以
APP_ENV为前缀的方案，是一次实际的 key 重命名；线上升级后旧 key 会自然过期，但如果有依赖原有命名约定的脚本/监控，需要同步调整或在变更说明中明确指出。如果希望 key 同时包含 App 名称和固定前缀，也可以考虑在配置中改为例如：'prefix' => env('APP_NAME', 'trigger') . ':trigger:mutex:',请确认当前的命名策略就是预期行为。
src/trigger/src/Mutex/RedisServerMutex.php (1)
34-53:RedisServerMutex的 name 构造逻辑已固定为prefix + connection，需确认是否满足所有场景当前构造函数中：
$this->name = sprintf( '%s%s', $options['prefix'] ?? 'trigger:mutex:', $this->connection ); $this->owner = $owner ?? Util::getInternalIp(); $this->timer = new Timer($this->logger);以及新增加的：
public function getName(): string { return $this->name; }含义是：
- 最终 Redis key 严格等于
prefix + connection；- 不再支持通过 options 传入单独的“name”概念（如果旧版有这种用法，则现在只能通过调整
prefix或connection来区分）；- 当
$connection === null时，key 退化为单纯的prefix，这在当前由Consumer统一传递 connection 的前提下是安全的，但如果后续在其他地方直接 newRedisServerMutex时需要注意。建议：
- 在注释或文档中明确：mutex 的唯一性由
{prefix}{connection}决定，getName()返回的就是 Redis key 本身；- 如果仍有“同一 connection 下多实例”的需求，可以考虑在后续版本中引入额外的可选后缀（如基于
$owner或显式 name），以避免直接在业务代码里拼装复杂的 prefix。整体实现本身是合理的，只是命名语义的变化需要在使用方/文档层面达成共识。
Also applies to: 55-58
src/trigger/src/Consumer.php (2)
31-51: 将核心依赖和标识字段从readonly改为可变，会削弱 Consumer 的不变量这里把多处属性从
readonly改成了普通可变属性：public Config $config; public string $name; public string $identifier; public ?HealthMonitor $healthMonitor = null; public ?ServerMutexInterface $serverMutex = null; public BinLogCurrentSnapshotInterface $binLogCurrentSnapshot; public function __construct( protected SubscriberManager $subscriberManager, protected TriggerManager $triggerManager, public string $connection = 'default', array $options = [], public ?LoggerInterface $logger = null ) { ... }这样做确实提高了 DI / 运行时注入的灵活性，但也意味着：
$connection、$name、$identifier等现在可以在运行期被任意修改，而这几个字段又被用于日志、server mutex 名称、Coordinator key 等，修改后容易出现状态不一致；- 之前依赖“构造后不变”的调用方（包括库内部代码）可能会默认认为这些属性不会被修改。
如果只是为了构造时通过容器注入参数，其实保留
readonly也不影响 DI；只有在确实需要运行期重写这些属性时才有必要放开。建议：
- 明确是否真的有运行时重写这些字段的需求；如无，考虑对最关键的几个标识（比如
$connection、$identifier）继续保持readonly；- 如果决定保留可变设计，可以在文档或类注释中说明“这些属性理论上可变，但不推荐在运行时修改”，避免后续误用。
58-69: HealthMonitor / ServerMutex 的初始化逻辑整体合理，但可以考虑异常降级与配置语义新逻辑：
if ($this->config->get('health_monitor.enable', true)) { $this->healthMonitor = make(HealthMonitor::class, ['consumer' => $this]); } if ($this->config->get('server_mutex.enable', true)) { $this->serverMutex = make(ServerMutexInterface::class, [ 'connection' => $this->connection, 'owner' => Util::getInternalIp(), 'options' => (array) $this->config->get('server_mutex', []), 'logger' => $this->logger, ]); }优点：
- 通过
health_monitor.enable/server_mutex.enable显式控制是否启用，对配置友好；ServerMutexInterface的实例化参数与新的RedisServerMutex构造函数匹配良好（connection + options + logger）。可以再考虑的点：
Util::getInternalIp()如果在某些环境下无法获取内网 IP（例如容器内部 DNS 异常），会抛出异常并中断整个Consumer构造。根据业务期望：
- 如果“拿不到 IP 就必须失败”，当前行为是OK的；
- 如果更希望“拿不到 IP 时仅禁用 server mutex，但仍允许消费”，可以在这里捕获异常、记录日志后跳过 mutex 初始化。
ServerMutexInterface在容器中的绑定需要保证构造参数名与这里传入的 key 一致（connection、owner、options、logger），否则会注入失败。建议在绑定处保持与这里的签名同步，或者在变更时同步更新。整体 wiring 没有明显逻辑问题，只是上述两个点可以根据具体部署环境和兼容性要求再评估一下。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/trigger/publish/trigger.php(1 hunks)src/trigger/src/Command/ServerMutexCommand.php(1 hunks)src/trigger/src/Consumer.php(1 hunks)src/trigger/src/Mutex/RedisServerMutex.php(1 hunks)src/trigger/src/Mutex/ServerMutexInterface.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/trigger/src/Mutex/ServerMutexInterface.php (2)
src/trigger/src/Consumer.php (1)
getName(138-141)src/trigger/src/Mutex/RedisServerMutex.php (1)
getName(55-58)
src/trigger/src/Command/ServerMutexCommand.php (1)
src/trigger/src/Command/ConsumeCommand.php (1)
Command(25-87)
src/trigger/src/Mutex/RedisServerMutex.php (3)
src/trigger/src/Consumer.php (2)
__construct(45-70)getName(138-141)src/trigger/src/Util.php (2)
Util(16-40)getInternalIp(21-39)src/trigger/src/Mutex/ServerMutexInterface.php (1)
getName(16-16)
src/trigger/src/Consumer.php (5)
src/trigger/src/Config.php (3)
Config(16-31)__construct(18-20)get(22-25)src/trigger/src/Monitor/HealthMonitor.php (2)
HealthMonitor(23-92)__construct(29-32)src/trigger/src/Mutex/RedisServerMutex.php (1)
__construct(36-53)src/trigger/src/Snapshot/RedisBinLogCurrentSnapshot.php (2)
__construct(25-30)get(38-63)src/trigger/src/Util.php (2)
Util(16-40)getInternalIp(21-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/trigger/src/Command/ServerMutexCommand.php (1)
22-29: 构造函数中return parent::__construct()导致静态分析错误，且命令注册方式建议对齐生态用法
- 构造函数问题（静态分析已报错）
- 在 PHP 中构造函数不应显式
return，parent::__construct()返回void，当前写法既违反语言规范，也触发了静态分析报错（GitHub Check 提示的两个错误）。- 建议仅调用父类构造函数而不返回，同时直接传入命令名称以对齐 Hyperf 命令常用写法：
- public function __construct(protected ContainerInterface $container) - { - return parent::__construct(); - } + public function __construct(protected ContainerInterface $container) + { + // 与 Hyperf 命令示例保持一致，显式设置命令名称 + parent::__construct('trigger:server-mutex'); + }这样可以消除当前静态分析失败，并与 Hyperf 官方/社区文档中的模式保持一致。
- 命令注册方式（延续之前评审的关注点）
- 当前类上仍然没有
#[Command]特性，而 FriendsOfHyperf 生态及 Hyperf 官方示例通常采用 Attribute 方式注册命令（见FriendsOfHyperf\CommandSignals、CommandValidation示例）。- 如果本组件依赖扫描 Attribute 来注册命令，那么缺少
#[Command]可能导致trigger:server-mutex实际并未被框架发现。- 如未通过 ConfigProvider 明确把
ServerMutexCommand::class放入commands列表，建议补充 Attribute：+use Hyperf\Command\Annotation\Command; + - class ServerMutexCommand extends \Hyperf\Command\Command +#[Command] +class ServerMutexCommand extends \Hyperf\Command\Command如你已经在 ConfigProvider 中手动注册了该命令，可以忽略此建议，但建议至少在 PR 描述或配置中保证这一点。
（此条与之前评审意见“缺少 #[Command] 属性可能无法注册命令”相同，属于重复提醒。）
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/trigger/src/Command/ServerMutexCommand.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/trigger/src/Command/ServerMutexCommand.php (3)
src/trigger/src/Mutex/RedisServerMutex.php (2)
__construct(36-53)release(113-119)src/trigger/src/Consumer.php (1)
__construct(45-70)src/trigger/src/Mutex/ServerMutexInterface.php (1)
release(20-20)
🪛 GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
src/trigger/src/Command/ServerMutexCommand.php
[failure] 28-28:
Result of method Hyperf\Command\Command::__construct() (void) is used.
[failure] 28-28:
Method FriendsOfHyperf\Trigger\Command\ServerMutexCommand::__construct() with return type void returns null but should not return anything.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/trigger/src/Command/ServerMutexCommand.php (2)
70-73: 保持参数传递的一致性与
list操作相同，此处的make()调用也缺少完整参数，建议保持与Consumer一致的参数形式。应用以下修改：
$options = $config->get("trigger.connections.{$connection}.server_mutex", []); $mutex = make(ServerMutexInterface::class, [ 'connection' => $connection, - 'options' => $options, + 'options' => (array) $options, + 'owner' => null, + 'logger' => null, ]);虽然
release(true)使用了强制释放标志，不依赖 owner 匹配，但保持参数传递的一致性有助于代码维护。
44-47: 互斥锁构造参数不完整，与 Consumer 模式不一致当前
make()调用仅传递了connection和options参数，但缺少关键参数：
- 缺少
owner参数：导致RedisServerMutex回退使用命令进程的 IP（Util::getInternalIp()），而非实际持有互斥锁的 consumer 进程 IP，这会导致列出的 owner 信息不准确- 缺少
logger参数：无法记录操作日志- 未对
options进行数组类型转换：可能引发类型不匹配问题参考
Consumer.php（第 62-67 行）的正确模式。建议修改为：
->transform(function ($config, $connection) use ($redis) { $mutex = make(ServerMutexInterface::class, [ 'connection' => $connection, - 'options' => $config['server_mutex'] ?? [], + 'options' => (array) ($config['server_mutex'] ?? []), + 'owner' => null, // 用于查询现有互斥锁，无需指定 owner + 'logger' => null, ]);注意：对于
list操作，owner应设为null，因为我们是在查询 Redis 中已存在的互斥锁信息，而不是创建新的互斥锁实例。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/trigger/src/Command/ServerMutexCommand.php(1 hunks)src/trigger/src/Consumer.php(1 hunks)src/trigger/src/Mutex/RedisServerMutex.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/trigger/src/Consumer.php
🧰 Additional context used
🧬 Code graph analysis (2)
src/trigger/src/Mutex/RedisServerMutex.php (3)
src/trigger/src/Consumer.php (2)
__construct(45-70)getName(138-141)src/trigger/src/Util.php (2)
Util(16-40)getInternalIp(21-39)src/trigger/src/Mutex/ServerMutexInterface.php (1)
getName(16-16)
src/trigger/src/Command/ServerMutexCommand.php (3)
src/trigger/src/Consumer.php (2)
__construct(45-70)getName(138-141)src/trigger/src/Mutex/RedisServerMutex.php (3)
__construct(36-53)getName(55-58)release(113-119)src/trigger/src/Mutex/ServerMutexInterface.php (2)
getName(16-16)release(20-20)
🔇 Additional comments (2)
src/trigger/src/Mutex/RedisServerMutex.php (1)
55-58: LGTM！
getName()方法的实现正确，满足了ServerMutexInterface的新增契约要求，使得互斥锁名称的访问更加统一和安全。src/trigger/src/Command/ServerMutexCommand.php (1)
40-42: 确认默认启用行为是否符合预期第 41 行使用
?? true作为enable标志的默认值，这意味着配置中缺少该键时，将默认启用互斥锁并包含在列表中。这与Consumer.php（第 61 行）的行为一致，但可能导致列出未明确配置互斥锁的连接。请确认此行为是否符合预期。如果希望仅列出显式启用互斥锁的连接，应使用：
->reject(function ($config, $connection) { - return ! ($config['server_mutex']['enable'] ?? true); + return ! ($config['server_mutex']['enable'] ?? false); })
huangdijia
added a commit
that referenced
this pull request
Nov 19, 2025
* refactor: improve server mutex implementation and configuration - Change server mutex prefix from APP_ENV to APP_NAME for better identification - Remove readonly properties from Consumer class to allow dependency injection flexibility - Improve RedisServerMutex constructor with better name construction and default owner handling - Add getName() method to ServerMutexInterface for consistency - Create ServerMutexCommand for mutex management operations This refactor improves the server mutex functionality by: - Using application name instead of environment for mutex prefix - Making Consumer properties mutable for better DI container compatibility - Enhancing RedisServerMutex with proper name formatting and owner handling - Providing interface consistency with getName() method * refactor: enhance ServerMutexCommand for improved action handling and configuration * refactor: update ServerMutexCommand and RedisServerMutex for improved clarity and functionality * Update * refactor: improve server mutex filtering logic to handle missing configuration * refactor: add ServerMutexCommand to the command list in ConfigProvider * refactor: improve variable naming in ServerMutexCommand for clarity * refactor: set default value for connection in RedisServerMutex constructor * refactor: update RedisServerMutex constructor to conditionally set options * refactor: move owner assignment in RedisServerMutex constructor for clarity --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
This PR improves the server mutex implementation in the trigger component with better configuration handling and dependency injection flexibility.
Changes
APP_ENVtoAPP_NAMEfor better identificationgetName()method for interface consistencyServerMutexCommandfor mutex management operationsBenefits
Testing
The changes maintain backward compatibility while improving the internal implementation. The server mutex functionality should work as before but with better configuration options.
Summary by CodeRabbit
发布说明
新功能
改进