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

simplifying raft progress inflights #4899

Closed
ongardie opened this issue Mar 29, 2016 · 6 comments
Closed

simplifying raft progress inflights #4899

ongardie opened this issue Mar 29, 2016 · 6 comments

Comments

@ongardie
Copy link

Hope this is an ok place to discuss. I came across some, shall we say, "clever", data structure for the "inflights" defined in this file: https://github.com/coreos/etcd/blob/v2.3.0/raft/progress.go#L66

My basic question is: what is the purpose of it? Is it only used for application-level flow control, to prevent the leader from sending too many entries to a follower too quickly? If that's all, would just an integer counting how many messages/entries/bytes have been sent recently get the job done?

cc @superfell

@xiang90
Copy link
Contributor

xiang90 commented Mar 29, 2016

what is the purpose of it? Is it only used for application-level flow control, to prevent the leader from sending too many entries to a follower too quickly?

Yes.

If that's all, would just an integer counting how many messages/entries/bytes have been sent recently get the job done?

Counting how many bytes/messages have been sent is not enough. We want to know how many bytes are inflight. So we need to know how many bytes/messages have been sent and how many bytes/messages have been received. The sliding buffer is a map from index to its size. So by sliding that window, we can calculate the inflight number of bytes.

@xiang90
Copy link
Contributor

xiang90 commented Mar 29, 2016

@ongardie If we only want to count the inflight number of entries, then we do not need this window. A simple counter is fine.

@ongardie
Copy link
Author

ongardie commented Apr 1, 2016

Ah, I didn't realize you were tracking bytes per message. Can you make that clear in the code comments? I see why an integer isn't enough now. I'm still confused as to which variables represent log indexes and which ones represent bytes.

Also, is inflights.size the same as len(inflights.buffer)? If so, I'd suggest removing it, and if not, please teach me Go :)

@xiang90
Copy link
Contributor

xiang90 commented Apr 1, 2016

Also, is inflights.size the same as len(inflights.buffer)? If so, I'd suggest removing it, and if not, please teach me Go :)

Obviously, you know that better than me. I will try to clean inflight up and make the doc better. Thanks for bringing this up.

@ongardie
Copy link
Author

ongardie commented Apr 1, 2016

if not, please teach me Go :)

Obviously, you know that better than me.

No way! I'm pretty sure I've written under 500 lines of Go in total still. I'm a noob.

Thanks for clarifying and working to clean this up.

@xiang90
Copy link
Contributor

xiang90 commented Oct 4, 2016

Closed by #6513

@xiang90 xiang90 closed this as completed Oct 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants