Skip to content

*: port coreos/etcd#9985#340

Merged
BusyJay merged 3 commits intotikv:0.4.xfrom
BusyJay:pick-9985-4.x
Mar 3, 2020
Merged

*: port coreos/etcd#9985#340
BusyJay merged 3 commits intotikv:0.4.xfrom
BusyJay:pick-9985-4.x

Conversation

@BusyJay
Copy link
Copy Markdown
Member

@BusyJay BusyJay commented Feb 27, 2020

The patch is to speed up log replication when a node is way behind than leader and logs are not compacted yet.

Signed-off-by: Jay Lee <BusyJayLee@gmail.com>
Signed-off-by: Jay Lee <BusyJayLee@gmail.com>
to_send.set_index(rs.index);
to_send.set_entries(req.take_entries());
*more_to_send = Some(to_send);
ctx.more_to_send.push(to_send);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about more_to_send: HashMap<u64, Message>? So we can send only 1 read index response to one peer.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

HashMap is slower than Vec, it may not be a good idea. On the other hand, the optimization assumes there is a queue in follower side, which may not be a valid case.

@hicqu
Copy link
Copy Markdown
Contributor

hicqu commented Mar 2, 2020

rest LGTM. BTW could you send a PR to master? Let's think about how to fix test cases about initializing with configuration.

@BusyJay
Copy link
Copy Markdown
Member Author

BusyJay commented Mar 2, 2020

I will port to master when tests issues are fixed.

hicqu
hicqu previously approved these changes Mar 2, 2020
Copy link
Copy Markdown
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

src/raft.rs Outdated
}

if send_append {
if ctx.has_reply {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The has_reply is a bit redundant. Maybe we can change it to

Suggested change
if ctx.has_reply {
if ctx.send_append || ctx.loop_append {

Signed-off-by: Jay Lee <BusyJayLee@gmail.com>
Copy link
Copy Markdown
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.

LGTM

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.

3 participants