-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add Flexible Mode For JRaft #1013
Conversation
# Conflicts: # jraft-core/src/main/java/com/alipay/sofa/jraft/Quorum.java # jraft-core/src/main/java/com/alipay/sofa/jraft/core/BallotBox.java # jraft-core/src/main/java/com/alipay/sofa/jraft/core/NodeImpl.java # jraft-core/src/main/java/com/alipay/sofa/jraft/entity/MajorityQuorum.java # jraft-core/src/main/java/com/alipay/sofa/jraft/entity/NWRQuorum.java # jraft-core/src/main/java/com/alipay/sofa/jraft/entity/QuorumConfiguration.java # jraft-core/src/main/java/com/alipay/sofa/jraft/entity/QuorumFactory.java # jraft-core/src/main/java/com/alipay/sofa/jraft/option/NodeOptions.java # jraft-core/src/test/java/com/alipay/sofa/jraft/entity/NWRQuorumTest.java
赞!我会尽快阅读。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我提了一些建议,可以在讨论讨论
|
||
import com.alipay.sofa.jraft.conf.Configuration; | ||
import com.alipay.sofa.jraft.entity.PeerId; | ||
import org.slf4j.Logger; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
按照项目本身的import顺序,会更好。
java.*
javax.*
org.*
com.*
all other import
static all other import
this.oldQuorum = this.oldPeers.size() / 2 + 1; | ||
return true; | ||
} | ||
public abstract void grant(final PeerId peerId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
看代码,这个方法应该不用修饰为abstract
@@ -20,6 +20,8 @@ | |||
|
|||
import javax.annotation.concurrent.ThreadSafe; | |||
|
|||
import com.alipay.sofa.jraft.Quorum; | |||
import com.alipay.sofa.jraft.entity.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不建议使用import *
以及import顺序
* @param conf current configuration | ||
* @param oldConf old configuration | ||
* @param done callback | ||
* @param quorumConfiguration quorum configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
format
public boolean appendPendingTask(final Configuration conf, final Configuration oldConf, final Closure done, | ||
final QuorumConfiguration quorumConfiguration) { | ||
final Quorum quorum; | ||
quorum = quorumConfiguration.isEnableFlexibleMode() ? new FlexibleQuorum(quorumConfiguration.getWriteFactor(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个判断在这是很频繁的,而isEnableFlexibleMode变化只会在启动、配置变更时,因此这个判断是否应该考虑放在这些地方。
例如:
interface Quorum{
int w(int n)
int r(int n)
}
class FlexibleQuorum impl Quorum
class MajorityQuorum impl Quorum
启动时:
private Quorum quorum = isEnableFlexibleMode? FlexibleQuorum: MajorityQuorum
使用时:
Ballot ballot = BallotFactor.createWriteBallot(oldConfig, newConfig, quorum)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
下文用到的getReadQuorum,也可以这样获取
newConf, | ||
oldConf, | ||
configurationChangeDone, | ||
options.isEnableFlexibleRaft() ? QuorumFactory.createFlexibleQuorumConfiguration( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
同样的很频繁,得考虑重新组织代码
/** | ||
* @author Akai | ||
*/ | ||
public class FlexibleQuorum extends Quorum { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
应该是缺少一个实体。上文给过伪代码
一个用于计算,一个用于grant,Factory用于创建read、write
public class QuorumConfiguration { | ||
private Integer readFactor; | ||
private Integer writeFactor; | ||
private boolean isEnableFlexibleMode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enableFlexibleMode可能会更好。下方的isEnableFlexibleMode()不用修改
|
||
public void setWriteQuorumFactor(int writeQuorumFactor) { | ||
this.writeQuorumFactor = writeQuorumFactor; | ||
enableFlexibleRaft(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不建议在get/set放太多逻辑,重新生成就都覆盖了
return enableFlexibleRaft; | ||
} | ||
|
||
private void enableFlexibleRaft() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
提供这样方便使用的方法是好的,但是我觉得它仍然需要set方法
Motivation:
Implementing flexible raft by Integrating QuorumNWR for SOFA-JRaft
Modification:
Result:
Fixes #1003 .
使用docker启动6个ubuntu容器进行jepsen测试,其中ubuntu-1作为control节点,ubuntu-2~6作为test节点,并通过mvn install自测试代码到本地仓库进行jepsen测试。
测试结果如下所示:
atomic.bridge:
atomic.configuration:
atomic.crash:
atomic.partition:
atomic.partition-majority:
atomic.pause: