WFE: Add new key-value ratelimits implementation#7089
Conversation
|
@beautifulentropy, this PR appears to contain configuration changes. Please ensure that a corresponding deployment ticket has been filed with the new configuration values. |
79f905d to
05ffd61
Compare
pgporada
left a comment
There was a problem hiding this comment.
//rocsp/rocsp.go NewReadingClient states rdb must be non-nil, but shouldn't there be a nil check inside the constructor for that rather than just a comment?
I didn't actually write that comment. It shows green because I removed the content in that comment which came directly after it and was no longer applicable due to a change I made inside the function. |
39fbdf5 to
e546830
Compare
e546830 to
f2e93de
Compare
aarongable
left a comment
There was a problem hiding this comment.
LGTM with the exception of just one question about the new context cancellation mechanism.
Integrate the key-value rate limits from #6947 into the WFE. Rate limits are backed by the Redis source added in #7016, and use the SRV record shard discovery added in #7042.
Part of #5545
Notes for reviewers:
There were a lot of small changes necessary to complete this change:
Changes in ratelimits:
Nameanfmt.Stringerand utilize the newString()method throughout.NewRegistrationsPerIPv6Range.*redis.Ringconstruction, outside of the Redis Source. These metrics are germane to client itself, not the source implementation in ratelimits.ratelimits.RedisSource.timeout. We should instead rely on the go-redis read and write timeouts.Changes in bredis:
rocsp_config.RedisConfigtobredis.Config.MustRegisterClientMetricsCollector()for registering go-redis client metrics.bredis.Configand a new constructorNewRingFromConfig()to handle go-redis client and service discovery construction. This is much cleaner than constructing a client inside of the WFE and will be useful if/when we add more Redis caches to the WFE.lookup.updateNow()now re-registers client metrics when new shards are discovered.Changes in rocsp/config:
NewReadingClient()toMakeReadClient()MakeReadClient()calls the newbredis.MustRegisterClientMetricsCollector()for consistency.Change in wfe2:
NewRegistrationsPerIP/v6Rangeinside the newcheckNewAccountLimits()func and if the subsequent ra.NewRegistration() call fails, refund the spent limit quota using the newrefundNewAccountLimits().Changes in cmd/boulder-wfe:
*redis.Ringclient, the bredis.Lookup (optionally), and add the resulting Limiter to the resultingWebFrontEndImpl.