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

check pending reads when quorum becomes small #363

Merged
merged 3 commits into from
May 21, 2020

Conversation

hicqu
Copy link
Contributor

@hicqu hicqu commented May 14, 2020

Close #345 .

Signed-off-by: qupeng qupeng@pingcap.com

Signed-off-by: qupeng <qupeng@pingcap.com>
src/raft.rs Outdated
@@ -2325,4 +2305,21 @@ impl<T: Storage> Raft<T> {
m.request_snapshot = self.pending_request_snapshot;
self.send(m);
}

fn response_ready_read(&mut self, mut req: Message, index: u64) -> Option<Message> {
Copy link
Member

Choose a reason for hiding this comment

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

s/response_ready_read/handle_ready_read_index/


// The quorum size is now smaller, consider to response some read requests.
// If there is only one peer, all pending read requests must be responsed.
if let Some(ctx) = self.read_only.last_pending_request_ctx() {
Copy link
Member

Choose a reason for hiding this comment

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

What if some reads in the front of the queue are ready to be processed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without iterating the pending queue, they can only become ready when more heartbeat responses are received. Do you have any solution to handle them without iterating?

@@ -4920,3 +4920,36 @@ fn test_group_commit_consistent() {
}
}
}

#[test]
fn test_read_when_quorum_becomes_small() {
Copy link
Member

Choose a reason for hiding this comment

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

Please add comment what it is testing.

Signed-off-by: qupeng <qupeng@pingcap.com>
BusyJay
BusyJay previously approved these changes May 14, 2020
@hicqu
Copy link
Contributor Author

hicqu commented May 19, 2020

/run-all-tests

@hicqu
Copy link
Contributor Author

hicqu commented May 19, 2020

/rebuild

Copy link
Member

@gengliqi gengliqi left a comment

Choose a reason for hiding this comment

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

rest LGTM

@@ -4920,3 +4920,38 @@ fn test_group_commit_consistent() {
}
}
}

// `test_read_when_quorum_becomes_small` tests read requests could be handled earlier
Copy link
Member

Choose a reason for hiding this comment

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

Seems it should be test_read_when_quorum_becomes_less?

Signed-off-by: qupeng <qupeng@pingcap.com>
@hicqu hicqu merged commit 513b82c into tikv:master May 21, 2020
@hicqu hicqu deleted the conf-change-with-read branch May 21, 2020 06:00
hicqu added a commit that referenced this pull request May 25, 2020
Signed-off-by: qupeng <qupeng@pingcap.com>
hicqu added a commit to hicqu/raft-rs that referenced this pull request May 25, 2020
Signed-off-by: qupeng <qupeng@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quorum change should check read index
3 participants