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

nsqd: reduce ReadyCount update frequency #1226

Merged

Conversation

andyxning
Copy link
Member

This PR will reduce the update frequency for ReadyCount when the new rdy count equals with the old one.

@andyxning
Copy link
Member Author

/cc @ploxiln @mreiferson

@andyxning andyxning force-pushed the nsqd_reduce_ReadyCount_update_frequency branch from 51e8dab to 85139c7 Compare December 23, 2019 13:30
@ploxiln
Copy link
Member

ploxiln commented Dec 23, 2019

This results in two atomic operations for the "change" case. It could be just one like:

old_count = atomic.SwapInt64(&c.ReadyCount, count)
if old_count != count {
    c.tryUpdateReadyState()
}

(I suppose the "no change" case happens quite a lot for some of your services? if it's not too frequent, it's not expensive ...)

@andyxning andyxning force-pushed the nsqd_reduce_ReadyCount_update_frequency branch from 85139c7 to 368af1a Compare December 24, 2019 03:33
@andyxning
Copy link
Member Author

andyxning commented Dec 24, 2019

@ploxiln Thanks for your good advice.

I suppose the "no change" case happens quite a lot for some of your services? if it's not too frequent, it's not expensive ...

Yes. it happens a lot. But even if it is not, avoiding useless logic is good, right? :)

@ploxiln
Copy link
Member

ploxiln commented Dec 24, 2019

cool. thanks

@ploxiln ploxiln merged commit 113a534 into nsqio:master Dec 24, 2019
@andyxning andyxning deleted the nsqd_reduce_ReadyCount_update_frequency branch December 24, 2019 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants