Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drains tokeninfo response body to enable TCP connection reuse #1511

Merged
merged 1 commit into from
Sep 1, 2020

Conversation

AlexanderYastrebov
Copy link
Member

AuthClient used by oauthTokeninfo* filters does not consume response body when token is invalid:

defer rsp.Body.Close()
if rsp.StatusCode != 200 {
return doc, errInvalidToken
}

According to https://golang.org/pkg/net/http/#Client.Do

If the Body is not both read to EOF and closed, the Client's underlying RoundTripper (typically Transport)
may not be able to re-use a persistent TCP connection to the server for a subsequent "keep-alive" request.

That is requests with invalid token prevent tokeninfo TCP connection reuse which leads to higher latencies.

Consider the following setup:

$ cat /tmp/tokeninfo.eskip 
tokeninfo: Path("/oauth2/tokeninfo")
-> status(400) -> inlineContent("{\"error\":\"invalid_request\",\"error_description\":\"Access Token not valid\"}")
-> <shunt>;

test: Path("/test")
-> oauthTokeninfoAllScope("foo", "bar")
-> status(204)
-> <shunt>;

and

bin/skipper -access-log-disabled -oauth2-tokeninfo-url="http://localhost:9090/oauth2/tokeninfo" -routes-file=/tmp/tokeninfo.eskip 

Benchmark results:

$ wrk --latency -H "Authorization: Bearer X" -c 4 -d 12 http://localhost:9090/test
Running 12s test @ http://localhost:9090/test
  2 threads and 4 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.39ms    1.37ms   9.61ms   80.53%
    Req/Sec     1.91k     1.93k    4.87k    67.50%
  Latency Distribution
     50%  528.00us
     75%    2.34ms
     90%    3.54ms
     99%    5.25ms
  45670 requests in 12.01s, 6.32MB read
  Non-2xx or 3xx responses: 45670
Requests/sec:   3803.71
Transfer/sec:    538.61KB

Consuming response body with:

@@ -110,6 +112,7 @@ func (ac *authClient) getTokeninfo(token string, ctx filters.FilterContext) (map
        defer rsp.Body.Close()
 
        if rsp.StatusCode != 200 {
+               io.Copy(ioutil.Discard, rsp.Body)
                return doc, errInvalidToken
        }

reduces latencies severalfold:

$ wrk --latency -H "Authorization: Bearer X" -c 4 -d 12 http://localhost:9090/test
Running 12s test @ http://localhost:9090/test
  2 threads and 4 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   291.47us  320.41us   8.19ms   92.99%
    Req/Sec     8.17k     0.85k   10.45k    76.76%
  Latency Distribution
     50%  190.00us
     75%  268.00us
     90%  518.00us
     99%    1.79ms
  195906 requests in 12.10s, 27.09MB read
  Non-2xx or 3xx responses: 195906
Requests/sec:  16191.23
Transfer/sec:      2.24MB

See also:

See https://golang.org/pkg/net/http/#Client.Do for details

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
@szuecs
Copy link
Member

szuecs commented Aug 25, 2020

👍

2 similar comments
@mikkeloscar
Copy link
Member

👍

@aryszka
Copy link
Contributor

aryszka commented Sep 1, 2020

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants