feat(erigon): add api ratelimit and apikey #42
Conversation
| sync.RWMutex | ||
| } | ||
|
|
||
| var rateLimit = &RateLimit{} |
There was a problem hiding this comment.
We should rename this better to ensure that the global rate limiter config is better managed.
There was a problem hiding this comment.
We should also initialize the map here instead of checking everywhere in the code whether the pointer is nil or not.
There was a problem hiding this comment.
Oh, greate! This is a very good suggestion for me
| func methodRateLimitAllow(method string) bool { | ||
| rateLimit.RLock() | ||
| rlm := rateLimit.rlm | ||
| rateLimit.RUnlock() |
There was a problem hiding this comment.
Missing defer here as read operation is not finish. We get the pointer to the map, and then unlock the mutex, which can cause issues downstream.
There was a problem hiding this comment.
I think we just need to get this pointer of this rlm map, becasue we use rateLimit.rlm = new(map) to update this rlm(we update thie pointer, but not the value in this map), so if I got this map pointer, I will read the old value this time, but the next time i will get a new one.
| } | ||
| return rlm | ||
| } | ||
| return nil |
There was a problem hiding this comment.
This is problematic and unnecessary here to set the map to nil if config is uninitialized.
| func apikeyMethodRateLimitAllow(api, method string) bool { | ||
| apiKeyRateLimit.RLock() | ||
| rlm := apiKeyRateLimit.rlm | ||
| apiKeyRateLimit.RUnlock() |
| k.Key = strings.ToLower(k.Key) | ||
| parse, err := time.Parse("2006-01-02", k.Timeout) | ||
| if err != nil { | ||
| log.Warn("parse key [%+v], error parsing timeout: %v", k, err) |
| rlc := RateLimitConfig{} | ||
| err := json.Unmarshal([]byte(cfg), &rlc) | ||
| if err != nil { | ||
| log.Warn("invalid rate limit config: %s", cfg) |
|
|
||
| // updateRateLimit updates the rate limit config | ||
| func updateRateLimit(rateLimit RateLimitConfig) map[string]*rate.Limiter { | ||
| log.Info("rate limit config updated", "config", rateLimit) |
| func updateRateLimit(rateLimit RateLimitConfig) map[string]*rate.Limiter { | ||
| log.Info("rate limit config updated", "config", rateLimit) | ||
| if len(rateLimit.RateLimitApis) > 0 { | ||
| log.Info("rate limit enabled", "api", rateLimit.RateLimitApis, "count", rateLimit.RateLimitCount, "bucket", rateLimit.RateLimitBucket) |
| log.Info("apikey rate limit config updated", "config", rateLimit) | ||
| for apikey, config := range rateLimit { | ||
| if len(config.RateLimitApis) > 0 { | ||
| log.Info("rate limit enabled", "api", config.RateLimitApis, "count", config.RateLimitCount, "bucket", config.RateLimitBucket) |
| // updateApikeyRateLimit updates the rate limit config | ||
| func updateApikeyRateLimit(rateLimit map[string]*RateLimitConfig) map[string]map[string]*rate.Limiter { | ||
| akrlm := make(map[string]map[string]*rate.Limiter) | ||
| log.Info("apikey rate limit config updated", "config", rateLimit) |
| sync.RWMutex | ||
| } | ||
|
|
||
| var rateLimit = &RateLimit{} |
There was a problem hiding this comment.
We should also initialize the map here instead of checking everywhere in the code whether the pointer is nil or not.
| } | ||
|
|
||
| // updateRateLimit updates the rate limit config | ||
| func updateRateLimit(rateLimit RateLimitConfig) map[string]*rate.Limiter { |
There was a problem hiding this comment.
rateLimit shadowing variable here is confusing and unnecessary
There was a problem hiding this comment.
this function is prepar to be used by apollo dynamic config
No description provided.