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

fix: reset redistribution delay on peer membership change #1403

Merged
merged 8 commits into from
Nov 6, 2024

Conversation

VinozzZ
Copy link
Contributor

@VinozzZ VinozzZ commented Oct 29, 2024

Which problem is this PR solving?

Short description of the changes

  • moved redistributeNotifier to its own file since collect.go is getting big
  • only notify the triggered channel when timer fires
  • reset timer when receiving a peer membership change from r.reset
  • added tests

@VinozzZ VinozzZ added the type: bug Something isn't working label Oct 29, 2024
@VinozzZ VinozzZ added this to the v2.9 milestone Oct 29, 2024
@VinozzZ VinozzZ self-assigned this Oct 29, 2024
@VinozzZ VinozzZ requested a review from a team as a code owner October 29, 2024 21:07
Copy link
Contributor

@kentquirk kentquirk left a comment

Choose a reason for hiding this comment

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

I believe this is correct, but I added some more comments because I've already forgotten the details from when we talked about this a while back. If I'm not correct, please fix it. I think this is one of those things that probably can't have enough explanation.

collect/trace_redistributer.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

VinozzZ commented Oct 30, 2024

@kentquirk That sounds correct to me! Thanks for adding the comments

collect/trace_redistributer.go Outdated Show resolved Hide resolved
collect/trace_redistributer_test.go Outdated Show resolved Hide resolved
collect/trace_redistributer_test.go Show resolved Hide resolved
collect/trace_redistributer.go Outdated Show resolved Hide resolved
@VinozzZ VinozzZ merged commit 96f5b92 into main Nov 6, 2024
5 checks passed
@VinozzZ VinozzZ deleted the yingrong/reset_timer_on_peer_membership_change branch November 6, 2024 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants