xnnyygn / xraft

xnnyygn's raft implementation
MIT License
228 stars 107 forks source link

bugfix 为OutBoundChannelGroup设置了获取连接超时时间,值为心跳间隔的一半 #12

Closed HsuChungYuan closed 3 years ago

HsuChungYuan commented 3 years ago

之前都是三节点测试,该修改在三节点测试中通过,但是突然想到了发起连接似乎被写成了串行化操作(同一时间只对一台服务器发起连接)而非并行化(同时对多台服务器发起连接),这会导致在集群中拥有更多节点时,超时时间叠加后超过心跳间隔,于是测试了11节点的情况,果然存在这个问题:6节点在线5节点下线导致超时时间叠加到了2.5秒,超过了心跳间隔。 会在下一次PR中修复这个问题。

之所以close上一次PR是因为在后来的测试中客户端的端口号错填了core的端口(而不是kv服务的端口),导致我以为出问题了(其实并没有)。

HsuChungYuan commented 3 years ago

个人认为connectTimeout可以单独作为一个配置并把他放在NodeConfig中。理论上与心跳间隔可能没有直接的关系。

只有一个约束,connectTimeout < logReplicationInterval

HsuChungYuan commented 3 years ago

个人认为connectTimeout可以单独作为一个配置并把他放在NodeConfig中。理论上与心跳间隔可能没有直接的关系。

主要在当前的实现中,获取连接为单线程同步操作,此时超时时间影响非常之大,必须小于心跳时间; 在我把获取连接修改为异步多线程之后,超时时间确实没有约束了

HsuChungYuan commented 3 years ago

个人认为connectTimeout可以单独作为一个配置并把他放在NodeConfig中。理论上与心跳间隔可能没有直接的关系。

主要在当前的实现中,获取连接为单线程同步操作,此时超时时间影响非常之大,必须小于心跳时间; 在我把获取连接修改为异步多线程之后,超时时间确实没有约束了

所以归根究底其实不是超时时间的问题,而是节点通信的线程模型存在问题,需要将NioConnector中的sendRequestVote以及sendAppendEntries从单线程修改为异步多线程

xnnyygn commented 3 years ago

NioConnector其实是支持多线程访问的,只是主线程只有一个。节点通信中,只有连接部分存在一个一个访问的情况,连接之后和各个节点之间的channel是在不同IO线程中的,相互不会有影响。你提到的异步多线程,我理解为发起连接的时候,不要一个一个连接,而是同时连接,不阻塞主线程。Netty其实是支持异步连接的,只要你把connect方法中的sync去掉。但是去掉后会带来一个问题,连接的同时想要发送消息会比较难处理,你可能需要一个消息队列。连接超时小于心跳时间可以间接解决这个问题,但是连接超时设置太短对慢网络不会友好。 一个理想的方式是,给每个节点分配一个线程,连接,消息处理全在线程中做。有新消息要发送的时候,发送给这个线程的队列,而不用管这个节点的线程现在是连接中还是已经连接上。这其实就是Actor模型了。 在这个PR里面,我觉得主要还是给一个单独的超时时间设置。线程模型,如果你有时间的话,可以新开一个PR。我自己写这个连接部分的时候,也重写了几次,也期待你能写出比我更好的连接模块。

HsuChungYuan commented 3 years ago

https://github.com/xnnyygn/xraft/commit/5f1a3c795fa06a79b50b69cc08e5a03536a91788#diff-0ae774413fc8757c435d8fd982e0ff1a 这是我前两天在您回答之前写的,经过11节点测试(6up5down)没有问题,也是考虑到将connect中的sync去掉会对代码修改幅度较大,所以我创建了一个cachedThreadPool,并传入了自定义的threadFactory以捕获异常,将NioConnector中发起投票操作和日志复制操作直接放到线程池中运行。这种方式会有什么问题吗?这应该符合您说的理想方式,但我不确定这是否符合您所说的Actor模型,我对其不是很了解。

xnnyygn commented 3 years ago

抱歉久等了。这两天我在尝试理想的异步多线程模型中应该怎么处理,得到的结论是

在握手之后才能算正式连接。发起连接到正式连接之间的消息可能选择保存全部(队列)或者只保留最后一条。连接超时理论上和心跳没有关系,只在发起连接时有用。

raft协议中本身没有握手,但是实现中需要。现有代码中启动时发送自己的NodeId也算是半个握手。

当然,在现有代码中实现起来改动会比较大,所以你的修改算是折中方案。连接阶段的消息顺序问题暂时不会有太大问题。