-
-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add Lock heartbeat #820
add Lock heartbeat #820
Conversation
概述演练此拉取请求对锁定机制进行了重大改进。在 变更
序列图sequenceDiagram
participant Lock
participant Coroutine
participant Logger
Lock->>Lock: 获取锁
Lock->>Coroutine: 启动 loop() 方法
loop 心跳间隔
Coroutine->>Lock: 调用 set()
alt 成功
Lock-->>Coroutine: 返回 true
else 失败
Coroutine->>Logger: 记录错误
Coroutine->>Coroutine: 等待 5 秒
end
end
可能相关的 PR
建议的审阅者
诗歌
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lock/src/Driver/AbstractLock.php (1)
165-184
: loop() 方法实现心跳续期
该循环在协程中定期调用set()
来续期锁,并在出现异常时记录日志后等待 5 秒再继续重试,通过$isRun
来结束循环。
- 建议:为充分利用协程环境,可以考虑用
\Hyperf\Engine\Coroutine::sleep($heartbeat)
替代阻塞式的sleep()
,以免阻塞协程调度。
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/lock/src/Driver/AbstractLock.php
(7 hunks)src/lock/src/Driver/CacheLock.php
(2 hunks)src/lock/src/Driver/CoroutineLock.php
(2 hunks)src/lock/src/Driver/DatabaseLock.php
(2 hunks)src/lock/src/Driver/FileSystemLock.php
(2 hunks)src/lock/src/Driver/LockInterface.php
(2 hunks)src/lock/src/Driver/RedisLock.php
(2 hunks)
🔇 Additional comments (20)
src/lock/src/Driver/AbstractLock.php (4)
39-40
: 新增属性用于循环控制
这两行新增了 $isRun
属性,用于在循环中控制是否继续保持锁的心跳,逻辑清晰且易读。
54-57
: 抽象方法 set() 的定义
新增了抽象方法 set()
,明确将锁的续期操作留给子类去实现,保证了功能上的模块化和可扩展性。
69-76
: get() 方法中引入 heartbeat 参数
在 get()
方法中增加了一个可选的 $heartbeat
参数,如果大于 0 就调用 loop()
来持续续期锁。这样在长时间执行的任务场景下,可以避免锁被意外释放,增强了锁的可靠性。
Line range hint 93-111
: block() 方法中引入 heartbeat 参数
此处与 get()
方法逻辑相似:阻塞等待成功获取锁后,如果 $heartbeat
大于 0 则会启动心跳循环,保持锁的有效性,减少锁过期导致的竞态问题。
src/lock/src/Driver/LockInterface.php (2)
24-24
: 扩展 get() 方法的参数
在接口中增加 $heartbeat
参数,为后续心跳续期功能提供了统一的接口约定。
34-34
: 扩展 block() 方法的参数
同理,为 block()
方法添加 $heartbeat
参数,使阻塞锁同样支持心跳续期的能力。
src/lock/src/Driver/FileSystemLock.php (3)
50-58
: 实现 set() 用于文件锁续期
此处重写了抽象父类 set()
方法,借助文件系统的 store->set()
,在锁有效期内完成续期,对文件锁场景非常直观。
65-65
: 在 release() 中停用心跳循环
给 $this->isRun
设为 false
,确保锁释放后不会再执行续期操作,避免出现死循环或逻辑异常。
79-79
: 强制释放时也需要停用循环
同样需要把 $this->isRun
设置为 false
,与 release() 保持一致,防止后续继续重复续期无效锁。
src/lock/src/Driver/CacheLock.php (3)
52-57
: 新增 set() 方法满足父类抽象要求
实现了对缓存驱动的续期逻辑,通过 $this->store->set()
更新锁过期时间。
64-65
: release() 中关闭心跳
在释放锁时,将 $this->isRun
置为 false
,可以防止在锁归还后继续续期。
79-79
: forceRelease() 也需停止心跳
强制释放时直接跳过权限检查,但同样需要停止续期循环,一致性良好。
src/lock/src/Driver/RedisLock.php (3)
55-67
: set() 方法支持 Redis 的续期策略
根据 $this->seconds > 0
决定是用 set(..., ['EX'=>...])
还是 setNX
。此逻辑与 acquire() 保持一致,确保在心跳续期时采用相同策略。
74-74
: release() 禁用继续续约
释放锁时将 $this->isRun
置为 false
,然后再执行 Lua 脚本做释放,保证锁与心跳循环状态一致。
84-84
: forceRelease() 禁用心跳
强制释放锁时同样把 $this->isRun
置为 false
,可避免锁已被删除却仍续期的情况。
src/lock/src/Driver/CoroutineLock.php (2)
101-102
: 在 release 中将 $this->isRun 设为 false 的操作
该赋值动作明确表达了锁处于非活跃状态,有助于避免重复释放或其他并发冲突。
116-117
: 在 forceRelease 中同样将 $this->isRun 设为 false
此处逻辑与 release 方法保持一致,能够清晰地表明强制释放锁后其状态。实现无问题。
src/lock/src/Driver/DatabaseLock.php (3)
73-88
: set 方法实现数据库续租功能
此处通过更新数据库中的锁过期时间来实现锁续租逻辑,能满足长时间持有锁的需求。请注意在心跳或循环调用此方法时,需确保出现异常时的容错处理。
建议在测试环境模拟数据更新失败或连接中断等场景,验证锁续租的可靠性。
97-98
: 在 release 中设置 $this->isRun = false
释放锁前先标记锁状态为非运行中,逻辑清晰合理,无明显问题。
117-118
: 强制释放锁后将 isRun 置为 false
与 release 方法的行为保持一致,保证锁状态的一致性,设计正确。
#[Override] | ||
protected function set(): bool | ||
{ | ||
return false; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
建议在 set 方法中实现心跳更新逻辑
该方法目前仅返回 false
,在长时间持有锁时无法及时更新锁的过期时间,可能会导致锁意外过期。建议在此处实现心跳/续租逻辑,以确保在持锁期间动态刷新过期时间。
是否需要帮您编写一个基于定时器或循环的续租逻辑来更新锁的过期时间?
执行时间超过锁的有效期,应该用wait包一层,抛出超时异常吧。 |
调整后,添加心跳; 锁的有效期意义有限, 会定期延长 |
我明白你调整是为了自动延长锁的有效期,但是原本锁的有效期就是为了给定一个有效的时间,超过了就自动释放锁。按你的思路,把说的时间设置为一个更大的值就可以了,执行完马上释放锁,效果跟你想要的一致。 |
设置一个更大的值面临一些问题: 锁被意外清除,loop方法 可以及时加锁 |
从概念上,你的需求是Mutex |
加锁的目的就是 Mutex 的 😂 |
我给你一个方案,你看看是不是能满足你的需求。 class MyRedisLock extends RedisLock
{
protected static array $timerIds = [];
protected static ?Timer = null;
public function acquire(): bool
{
self::$timer ??= new Timer();
self::$timerIds[$this->name] = self::$timer->tick(1000, fn() => /* loop 的逻辑 */);
return parent::acquire();
}
public function release(): bool
{
self::$timer?->clear(self::$timerIds[$this->name] ?? 0);
return parent::release();
}
public function forceRelease(): void
{
self::$timer?->clear(self::$timerIds[$this->name] ?? 0);
parent::forceRelease();
}
} 通过AOP实现也可以。 |
方案都可以,这个是一个不错的功能。 |
这样调整改变了原有的功能,是一个大BC了。 |
OK, 希望下一个大版本能支持 |
增加心跳heartbeat参数;
防止任务执行过长,锁失效
Summary by CodeRabbit
新功能
方法变更
get()
和block()
方法的签名,增加了可选的心跳参数set()
方法改进