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 can't retry InstallSnapshot problem( #605 ) #606

Merged
merged 5 commits into from
Jun 16, 2021
Merged

fix can't retry InstallSnapshot problem( #605 ) #606

merged 5 commits into from
Jun 16, 2021

Conversation

312223105
Copy link
Contributor

Motivation:

Fixed the problem that all InstallSnapshot sessions will be blocked when a new InstallSnapshot request arrives, see #605

Result:

Fixes #605 .

@sofastack-bot
Copy link

sofastack-bot bot commented Jun 10, 2021

Hi @312223105, Please sign Contributor License Agreement!

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

@fengjiachun
Copy link
Contributor

@312223105 你好,先执行一下 mvn clean compile(会自动执行代码格式化)再提交

@sofastack-bot sofastack-bot bot added cla:yes and removed cla:no labels Jun 11, 2021
} else if (m.request.getMeta().getLastIncludedIndex() > ds.request.getMeta().getLastIncludedIndex()) {
// |is| is older
// |ds| is older
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is error, the is is installing snapshot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is line 625 also wrong? My understanding is that |xx| means xx is a variable. But there is no variable is in code.

Copy link
Contributor

Choose a reason for hiding this comment

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

m is is, it was renamed someday before, but the comment was not changed.

Copy link
Contributor

@fengjiachun fengjiachun left a comment

Choose a reason for hiding this comment

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

lgtm

@fengjiachun fengjiachun merged commit a1fa8c3 into sofastack:master Jun 16, 2021
horizonzy pushed a commit to horizonzy/sofa-jraft that referenced this pull request Jun 25, 2021
* (fix) Fixed the problem that all InstallSnapshot sessions will be blocked when a new InstallSnapshot request arrives, see sofastack#605
horizonzy pushed a commit to horizonzy/sofa-jraft that referenced this pull request Jun 25, 2021
* (fix) Fixed the problem that all InstallSnapshot sessions will be blocked when a new InstallSnapshot request arrives, see sofastack#605
@fengjiachun fengjiachun mentioned this pull request Aug 10, 2021
10 tasks
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 First-time contributor question Further information is requested size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

若InstallSnapshot执行期间再次接收到新的InstallSnapshot请求,loadSnapshot将不会再被执行
3 participants