Skip to content
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

fix elect priority #357

Merged
merged 2 commits into from
Nov 27, 2019
Merged

fix elect priority #357

merged 2 commits into from
Nov 27, 2019

Conversation

fengjiachun
Copy link
Contributor

Motivation:

When leader stepdown, select (maxIndex, maxPriority) as next candidate

Modification:

Result:

@sofastack-bot sofastack-bot bot added bug Something isn't working cla:yes size/M labels Nov 26, 2019
final long nextIndex = Replicator.getNextIndex(entry.getValue());
if (nextIndex > maxIndex) {
if (nextIndex >= maxIndex && nextPriority > priority) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个改动可能是有问题的,改变了原来的语义。在没有启用 priority 的情况下,是选择出复制最多数据(nextIndex 最大)的节点来作为下一个 leader;改动后,同样在没有启用 priority 的情况下,不会去选择出复制最多(因为 nextPriority > priority 这个条件),可能需要将 nextPriority > priority 改成 nextPriority >= priority

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

恩,这里有问题,又改了这段逻辑你再看下

@killme2008 killme2008 merged commit 570219d into master Nov 27, 2019
@killme2008 killme2008 deleted the fix/minor_fix_elect_priority branch November 27, 2019 12:19
@zongtanghu
Copy link
Contributor

赞!这里是我开始时候没有考虑到的点,感谢家纯帮忙fix,感谢伯岩的review哈 @fengjiachun @killme2008

luozhiyun993 pushed a commit to luozhiyun993/sofa-jraft that referenced this pull request Nov 28, 2019
* 'master' of https://github.com/sofastack/sofa-jraft:
  Feature/some changes (sofastack#358)
  fix elect priority (sofastack#357)
  Remove unused imports (sofastack#356)
  feat/priority_by_zongtanghu (sofastack#345)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cla:yes size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants