implement distributied lock with redis cluster #284

whalesongAndLittleFish · 2021-10-26T11:25:20Z

What this PR does:

add redis cluster lock(red lock implement)

Which issue(s) this PR fixes:

Fixes #249

mosn-community-bot · 2021-10-26T11:25:23Z

Hi @whalesongAndLittleFish, welcome to mosn community, Please sign Contributor License Agreement!

After you signed CLA, we will automatically sync the status of this pull request in 3 minutes.

seeflood · 2021-10-26T14:55:28Z

Thanks for your contribution!

whalesongAndLittleFish · 2021-10-27T11:22:00Z

@seeflood 你好，我在实现的时候是并发进行的加锁和解锁的过程，请问一下需要改用采用串行加锁的方式吗。如果这两种方式都可以的话，需要改成根据配置来决定串行还是并发吗

ZLBer · 2021-10-27T11:59:30Z

@whalesongAndLittleFish 串行加锁有什么优势吗

whalesongAndLittleFish · 2021-10-27T12:26:53Z

@whalesongAndLittleFish 串行加锁有什么优势吗

官方文档提到的做法是依次对单个节点去进行加锁的操作，但为什么要依次进行没有看到解释，然后当时做的时候我觉得串行的做法会对效率有影响，所以用并发的方式做的。但是今天同事提到并发的方式可能会导致请求量较多的时候在短时间创建过多的协程，所以想问下需不需要根据不同的场景，通过参数来决定选用哪种方式。（不知道考虑的对不对，麻烦大佬们多多指教(⊙﹏⊙)）

seeflood · 2021-10-28T02:42:03Z

@whalesongAndLittleFish 如果怕短时间创建大量协程，可以通过协程池的方式优化，比如 pkg/filter/network/tcpcopy/persistence/work_pool.go 就是通过协程池做处理，不过这段代码不是通用协程池；
如果要改变算法，除了性能，关键是小心正确性哈~redlock我很久没看忘记了，你先检查下正确性，晚点我复习一下

whalesongAndLittleFish · 2021-10-28T03:23:34Z

@whalesongAndLittleFish 如果怕短时间创建大量协程，可以通过协程池的方式优化，比如 pkg/filter/network/tcpcopy/persistence/work_pool.go 就是通过协程池做处理，不过这段代码不是通用协程池；如果要改变算法，除了性能，关键是小心正确性哈~redlock我很久没看忘记了，你先检查下正确性，晚点我复习一下

okkkk，我再验证下~

nejisama · 2021-10-28T03:37:37Z

@whalesongAndLittleFish 如果怕短时间创建大量协程，可以通过协程池的方式优化，比如 pkg/filter/network/tcpcopy/persistence/work_pool.go 就是通过协程池做处理，不过这段代码不是通用协程池；如果要改变算法，除了性能，关键是小心正确性哈~redlock我很久没看忘记了，你先检查下正确性，晚点我复习一下

不要使用这个tcpcopy 当作性能优化的点，这个tcpcopy目前的性能就不太行。可以找一下其他go 实现的协程池逻辑，比如https://github.com/mosn/mosn/blob/master/pkg/sync/workerpool.go

seeflood · 2021-10-28T03:40:35Z

恩恩我拿pkg/filter/network/tcpcopy/persistence/work_pool.go举个例子，不要直接用哈~
通用协程池库有很多，比如下面这些（我没用过），或者看看@nejisama 帖的mosn里的

https://github.com/gammazero/workerpool
https://github.com/ivpusic/grpool
https://github.com/panjf2000/ants
https://github.com/Jeffail/tunny

whalesongAndLittleFish · 2021-10-28T11:01:16Z

恩恩我拿pkg/filter/network/tcpcopy/persistence/work_pool.go举个例子，不要直接用哈~ 通用协程池库有很多，比如下面这些（我没用过），或者看看@nejisama 帖的mosn里的

https://github.com/gammazero/workerpool https://github.com/ivpusic/grpool https://github.com/panjf2000/ants https://github.com/Jeffail/tunny

okk~ 我去瞅瞅

whalesongAndLittleFish · 2021-11-01T07:59:04Z

恩恩我拿pkg/filter/network/tcpcopy/persistence/work_pool.go举个例子，不要直接用哈~ 通用协程池库有很多，比如下面这些（我没用过），或者看看@nejisama 帖的mosn里的

https://github.com/gammazero/workerpool https://github.com/ivpusic/grpool https://github.com/panjf2000/ants https://github.com/Jeffail/tunny

你好，我在新的提交里尝试使用了@nejisama 推荐的https://github.com/mosn/mosn/blob/master/pkg/sync/workerpool.go
里的协程池来控制并发。然后和原算法比对，暂时没有发现正确性的问题，相比于原算法中的依次加锁记录累计时间，并发加锁计算全部完成的累计时间我认为应该不会有额外的正确性问题出现，其他流程都一致，只是执行任务、收集结果的方式不同。

