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

v3rpc: add jitter to progress notification #9278

Merged
merged 1 commit into from
Feb 5, 2018
Merged

Conversation

xiang90
Copy link
Contributor

@xiang90 xiang90 commented Feb 5, 2018

Fix #9246

/cc @cw9

// add rand(1/10*progressReportInterval) as jitter so that etcdserver will not
// send progress notifications to watchers around the same time even when watchers
// are created around the same time (which is common when a client restarts itself).
jitter := time.Duration(rand.Int63n(int64(progressReportInterval) / 10))
Copy link

Choose a reason for hiding this comment

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

Should int64(progressReportInterval) / 10 be done within the Rlock?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh. you are right. fixing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cw9 i still keep the rand func out of the lock for good locking practices to minimize the critical section since rand can be expensive.

Copy link

@cw9 cw9 left a comment

Choose a reason for hiding this comment

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

LGTM

@xiang90 xiang90 added this to the v3.4.0 milestone Feb 5, 2018
@xiang90 xiang90 merged commit c80ca24 into etcd-io:master Feb 5, 2018
@xiang90 xiang90 deleted the jitter branch February 5, 2018 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants