Skip to content
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

关于优化Lock::lock的建议 #516

Open
Tracked by #589
wiilruo opened this issue May 13, 2023 · 1 comment
Open
Tracked by #589

关于优化Lock::lock的建议 #516

wiilruo opened this issue May 13, 2023 · 1 comment
Labels
2.1 3.0 Plan version 3.0 enhancement New feature or request

Comments

@wiilruo
Copy link
Contributor

wiilruo commented May 13, 2023

  • 你遇到了什么问题、建议:

目前Lock::lock的第一个参数ID 是给予static::getInstance的lockConfigId
这样的操作有一定误导性。
在实际业务中,可能为存在很多动态的lockId,不可能为大量动态的lockId来写对应的Lock实现类。

因此提出2中修改意见:
第一种:建议修改将id参数给与static::getInstance的lockId参数,并给Lock::lock增加第四个参数$lockConfigId。

image image

第二种: 修改getInstance逻辑,将lockConfigId不存在时的逻辑进行优化,服于默认实现
image

image

实际业务使用示例:
image

  • 请执行下面的命令获取环境信息。

php -v & php --ri swoole & composer info | grep -a imi

# 粘贴到这里

  • 如果可以,提供最小可复现代码:
    第一种修改代码实现:
 /**
     * 加锁,会挂起协程.
     *
     * @param callable|null $taskCallable      加锁后执行的任务,可为空;如果不为空,则执行完后自动解锁
     * @param callable|null $afterLockCallable 当获得锁后执行的回调,只有当 $taskCallable 不为 null 时有效。该回调返回 true 则不执行 $taskCallable
     */
    public static function lock(?string $id = null, ?callable $taskCallable = null, ?callable $afterLockCallable = null, ?string $lockConfigId = null): bool
    {
        return static::getInstance($lockConfigId, $id)->lock($taskCallable, $afterLockCallable);
    }

第二种修改代码实现:

/**
     * 获取锁对象
     */
    public static function getInstance(?string $lockConfigId = null, ?string $lockId = null): ILockHandler
    {
        if (!self::$inited) {
            self::init();
        }
        if (!$lockConfigId) {
            $lockConfigId = static::getDefaultId();
        }
        $instances = &self::$instances;
        if (null === $lockId && isset($instances[$lockConfigId])) {
            return $instances[$lockConfigId];
        }
        $options = &self::$options;
        if (!isset($options[$lockConfigId])) {
            $defaultId = static::getDefaultId();
            if (!isset($options[$defaultId])) {
                throw new \RuntimeException(sprintf('Lock %s does not exists, has no default lock config', $lockConfigId));
            }
            $lockId = $lockConfigId;
            $option = $options[$defaultId];
        } else {
            $option = $options[$lockConfigId];
        }

        if (null === $lockId) {
            return $instances[$lockConfigId] = App::newInstance($option->class, $lockConfigId, $option->options);
        } else {
            return App::newInstance($option->class, $lockId, $option->options);
        }
    }
@Yurunsoft
Copy link
Member

第一种修改会破坏兼容性,造成BC,可以考虑在 3.0 里实现。

第二种修改逻辑和参数名含义冲突,不规范,不考虑采纳。

2.1 里可以做的修改就是给 lock 方法增加第4个参数 $lockId,第一个参数改名$lockConfigId,保持原有含义。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.1 3.0 Plan version 3.0 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants