nsqio / go-nsq

The official Go package for NSQ
MIT License
2.59k stars 444 forks source link

reduce duplicate rdy update requests #282

Closed andyxning closed 4 years ago

andyxning commented 4 years ago

This PR will reduct duplicate rdy update requests.

andyxning commented 4 years ago

/cc @ploxiln @mreiferson

andyxning commented 4 years ago

/cc @ploxiln @mreiferson

ploxiln commented 4 years ago

Yeah, seems safe, I might just merge it, since you at least will use it plenty. But I would like to get some input from the other maintainers.

By the way, did you consider this alternative:

--- a/consumer.go
+++ b/consumer.go
@@ -975,7 +975,7 @@ func (r *Consumer) updateRDY(c *Conn, count int64) error {
 }

 func (r *Consumer) sendRDY(c *Conn, count int64) error {
-       if count == 0 && c.LastRDY() == 0 {
+       if count == c.LastRDY() {
                // no need to send. It's already that RDY count
                return nil
        }
andyxning commented 4 years ago

@ploxiln Thanks for the suggestion. The key point of implementing using the code in the PR is that if the count is not zero, then SetRDY will update lastRdyTimestamp. Since rdy manage is tricky, i think we should not neglect it.

ploxiln commented 4 years ago

Yeah, fair.

Some observations:

So this should be fine. Maybe later someone will come up with a good refactor of the whole RDY distribution algorithm (e.g. to address #277).