{"action":"created","issue":{"url":"https://api.github.com/repos/GoogleCloudPlatform/kubernetes/issues/3773","labels_url":"https://api.github.com/repos/GoogleCloudPlatform/kubernetes/issues/3773/labels{/name}","comments_url":"https://api.github.com/repos/GoogleCloudPlatform/kubernetes/issues/3773/comments","events_url":"https://api.github.com/repos/GoogleCloudPlatform/kubernetes/issues/3773/events","html_url":"https://github.com/GoogleCloudPlatform/kubernetes/issues/3773","id":55338473,"number":3773,"title":"Rate Limiter has initialization race condition","user":{"login":"satnam6502","id":6164339,"avatar_url":"https://avatars.githubusercontent.com/u/6164339?v=3","gravatar_id":"","url":"https://api.github.com/users/satnam6502","html_url":"https://github.com/satnam6502","followers_url":"https://api.github.com/users/satnam6502/followers","following_url":"https://api.github.com/users/satnam6502/following{/other_user}","gists_url":"https://api.github.com/users/satnam6502/gists{/gist_id}","starred_url":"https://api.github.com/users/satnam6502/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/satnam6502/subscriptions","organizations_url":"https://api.github.com/users/satnam6502/orgs","repos_url":"https://api.github.com/users/satnam6502/repos","events_url":"https://api.github.com/users/satnam6502/events{/privacy}","received_events_url":"https://api.github.com/users/satnam6502/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/GoogleCloudPlatform/kubernetes/labels/area/reliability","name":"area/reliability","color":"a10d01"},{"url":"https://api.github.com/repos/GoogleCloudPlatform/kubernetes/labels/priority/P3","name":"priority/P3","color":"fef2c0"}],"state":"closed","locked":false,"assignee":{"login":"brendandburns","id":5751682,"avatar_url":"https://avatars.githubusercontent.com/u/5751682?v=3","gravatar_id":"","url":"https://api.github.com/users/brendandburns","html_url":"https://github.com/brendandburns","followers_url":"https://api.github.com/users/brendandburns/followers","following_url":"https://api.github.com/users/brendandburns/following{/other_user}","gists_url":"https://api.github.com/users/brendandburns/gists{/gist_id}","starred_url":"https://api.github.com/users/brendandburns/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/brendandburns/subscriptions","organizations_url":"https://api.github.com/users/brendandburns/orgs","repos_url":"https://api.github.com/users/brendandburns/repos","events_url":"https://api.github.com/users/brendandburns/events{/privacy}","received_events_url":"https://api.github.com/users/brendandburns/received_events","type":"User","site_admin":false},"milestone":null,"comments":3,"created_at":"2015-01-23T21:58:17Z","updated_at":"2015-01-26T19:18:12Z","closed_at":"2015-01-26T19:07:40Z","body":"I've been looking into why the TestROService Go end-to-end test often fails with a rate limit error. It only makes one API call but it makes that call pretty much immediately:\r\n```\r\nfunc TestKubernetesROService(c *client.Client) bool {\r\n\tsvc := api.ServiceList{}\r\n\terr := c.Get().\r\n\t\tNamespace(\"default\").\r\n\t\tAbsPath(\"/api/v1beta1/proxy/services/kubernetes-ro/api/v1beta1/services\").\r\n\t\tDo().\r\n\t\tInto(&svc)\r\n```\r\nNow that a fresh client is created for each test, we can encounter a race where the channel used to represent tokens for service availability are not yet pre-charged before a client can make a call. Here is the code in handler.go that checks for tokens:\r\n```\r\n// RateLimit uses rl to rate limit accepting requests to 'handler'.\r\nfunc RateLimit(rl util.RateLimiter, handler http.Handler) http.Handler {\r\n\treturn http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {\r\n\t\tif rl.CanAccept() {\r\n\t\t\thandler.ServeHTTP(w, req)\r\n\t\t\treturn\r\n\t\t}\r\n\t\tw.WriteHeader(http.StatusServiceUnavailable)\r\n\t\tw.Header().Set(\"Retry-After\", \"1\")\r\n\t\tfmt.Fprintf(w, \"Rate limit exceeded.\")\r\n\t})\r\n}\r\n```\r\nThe problem is the `cl.CanAccept()` can be called before the token channel is populated. The rate limiting is done in throttle.go and here are some relevant snippet.\r\n\r\nFirst, the code that creates the ticker. This will insert tokens into the `ticker` channel at the QPS rate.\r\n```\r\nfunc NewTokenBucketRateLimiter(qps float32, burst int) RateLimiter {\r\n\tticker := time.Tick(time.Duration(float32(time.Second) / qps))\r\n\trate := newTokenBucketRateLimiterFromTicker(ticker, burst)\r\n\tgo rate.run()\r\n\treturn rate\r\n}\r\n```\r\n\r\nThe initialization will pre-charge the channel to provide a window in which a burst is permitted as long as the QPS is not violated over a window that is the size of the burst.\r\n```\r\nfunc newTokenBucketRateLimiterFromTicker(ticker <-chan time.Time, burst int) *tickRateLimiter {\r\n\tif burst < 1 {\r\n\t\tpanic(\"burst must be a positive integer\")\r\n\t}\r\n\trate := &tickRateLimiter{\r\n\t\ttokens: make(chan bool, burst),\r\n\t\tticker: ticker,\r\n\t\tstop:   make(chan bool),\r\n\t}\r\n\tfor i := 0; i < burst; i++ {\r\n\t\trate.tokens <- true\r\n\t}\r\n\treturn rate\r\n}\r\n```\r\nThe problem is that there is a short period where neither the first tick nor the pre-charge have been executed but a request could already have been made -- which would be rejected (incorrectly) for rate limit reasons.\r\n\r\nHopefully the Retry-After PR #3704 that I submitted will mask this situation.\r\n@filbranden \r\n"},"comment":{"url":"https://api.github.com/repos/GoogleCloudPlatform/kubernetes/issues/comments/71519587","html_url":"https://github.com/GoogleCloudPlatform/kubernetes/issues/3773#issuecomment-71519587","issue_url":"https://api.github.com/repos/GoogleCloudPlatform/kubernetes/issues/3773","id":71519587,"user":{"login":"satnam6502","id":6164339,"avatar_url":"https://avatars.githubusercontent.com/u/6164339?v=3","gravatar_id":"","url":"https://api.github.com/users/satnam6502","html_url":"https://github.com/satnam6502","followers_url":"https://api.github.com/users/satnam6502/followers","following_url":"https://api.github.com/users/satnam6502/following{/other_user}","gists_url":"https://api.github.com/users/satnam6502/gists{/gist_id}","starred_url":"https://api.github.com/users/satnam6502/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/satnam6502/subscriptions","organizations_url":"https://api.github.com/users/satnam6502/orgs","repos_url":"https://api.github.com/users/satnam6502/repos","events_url":"https://api.github.com/users/satnam6502/events{/privacy}","received_events_url":"https://api.github.com/users/satnam6502/received_events","type":"User","site_admin":false},"created_at":"2015-01-26T19:18:12Z","updated_at":"2015-01-26T19:18:12Z","body":"@brendandburns Let me explain a but more my confusion. This is where you return the rate limiter:\r\n```\r\nfunc NewTokenBucketRateLimiter(qps float32, burst int) RateLimiter {\r\n\tticker := time.Tick(time.Duration(float32(time.Second) / qps))\r\n\trate := newTokenBucketRateLimiterFromTicker(ticker, burst)\r\n\tgo rate.run()\r\n\treturn rate\r\n}\r\n```\r\nWhat pre-charges the channel with tickeds? The goroutine that asynchronously runs `rate.run()` relative to this function. So I thought the returned rate could be used before the goroutine had a chance to fill up the channel. "}}