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

RATIS-1399. should notify all the log senders Asynchronously #496

Merged
merged 2 commits into from Sep 10, 2021

Conversation

JacksonYao287
Copy link

What changes were proposed in this pull request?

//LeaderStateImpl.java

void notifySenders() {
  senders.forEach(LogAppender::notifyLogAppender);
}

if raft , one slow log sender should not block others sender

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-1399

How was this patch tested?

no need

@JacksonYao287
Copy link
Author

@szetszwo @bshashikant @guihecheng PTAL!

@szetszwo
Copy link
Contributor

szetszwo commented Sep 2, 2021

@JacksonYao287 , the change looks good. Have you tested it?

@JacksonYao287
Copy link
Author

JacksonYao287 commented Sep 3, 2021

@szetszwo thanks for the review, i think the logic here is correct, and i will test it in my cluster to confirm it

@guihecheng
Copy link

LGTM

@szetszwo
Copy link
Contributor

szetszwo commented Sep 9, 2021

..., and i will test it in my cluster to confirm it

@JacksonYao287 , have you got a chance to test it?

@JacksonYao287
Copy link
Author

JacksonYao287 commented Sep 9, 2021

@szetszwo i have tested this patch. i start a scm first(leader), but i find a problem when the second raft peer start(follower) that it will fail to install snapshot. i am investigating this issue. do you have any idea about this? i open a jira for this issue
https://issues.apache.org/jira/browse/RATIS-1402

@JacksonYao287
Copy link
Author

@szetszwo hi, we have tested this patch and it works as expected, the performance does not downgrade again when a follower crashes. seems we can merge this patch now.

@szetszwo
Copy link
Contributor

@szetszwo i have tested this patch. i start a scm first(leader), but i find a problem when the second raft peer start(follower) that it will fail to install snapshot. i am investigating this issue. do you have any idea about this? i open a jira for this issue
https://issues.apache.org/jira/browse/RATIS-1402

Sorry that I don't aware this problem. As you mentioned in RATIS-1402, it probably was caused by RATIS-1399.

@szetszwo szetszwo merged commit 7a85273 into apache:master Sep 10, 2021
@JacksonYao287
Copy link
Author

JacksonYao287 commented Sep 10, 2021

Sorry that I don't aware this problem. As you mentioned in RATIS-1402, it probably was caused by RATIS-1399.

no , i think it is caused by RATIS-1390 not RATIS-1399. when i test this patch , i revert RATIS-1390. i will try to fix it in RATIS-1402.

thanks @szetszwo for the review and @guihecheng for the great work

@JacksonYao287 JacksonYao287 deleted the RATIS-1399 branch September 10, 2021 22:59
symious pushed a commit to symious/ratis that referenced this pull request Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants