-
Notifications
You must be signed in to change notification settings - Fork 42
implement a garbage-collector for unused reuse connections #73
Conversation
5fde971
to
a68dcbb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bunch of nits but otherwise LGTM.
func (c *reuseConn) IncreaseCount() { atomic.AddInt32(&c.refCount, 1) } | ||
func (c *reuseConn) DecreaseCount() { atomic.AddInt32(&c.refCount, -1) } | ||
func (c *reuseConn) GetCount() int { return int(atomic.LoadInt32(&c.refCount)) } | ||
func (c *reuseConn) IncreaseCount() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should probably be private.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it actually matters. We're never exporting a reuseConn
from this package, so there's no way a user could call this function anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. But can't the user type assert?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really. What we're returning is (wrapped) QUIC connections, not UDP connections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, fair enough. I still think private functions are clearer but, well, we're not consistent about that anyways.
c.mutex.Unlock() | ||
} | ||
|
||
func (c *reuseConn) ShouldGarbageCollect(now time.Time) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: also should be private.
c.mutex.Unlock() | ||
} | ||
|
||
func (c *reuseConn) DecreaseCount() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also should be private.
a68dcbb
to
a53ee68
Compare
Fixes #70.
This PR implements the sweeper as suggested in #63.
The sweeper runs every 30 seconds and removes connections that have been unused for more than 10 seconds. In order to not leak go routines, it stops itself once a
reuse
doesn't have any more active connections, and is restarted as soon as a connection is added.