Closed philips closed 4 years ago
Hey Brandon - that seems fine, but I'm a bit worried about the lock contention as we'd need to lock the entire map, build a new data structure with all the string keys, and return that. Given the focus on performance, that could get expensive, especially if you're calling Keys() frequently.
Are you interested in the specific keys or the number of keys? Having a Size()
or NumKeys()
would be much more efficient because we can just grab the size of the map and avoid the extra allocation of the []string
.
I would probably do the operation every 30 seconds or something like that.
I suppose the thing I am most interested in is keys that hit a ratelimit. So, I could just keep a separate data structure.
I think I will close this for now and just see if rate limit hits is enough visibility.
On Fri, Aug 14, 2020, 6:59 AM Seth Vargo notifications@github.com wrote:
Hey Brandon - that seems fine, but I'm a bit worried about the lock contention as we'd need to lock the entire map, build a new data structure with all the string keys, and return that. Given the focus on performance, that could get expensive, especially if you're calling Keys() frequently.
Are you interested in the specific keys or the number of keys? Having a Size() or NumKeys() would be much more efficient because we can just grab the size of the map and avoid the extra allocation of the []string.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sethvargo/go-limiter/issues/13#issuecomment-674088583, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAIGCHKE3YTOJPFNJ4JSVLSAU7LBANCNFSM4P7HOYIQ .
Okay sounds good - I'm definitely interested in making this flexible and extensible 😄
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For generating monitoring metrics, say to export to prometheus, it would be nice to be able to enumerate the keys in the store. An interface as simple as this would be fine with me:
This interface with a Peek (#11) would make it possible to implement metrics, I think.