To change how GLOBAL behavior operates. Previously, the owner of the rate limit would broadcast the computed result of the rate limit to other peers, and the computed result from the owner is returned to clients who submit hits to a peer. However, after some great feed back on #218 and #216 It has become clear that we should instead allow the local peers to compute the result of the request based on the hits broadcast by the owning peer.
In the new behavior a peer will compute the result of the rate limit request and immediately return that computed result. This means that the non owning peer will compute the result with the current Remaining value it currently has in it's cache. To put it another way, the peer cache will no longer hold the computed result from the owner.
In order to facilitate this change, I've added many more tests around global functionality which should help ensure we don't break behavior going forward.
Implementation
The global broadcaster will now use the new DRAIN_OVER_LIMIT behavior when accepting peer rate limits. This allows many peers to send a burst of hits to the owner while still updating the remaining even if the requested hits is more that is available. See https://github.com/mailgun/gubernator/pull/218#discussion_r1483302784
Added Tests to ensure the "do not update remaining if requesting more than is remaining" semantic does not change.
Added cluster.ListNonOwningDaemons() to assist in global testing
Added cluster.FindOwningDaemon() to assist in global testing
Added waitForBroadcast() to functional testing suite tools so we can remove the sleep statements.
Moved newStaticBuilder() to daemon.go
Added MustClient() and Client() to gubernator.Daemon to make it easier to find and send RPC requests to a daemon instance.
RESET_REMAINING now works correctly with global behavior
Fixed an issue where the peer that received the rate limit would broadcast the rate limit to all peers instead of waiting for the owning peer to broadcast.
Now correctly populating Daemon.InstanceID and Added InstanceID to the Instance
Bumped Lint Version
Misc Test fixes and improvements. TestLeakyBucketGregorian should no longer occasionally fail the test suite.
This PR replaces #218 and #216 but keeps the commits from those PR's
Thank you to these wonderful people, for giving great feed back and working on this!
Purpose
To change how
GLOBAL
behavior operates. Previously, the owner of the rate limit would broadcast the computed result of the rate limit to other peers, and the computed result from the owner is returned to clients who submit hits to a peer. However, after some great feed back on #218 and #216 It has become clear that we should instead allow the local peers to compute the result of the request based on the hits broadcast by the owning peer.In the new behavior a peer will compute the result of the rate limit request and immediately return that computed result. This means that the non owning peer will compute the result with the current
Remaining
value it currently has in it's cache. To put it another way, the peer cache will no longer hold the computed result from the owner.In order to facilitate this change, I've added many more tests around global functionality which should help ensure we don't break behavior going forward.
Implementation
DRAIN_OVER_LIMIT
behavior when accepting peer rate limits. This allows many peers to send a burst of hits to the owner while still updating the remaining even if the requested hits is more that is available. See https://github.com/mailgun/gubernator/pull/218#discussion_r1483302784cluster.ListNonOwningDaemons()
to assist in global testingcluster.FindOwningDaemon()
to assist in global testingwaitForBroadcast()
to functional testing suite tools so we can remove the sleep statements.newStaticBuilder()
to daemon.goMustClient()
andClient()
togubernator.Daemon
to make it easier to find and send RPC requests to a daemon instance.RESET_REMAINING
now works correctly with global behaviorDaemon.InstanceID
and AddedInstanceID
to theInstance
TestLeakyBucketGregorian
should no longer occasionally fail the test suite.This PR replaces #218 and #216 but keeps the commits from those PR's
Thank you to these wonderful people, for giving great feed back and working on this!