codecov · 2021-11-04T09:00:46Z

Codecov Report

Merging #284 (aaa7c65) into main (6abb5ec) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #284   +/-   ##
=======================================
  Coverage   56.65%   56.65%           
=======================================
  Files          48       48           
  Lines        2058     2058           
=======================================
  Hits         1166     1166           
  Misses        769      769           
  Partials      123      123

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6abb5ec...aaa7c65. Read the comment docs.

seeflood · 2021-11-04T13:24:04Z

@ZLBer @stulzq 各位大佬有空帮忙review下这个PR么~

ZLBer · 2021-11-05T02:17:57Z

@whalesongAndLittleFish 我有个疑问，既然4个、5个节点都需要3个节点加锁成功才算整体加锁成功，为什么不维护更少的4个节点？

whalesongAndLittleFish · 2021-11-05T02:19:36Z

@whalesongAndLittleFish 我有个疑问，既然4个、5个节点都需要3个节点加锁成功才算整体加锁成功，为什么不维护更少的4个节点？

5个节点的话就能够容忍两个节点宕机呀，如果4个就只能容忍一个节点宕机了

ZLBer · 2021-11-05T02:42:55Z

@whalesongAndLittleFish 一定要限制5个及以上吗？3个？甚至1个可以把单节点的情况都覆盖了

whalesongAndLittleFish · 2021-11-05T03:13:25Z

@whalesongAndLittleFish 一定要限制5个及以上吗？3个？甚至1个可以把单节点的情况都覆盖了

实在要用三个其实也可以，但是和四个节点的情况一样只能容忍一个节点异常，最好还是五个嘛。我想的话，如果1个的话感觉可以直接使用已经实现的单点锁，就不用这个集群方案了

whalesongAndLittleFish · 2021-11-05T03:26:19Z

@whalesongAndLittleFish 一定要限制5个及以上吗？3个？甚至1个可以把单节点的情况都覆盖了

我在实现的时候没有硬性判定需要五个节点以上，节点数原则上不限制，判定成功的的规则都是过半就成功，如果真要这样使用也可以的

components/lock/redis/cluster_redis_lock.go

ZLBer · 2021-11-05T04:10:46Z

实在要用三个其实也可以，但是和四个节点的情况一样只能容忍一个节点异常，最好还是五个嘛。我想的话，如果1个的话感觉可以直接使用已经实现的单点锁，就不用这个集群方案了

所以我在想是不是两个方案可以统一起来了？我看代码里有判断五个节点啊。

whalesongAndLittleFish · 2021-11-05T05:49:14Z

实在要用三个其实也可以，但是和四个节点的情况一样只能容忍一个节点异常，最好还是五个嘛。我想的话，如果1个的话感觉可以直接使用已经实现的单点锁，就不用这个集群方案了

所以我在想是不是两个方案可以统一起来了？我看代码里有判断五个节点啊。

嗷嗷感谢提醒，我修改一下，两个方案统一的话感觉需要和他们讨论一下，我先解决下问题

whalesongAndLittleFish · 2021-11-05T06:23:46Z

实在要用三个其实也可以，但是和四个节点的情况一样只能容忍一个节点异常，最好还是五个嘛。我想的话，如果1个的话感觉可以直接使用已经实现的单点锁，就不用这个集群方案了

所以我在想是不是两个方案可以统一起来了？我看代码里有判断五个节点啊。

嗷嗷感谢提醒，我修改一下，两个方案统一的话感觉需要和他们讨论一下，我先解决下问题

已修改

components/pkg/utils/redis.go

ZLBer

LGTM

whalesongAndLittleFish · 2021-11-05T09:44:35Z

LGTM

感谢大佬的review~，辛苦了(ง •̀o•́)ง

stulzq

LGTM

components/lock/redis/cluster_redis_lock.go

configs/config_lock_redis_cluster.json

修改集群锁文档修改集群锁文档修改集群锁文档

components/lock/redis/cluster_redis_lock.go

remove unused replicas

seeflood

LGTM

add redis cluster lock,add demo client,add unit test

95ef918

mosn-community-bot bot added cla:no First-time contributor size/XL cla:yes and removed cla:no labels Oct 26, 2021

Merge branch 'main' into add_red_lock

789a9b9

WhaleSong added 2 commits November 1, 2021 15:33

use WorkerPool to control concurrency

9dc2ca2

merge origin branch

e5f6d3d

modify go.mod,go.sum

d800e9b

whalesongAndLittleFish force-pushed the add_red_lock branch from 003c2c7 to d800e9b Compare November 4, 2021 07:33

mosn-community-bot bot added size/XXL and removed size/XL labels Nov 4, 2021

WhaleSong and others added 2 commits November 4, 2021 15:41

modify go.sum

72fdabc

Merge branch 'main' into add_red_lock

2068390

WhaleSong added 2 commits November 4, 2021 17:04

add license header

ec576f1

Merge remote-tracking branch 'origin/add_red_lock' into sxx_test1

e7ba537

ZLBer reviewed Nov 5, 2021

View reviewed changes

resolve conversations

1cc4357

whalesongAndLittleFish closed this Nov 5, 2021

whalesongAndLittleFish reopened this Nov 5, 2021

ZLBer reviewed Nov 5, 2021

View reviewed changes

components/pkg/utils/redis.go Show resolved Hide resolved

add comment in cluster_redis_lock,add currency of WorkPool verifying

7e09d80

ZLBer previously approved these changes Nov 5, 2021

View reviewed changes

Merge branch 'main' into add_red_lock

cfbc104

stulzq previously approved these changes Nov 5, 2021

View reviewed changes

components/lock/redis/cluster_redis_lock.go Outdated Show resolved Hide resolved

seeflood requested changes Nov 5, 2021

View reviewed changes

configs/config_lock_redis_cluster.json Show resolved Hide resolved

seeflood dismissed stale reviews from stulzq and ZLBer via cfbc104 November 10, 2021 04:42

WhaleSong added 3 commits November 10, 2021 17:25

新增集群锁文档

64d1661

Merge remote-tracking branch 'origin/add_red_lock' into sxx_test1

d6e3868

新增集群锁文档

b0c8ab0

修改集群锁文档修改集群锁文档修改集群锁文档

whalesongAndLittleFish force-pushed the add_red_lock branch from b55d2fd to b0c8ab0 Compare November 10, 2021 09:38

MoonShining reviewed Nov 10, 2021

View reviewed changes

components/lock/redis/cluster_redis_lock.go Outdated Show resolved Hide resolved

seeflood added 2 commits November 11, 2021 10:42

Merge branch 'main' into add_red_lock

d57ea5d

Update cluster_redis_lock.go

aaa7c65

remove unused replicas

seeflood approved these changes Nov 11, 2021

View reviewed changes

seeflood merged commit 8c0d0f9 into mosn:main Nov 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement distributied lock with redis cluster #284

implement distributied lock with redis cluster #284

whalesongAndLittleFish commented Oct 26, 2021

mosn-community-bot bot commented Oct 26, 2021

seeflood commented Oct 26, 2021

whalesongAndLittleFish commented Oct 27, 2021

ZLBer commented Oct 27, 2021

whalesongAndLittleFish commented Oct 27, 2021

seeflood commented Oct 28, 2021

whalesongAndLittleFish commented Oct 28, 2021

nejisama commented Oct 28, 2021

seeflood commented Oct 28, 2021

whalesongAndLittleFish commented Oct 28, 2021

whalesongAndLittleFish commented Nov 1, 2021 •

edited

Loading

codecov bot commented Nov 4, 2021 •

edited

Loading

seeflood commented Nov 4, 2021

ZLBer commented Nov 5, 2021

whalesongAndLittleFish commented Nov 5, 2021

ZLBer commented Nov 5, 2021

whalesongAndLittleFish commented Nov 5, 2021

whalesongAndLittleFish commented Nov 5, 2021

ZLBer commented Nov 5, 2021

whalesongAndLittleFish commented Nov 5, 2021

whalesongAndLittleFish commented Nov 5, 2021

ZLBer left a comment

whalesongAndLittleFish commented Nov 5, 2021

stulzq left a comment

seeflood left a comment

implement distributied lock with redis cluster #284

implement distributied lock with redis cluster #284

Conversation

whalesongAndLittleFish commented Oct 26, 2021

mosn-community-bot bot commented Oct 26, 2021

seeflood commented Oct 26, 2021

whalesongAndLittleFish commented Oct 27, 2021

ZLBer commented Oct 27, 2021

whalesongAndLittleFish commented Oct 27, 2021

seeflood commented Oct 28, 2021

whalesongAndLittleFish commented Oct 28, 2021

nejisama commented Oct 28, 2021

seeflood commented Oct 28, 2021

whalesongAndLittleFish commented Oct 28, 2021

whalesongAndLittleFish commented Nov 1, 2021 • edited Loading

codecov bot commented Nov 4, 2021 • edited Loading

Codecov Report

seeflood commented Nov 4, 2021

ZLBer commented Nov 5, 2021

whalesongAndLittleFish commented Nov 5, 2021

ZLBer commented Nov 5, 2021

whalesongAndLittleFish commented Nov 5, 2021

whalesongAndLittleFish commented Nov 5, 2021

ZLBer commented Nov 5, 2021

whalesongAndLittleFish commented Nov 5, 2021

whalesongAndLittleFish commented Nov 5, 2021

ZLBer left a comment

Choose a reason for hiding this comment

whalesongAndLittleFish commented Nov 5, 2021

stulzq left a comment

Choose a reason for hiding this comment

seeflood left a comment

Choose a reason for hiding this comment

whalesongAndLittleFish commented Nov 1, 2021 •

edited

Loading

codecov bot commented Nov 4, 2021 •

edited

Loading