Skip to content

Improving Performance on the API Gzip Handler #5347

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

Merged
merged 4 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* [ENHANCEMENT] Compactor: Exposing Thanos accept-malformed-index to Cortex compactor. #5334
* [ENHANCEMENT] Update Go version to 1.20.4. #5299
* [ENHANCEMENT] Log: Avoid expensive log.Valuer evaluation for disallowed levels. #5297
* [ENHANCEMENT] Improving Performance on the API Gzip Handler. #5347
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the changelog? It should be an internal details and users don't need to know about that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hummm... i think you are right... Should i remove in another PR now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you can update it in another pr

* [BUGFIX] Ruler: Validate if rule group can be safely converted back to rule group yaml from protobuf message #5265
* [BUGFIX] Querier: Convert gRPC `ResourceExhausted` status code from store gateway to 422 limit error. #5286
* [BUGFIX] Alertmanager: Route web-ui requests to the alertmanager distributor when sharding is enabled. #5293
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ go 1.18

require (
github.com/Masterminds/squirrel v1.5.4
github.com/NYTimes/gziphandler v1.1.1
github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137
github.com/alicebob/miniredis/v2 v2.30.1
github.com/armon/go-metrics v0.4.1
Expand Down
1 change: 0 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,6 @@ github.com/Microsoft/hcsshim v0.9.2/go.mod h1:7pLA8lDk46WKDWlVsENo92gC0XFa8rbKfy
github.com/Microsoft/hcsshim/test v0.0.0-20201218223536-d3e5debf77da/go.mod h1:5hlzMzRKMLyo42nCZ9oml8AdTlq/0cvIaBv6tK1RehU=
github.com/Microsoft/hcsshim/test v0.0.0-20210227013316-43a75bb4edd3/go.mod h1:mw7qgWloBUl75W/gVH3cQszUg1+gUITj7D6NY7ywVnY=
github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ=
github.com/NYTimes/gziphandler v1.1.1 h1:ZUDjpQae29j0ryrS0u/B8HZfJBtBQHjqw2rQ2cqUQ3I=
github.com/NYTimes/gziphandler v1.1.1/go.mod h1:n/CVRwUEOgIxrgPvAQhUUr9oeUtvrhMomdKFjzJNB0c=
github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
github.com/OneOfOne/xxhash v1.2.6 h1:U68crOE3y3MPttCMQGywZOLrTeF5HHJ3/vDBCJn9/bA=
Expand Down
6 changes: 3 additions & 3 deletions pkg/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import (
"path"
"strings"

"github.com/NYTimes/gziphandler"
"github.com/felixge/fgprof"
"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/klauspost/compress/gzhttp"
"github.com/prometheus/prometheus/storage"
"github.com/weaveworks/common/middleware"
"github.com/weaveworks/common/server"
Expand Down Expand Up @@ -145,7 +145,7 @@ func (a *API) RegisterRoute(path string, handler http.Handler, auth bool, method
}

if a.cfg.ResponseCompression {
handler = gziphandler.GzipHandler(handler)
handler = gzhttp.GzipHandler(handler)
}
if a.HTTPHeaderMiddleware != nil {
handler = a.HTTPHeaderMiddleware.Wrap(handler)
Expand All @@ -165,7 +165,7 @@ func (a *API) RegisterRoutesWithPrefix(prefix string, handler http.Handler, auth
}

if a.cfg.ResponseCompression {
handler = gziphandler.GzipHandler(handler)
handler = gzhttp.GzipHandler(handler)
}
if a.HTTPHeaderMiddleware != nil {
handler = a.HTTPHeaderMiddleware.Wrap(handler)
Expand Down
117 changes: 117 additions & 0 deletions pkg/api/api_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
package api

import (
"encoding/json"
"fmt"
"io"
"net/http"
"testing"

"github.com/gorilla/mux"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/prometheus/model/labels"
"github.com/stretchr/testify/require"
"github.com/weaveworks/common/server"
)

const (
acceptEncodingHeader = "Accept-Encoding"
gzipEncoding = "gzip"
)

type FakeLogger struct{}

func (fl *FakeLogger) Log(keyvals ...interface{}) error {
Expand Down Expand Up @@ -94,3 +105,109 @@ func TestNewApiWithoutHeaderLogging(t *testing.T) {
require.Nil(t, api.HTTPHeaderMiddleware)

}

func Benchmark_Compression(b *testing.B) {
client := &http.Client{
Transport: &http.Transport{
DisableCompression: true,
},
}

cfg := Config{
ResponseCompression: true,
}

cases := map[string]struct {
enc string
numberOfLabels int
}{
"gzip-10-labels": {
enc: gzipEncoding,
numberOfLabels: 10,
},
"gzip-100-labels": {
enc: gzipEncoding,
numberOfLabels: 100,
},
"gzip-1K-labels": {
enc: gzipEncoding,
numberOfLabels: 1000,
},
"gzip-10K-labels": {
enc: gzipEncoding,
numberOfLabels: 10000,
},
"gzip-100K-labels": {
enc: gzipEncoding,
numberOfLabels: 100000,
},
"gzip-1M-labels": {
enc: gzipEncoding,
numberOfLabels: 1000000,
},
}

for name, tc := range cases {
b.Run(name, func(b *testing.B) {
serverCfg := server.Config{
HTTPListenNetwork: server.DefaultNetwork,
HTTPListenPort: 8080,
Registerer: prometheus.NewRegistry(),
}

server, err := server.New(serverCfg)
require.NoError(b, err)
api, err := New(cfg, serverCfg, server, &FakeLogger{})
require.NoError(b, err)

labels := labels.ScratchBuilder{}

for i := 0; i < tc.numberOfLabels; i++ {
labels.Add(fmt.Sprintf("Name%v", i), fmt.Sprintf("Value%v", i))
}

respBody, err := json.Marshal(labels.Labels())
require.NoError(b, err)

api.RegisterRoute("/foo_endpoint", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
_, err := w.Write(respBody)
require.NoError(b, err)
}), false, "GET")

go func() {
err := server.Run()
require.NoError(b, err)
}()

defer server.Shutdown()
req, _ := http.NewRequest("GET", "http://"+server.HTTPListenAddr().String()+"/foo_endpoint", nil)
req.Header.Set(acceptEncodingHeader, "gzip")

b.ReportAllocs()
b.ResetTimer()

// Reusing the array to read the body and avoid allocation on the test
encRespBody := make([]byte, len(respBody))

for i := 0; i < b.N; i++ {
resp, err := client.Do(req)

require.NoError(b, err)

require.NoError(b, err, "client get failed with unexpected error")

responseBodySize := 0
for {
n, err := resp.Body.Read(encRespBody)
responseBodySize += n
if err == io.EOF {
break
}
}

b.ReportMetric(float64(responseBodySize), "ContentLength")
}
})
}
}
1 change: 0 additions & 1 deletion vendor/github.com/NYTimes/gziphandler/.gitignore

This file was deleted.

10 changes: 0 additions & 10 deletions vendor/github.com/NYTimes/gziphandler/.travis.yml

This file was deleted.

75 changes: 0 additions & 75 deletions vendor/github.com/NYTimes/gziphandler/CODE_OF_CONDUCT.md

This file was deleted.

30 changes: 0 additions & 30 deletions vendor/github.com/NYTimes/gziphandler/CONTRIBUTING.md

This file was deleted.

56 changes: 0 additions & 56 deletions vendor/github.com/NYTimes/gziphandler/README.md

This file was deleted.

Loading