Skip to content

修正并发情况下锁的正确性 #4

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

Merged
merged 1 commit into from
Aug 9, 2024
Merged

Conversation

akkuman
Copy link
Contributor

@akkuman akkuman commented Aug 9, 2024

此处可能会有问题
有一种情况:两个实例同时下载锁文件,发现锁不存在,然后两个实例均进行锁文件上传,此时实际上两个实例均获取到了锁,但预期只能有一个获取到锁

@88250 88250 merged commit 2dee571 into siyuan-note:main Aug 9, 2024
88250 added a commit that referenced this pull request Aug 9, 2024

Verified

This commit was signed with the committer’s verified signature.
@akkuman
Copy link
Contributor Author

akkuman commented Aug 9, 2024

@88250 研究了一下,这种实现还是会有问题,只是降低了发生的概率。

在这种情况下会有问题:假设两个并发设备上锁,同时检测到锁文件不存在。
然后设备A上传了锁文件,接着下载检查发现锁是自己的,此时A拿到了锁。紧接着设备B同样上传了锁文件,下载检查发现也是自己的,此时B也拿到了锁。

估计还得再找解决方案,大概思路如下

在网上看到了一种方法:

  1. 按 lock-prefix.* 遍历文件检查是否上锁
  2. 如果无锁,则上传 lock-prefix.deviceid
  3. 再次遍历 lock-prefix.*,按时间正序,取时间最早的锁文件,检查该锁文件是否和自己的设备匹配,获取锁失败的设备,删除掉自己的锁文件
  4. 成功获得锁的设备,在临界区退出前,删除锁文件

不过这种方案也不是完全100%有效,参见 https://stackoverflow.com/a/75347123

过两天我看看实现这种方案

@88250
Copy link
Member

88250 commented Aug 9, 2024

这里锁不住问题也不是很大,因为同步上传过程中最后有一步校验 index,发现云端 index 已经变更,则会放弃当前同步

image

另外,并发同步操作的概率应该不高,因为是单人使用,同时操作多设备的可能性很低。在用户指南中也提过,最好是交替设备同步。

@88250
Copy link
Member

88250 commented Aug 9, 2024

如果这里的方案太复杂,可能回报并不高,并且很可能会带来其他问题(比如耗时增加、API 调用次数增加等)。

目前已经多调用了一次下载,并且这个调用没有加入重试(有时 S3 存储是查不到刚刚上传的文件的,特别是路径没有变化的 lock-sync,或者查到的是旧数据的问题,这可能和服务商实现的分布式存储/数据缓存有一定关系),所以这个改动可能还是有点问题的,下面截图是上传 index 后的确认过程:

image

我个人建议,为了稳定起见,这个提交最好还是回滚,找到稳妥的方案后我们再讨论看看。

@akkuman
Copy link
Contributor Author

akkuman commented Aug 9, 2024

@88250 我也觉得还是先回滚好了,目前这个实现并没有解决这个并发问题,后面index有做二次确认的话,整体应该也不会有同步问题

@88250
Copy link
Member

88250 commented Aug 9, 2024

好的,那我改回去吧。

@akkuman akkuman deleted the fix-lock branch August 9, 2024 14:40
88250 added a commit that referenced this pull request Aug 9, 2024

Verified

This commit was signed with the committer’s verified signature.
@akkuman
Copy link
Contributor Author

akkuman commented Aug 9, 2024

@88250 估计基于 s3 寻求一个完美普适的锁,并且能适配所有s3服务的,不太可能实现😂

@88250
Copy link
Member

88250 commented Aug 9, 2024

那就不研究了,这里能挡一点算一点,后面靠 index 校验,目前跑下来问题不大,我还没有收到过反馈并发覆盖问题的。

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.

None yet

2 participants