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

raft: clean-up maybeSendAppend area #136

Merged
merged 5 commits into from
Jan 30, 2024
Merged

Conversation

pav-kv
Copy link
Contributor

@pav-kv pav-kv commented Jan 25, 2024

This PR does a mechanical refactoring of raft.maybeSendAppend method. A new maybeSendSnapshot method is factored out for readability, and a couple of error conditions are optimized out. See individual commits.

These are preparatory clean-ups for #130.

This is a mechanical change, for code readability purposes.

Signed-off-by: Pavel Kalinnikov <[email protected]>
Signed-off-by: Pavel Kalinnikov <[email protected]>
@pav-kv
Copy link
Contributor Author

pav-kv commented Jan 25, 2024

@ahrtr PTAL. This is a mechanical change, should be easy to review. See individual commits.

@pav-kv pav-kv requested a review from ahrtr January 25, 2024 17:06
@pav-kv pav-kv force-pushed the send-append-cleanup branch from 3171824 to bf42310 Compare January 26, 2024 15:35
@pav-kv pav-kv requested a review from ahrtr January 26, 2024 15:36
Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

lgtm

Thanks.

@erikgrinaker could you take a second look?

tracker/progress.go Show resolved Hide resolved
raft.go Show resolved Hide resolved
raft.go Outdated Show resolved Hide resolved
@ahrtr
Copy link
Member

ahrtr commented Jan 30, 2024

Please resolve all review comments (or just mark it as resolved if you want to keep it as it's for now) before I merge this PR. thx

The method will not be used in states other than StateProbe or
StateReplicate, so there is little sense in having the error path in it.

Signed-off-by: Pavel Kalinnikov <[email protected]>
Previously, maybeSendAppend would also try to fetch entries, only to
return a few lines below at the errors check.

Signed-off-by: Pavel Kalinnikov <[email protected]>
@pav-kv pav-kv force-pushed the send-append-cleanup branch from bf42310 to c55352f Compare January 30, 2024 15:47
@pav-kv
Copy link
Contributor Author

pav-kv commented Jan 30, 2024

@ahrtr Done. Can be merged, thank you.

@ahrtr ahrtr merged commit d14e61b into etcd-io:main Jan 30, 2024
10 checks passed
@pav-kv pav-kv deleted the send-append-cleanup branch January 30, 2024 16:25
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