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.

Sign up for GitHub

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

Jump to bottom

raft: fast fail while dropping index reading #9567

Closed
wants to merge 2 commits into from
Closed

raft: fast fail while dropping index reading #9567

wants to merge 2 commits into from

Conversation

nolouch
Copy link
Contributor

@nolouch nolouch commented Apr 12, 2018

The read request will block the following read request until the context timeout when the read message dropped in the raft, try to fast fail in some case.
This pull request is used to fix #9566

@nolouch nolouch force-pushed the fast-read branch 2 times, most recently from 99eff43 to 412c4d2 Compare April 14, 2018 03:54
@xiang90
Copy link
Contributor

xiang90 commented Apr 15, 2018

@nolouch Have you tested it from etcd point of view? And can you add an integration test for this into etcdserver side?

@nolouch
Copy link
Contributor Author

nolouch commented Apr 16, 2018

ok, I will add the test.

siddontang
siddontang reviewed Jun 15, 2018
View reviewed changes
raft/node.go
@@ -449,23 +454,17 @@ func (n *node) stepWait(ctx context.Context, m pb.Message) error {
// Step advances the state machine using msgs. The ctx.Err() will be returned,
// if any.
func (n *node) stepWithWaitOption(ctx context.Context, m pb.Message, wait bool) error {
if m.Type != pb.MsgProp {
Copy link
Contributor

Choose a reason for hiding this comment

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

why removing them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This logic is no longer suitable because the recvc channel also need to handle the error.

@nolouch nolouch force-pushed the fast-read branch 3 times, most recently from 235e0f1 to 06928fc Compare September 13, 2018 08:30
@nolouch
Copy link
Contributor Author

nolouch commented Sep 13, 2018 •
edited
Loading

Sorry for the delay in this PR.
In my test, this patch can only solve part of the problem. If the request coming in the follower, and before it dropping the leader while meeting the network partition, the problem also exists. and I changed the behavior of the client to bypass this problem.
I will send a new PR that drop the old read request immediately if found the leader changed. So can prevent blocking the read cause of I mentioned above. and this pr can start the review again.

@nolouch nolouch mentioned this pull request Sep 14, 2018
Merged
@xiang90
Copy link
Contributor

xiang90 commented Sep 24, 2018

closing in favor of #10094

@xiang90 xiang90 closed this Sep 24, 2018
@nolouch nolouch deleted the fast-read branch September 24, 2018 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@siddontang siddontang siddontang left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

fail fast on the index request dropping while leader changed
3 participants
@nolouch @xiang90 @siddontang