sofastack / sofa-jraft

A production-grade java implementation of RAFT consensus algorithm.
https://www.sofastack.tech/projects/sofa-jraft/
Apache License 2.0
3.56k stars 1.14k forks source link

当2000个客户端并发压测调用cliService.getPeers()时,触发重新选主 #992

Closed zhipingu closed 1 year ago

zhipingu commented 1 year ago

当2000个客户端并发压测调用cliService.getPeers()时,触发重新选主,但是raft processor的默认executor:Bolt-default-executor线程池也才20个线程,没有达到最大值400,。

原raft leader报steps down when alive nodes don't satisfy quorum, term=xxx deadnodes=xxx, conf=xxxx, onLeaderStop:status=Status[ERAFTTIMEDOUT<10001>]:Mojority of the group dies:3/5]

所有raft节点日志报fail to get leader of group xxxx, Unknow leader 还有ERAFTTIMEDOUT<10011>:Lost connection from leader xxxx

fengjiachun commented 1 year ago

晕倒,压测 cliService.getPeers() 是什么鬼?压它干什么

fengjiachun commented 1 year ago

我不太理解这个行为,我怀疑你在故意气我 :), CliService 请先要理解它是做什么的,如果你这的有场景需要频繁调用 getpeers,那么请在上层做个缓存,否则你频繁调用 getpeers 导致 Node 获取读锁, 会阻塞掉获取写锁的行为

zhipingu commented 1 year ago

我不太理解这个行为,我怀疑你在故意气我 :), CliService 请先要理解它是做什么的,如果你这的有场景需要频繁调用 getpeers,那么请在上层做个缓存,否则你频繁调用 getpeers 导致 Node 获取读锁, 会阻塞掉获取写锁的行为

哈哈,不是故意气你呢!现在2000多个客户端同时启动去获取集群列表,调用了服务端这个接口触发的,不是为了故意压getpeers()

fengjiachun commented 1 year ago

否则你频繁调用 getpeers 导致 Node 获取读锁, 会阻塞掉获取写锁的行为

这句话的意思是, jraft core 的 NodeImpl 实现 raft 的主要流程的逻辑全部实在 write lock 保护内执行的, get_peers 占用了 read lock, 两者是互斥的

funky-eyes commented 1 year ago

@fengjiachun 我觉得扩容几百上千个client节点是有可能的,这个时候就会出现并发的获取peers,是不是得想办法解决下这种情况,可能会影响应用扩容效率,并且还可能影响raft节点导致重选举

funky-eyes commented 1 year ago

我不太理解为什么一定要加读锁才去读conf里的peers,是否可以变成双检锁的形式?这部分数据强一致的要求高吗?读锁都被占着没释放,写锁获取不了就尴尬了

fengjiachun commented 1 year ago

可以画个架构的草图,我理解一下这个场景

killme2008 commented 1 year ago

你们可以自己在服务端对 getPeers 做个 cache 的,然后封装一个接口提供给客户端,不建议直接暴露 jraft 的接口。

funky-eyes commented 1 year ago

你们可以自己在服务端对 getPeers 做个 cache 的,然后封装一个接口提供给客户端,不建议直接暴露 jraft 的接口。

我是这样做的,我们在server封装了一个listener,当状态机里的通知被勾起的时候再通知client,但是避免不了业务应用启动的时候需要先获取一次集群信息,即便有缓存,这里还是涉及到缓存一致性问题,所以我在想getpeers是否真的要求一致性高,本身我看了实现是new ArrayList<>(this.peers); 网络本身就不确定,即便加了锁,返回了当前最新最正确的peers也可能在返回过程中到发请求给leader时集群变更了 node#listPeers的实现我觉得判断当前节点是否是leader我觉得有必要,但是下边的this.conf.getConf().listPeers(); 感觉并不需要上锁,每次peer变更都是copyonwrite方式,新建一个新的conf来替换,其中的peers只涉及到遍历,没有修改(过滤处理后放入新的list),新的list再进行remove,再替换node实例中的conf,也就是在这个remove动作的时候不存在有其它线程进入拿到一个可变的listpeers

funky-eyes commented 1 year ago

可以画个架构的草图,我理解一下这个场景

可以理解为扩容后,client节点需要获取集群元数据,也就是集群成员列表,对leader发出rpc请求,这是启动后必做的

fengjiachun commented 1 year ago

可以画个架构的草图,我理解一下这个场景

可以理解为扩容后,client节点需要获取集群元数据,也就是集群成员列表,对leader发出rpc请求,这是启动后必做的

我是比较怀疑是否有必要对 2000 客户端暴露这个服务

fengjiachun commented 1 year ago

node#listPeers的实现我觉得判断当前节点是否是leader我觉得有必要,但是下边的this.conf.getConf().listPeers(); 感觉并不需要上锁,

应该是想简单了, conf 本身不是并发安全的,conf 是在写锁里更新的,如果读之前不获取读锁,那么就没有 happen-before 的关系,就不保证可见性,指令重排等问题

不是不能改,只是改起来没那么简单

funky-eyes commented 1 year ago

node#listPeers的实现我觉得判断当前节点是否是leader我觉得有必要,但是下边的this.conf.getConf().listPeers(); 感觉并不需要上锁,

应该是想简单了, conf 本身不是并发安全的,conf 是在写锁里更新的,如果读之前不获取读锁,那么就没有 happen-before 的关系,就不保证可见性,指令重排等问题

不是不能改,只是改起来没那么简单

conf写锁更新这个没问题,也就是list是每次写锁获取后采用的copyonwrite替换,我如果不加锁,无非得到的是上一次的conf,因为没有happen-before 的关系,但是可见性没问题的,也没设计指令重拍,因为这些地方通通不涉及到list的读取加修改的动作,只获取一个conf list,就是在getpeers时,另一个地方在操作的list并不是getpeers的list实例,或者另一个地方操作完了,

 this.id = id;
        this.conf = conf;
        this.oldConf = oldConf;

的时候,可见性有问题嘛,我觉得只要在unsafeRegisterConfChange和start两者之间进行的getpeers都没有线程安全问题(除非要强一致,但是这样到client本身就有延迟,没必要),可以加一个atomicboolean,判断只要leader没有进行unsafeRegisterConfChange(可能出现节点变更),和还没完全启动,这个时候需要拿锁,其余感觉可以直接给出去list,因为你们的意思都可以让业务做一个中间层缓存这些数据本身就存在一致性问题

killme2008 commented 1 year ago

这个事情根本没这么复杂,getPeers 得到的就是一个瞬时状态,本来就不是强一致的。你在自己的服务里,定期在服务端调用下 getPeers ,将结果 cache 下,利用 listener 来主动更新,结合一个 TTL,就可以达到很好的效果,客户端 getPeers 就走这个 cache 即可。

fengjiachun commented 1 year ago

我如果不加锁,无非得到的是上一次的conf

你可能得到一个没有构建完全的对象,这个不解释了,你可以再想想

我比较赞同 @killme2008 的方案

funky-eyes commented 1 year ago

我如果不加锁,无非得到的是上一次的conf

你可能得到一个没有构建完全的对象,这个不解释了,你可以再想想

我比较赞同 @killme2008 的方案

所以我说是start后才不需要拿读锁,如果还没start就拿锁再读,已经初始化的list并且已经附值了,永远都会有个list,只是前后顺序的问题

funky-eyes commented 1 year ago

而且这块问题可以加volatile,你是担心重排序后,附值先,初始化后的问题吧

fengjiachun commented 1 year ago

而且这块问题可以加volatile,你是担心重排序后,附值先,初始化后的问题吧

你应该没有仔细看代码,可以再看下,要比你想的复杂些,conf 还有很多可变(更新)的 API,比如 setConf,addPeers 之类,这些方法在 writeLock 里都可能有调用,不是仅仅一个 volatile conf 就可以解决的,这个问题不用再纠结了

不如重新思考是否真的存在这个场景,比如是否真的需要直连 jraft 调用这个 API,jraft 本身是个 lib 不是服务,在你的服务层作 cache 是否可以? 2000 个节点我认为直接连接的应该是你的服务,而不是 jraft lib,另外总共才 2000 个节点吧?是不是真的重新部署是一把梭全部同一时刻重启,有没有打散和分组的机制?

zhipingu commented 1 year ago

而且这块问题可以加volatile,你是担心重排序后,附值先,初始化后的问题吧

你应该没有仔细看代码,可以再看下,要比你想的复杂些,conf 还有很多可变(更新)的 API,比如 setConf,addPeers 之类,这些方法在 writeLock 里都可能有调用,不是仅仅一个 volatile conf 就可以解决的,这个问题不用再纠结了

不如重新思考是否真的存在这个场景,比如是否真的需要直连 jraft 调用这个 API,jraft 本身是个 lib 不是服务,在你的服务层作 cache 是否可以? 2000 个节点我认为直接连接的应该是你的服务,而不是 jraft lib,另外总共才 2000 个节点吧?是不是真的重新部署是一把梭全部同一时刻重启,有没有打散和分组的机制?

谢谢大家的建议,我这2天阳了,没回复,后边我服务端优化一下,这个issue可以关了

fengjiachun commented 1 year ago

祝你早日康复,我也阳了 ^_^