-
Notifications
You must be signed in to change notification settings - Fork 832
add zstd and snappy compression for query api #6848
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
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
bd8885e
add zstd and snappy compression for query api
afhassan c4a6b86
parse X-Uncompressed-Length only if header exists
afhassan 4f76eea
fix formatting
afhassan ecafc44
refactor query decompression
afhassan cdbb153
ensure zstd reader is closed after decompression
afhassan 968e173
Merge branch 'cortexproject:master' into master
afhassan b9676a9
Merge branch 'cortexproject:master' into master
afhassan 370cdde
add tests for zstd and snappy compression
afhassan cde5f41
update changelog
afhassan 4a51ea2
update docs
afhassan 100bc4d
apply query response size limit after decompression if header is missing
afhassan fe53aa3
Merge branch 'master' into master
afhassan 4ec242b
fix formatting
afhassan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
package queryapi | ||
|
||
import ( | ||
"io" | ||
"net/http" | ||
"strings" | ||
|
||
"github.com/klauspost/compress/gzip" | ||
"github.com/klauspost/compress/snappy" | ||
"github.com/klauspost/compress/zlib" | ||
"github.com/klauspost/compress/zstd" | ||
) | ||
|
||
const ( | ||
acceptEncodingHeader = "Accept-Encoding" | ||
contentEncodingHeader = "Content-Encoding" | ||
gzipEncoding = "gzip" | ||
deflateEncoding = "deflate" | ||
snappyEncoding = "snappy" | ||
zstdEncoding = "zstd" | ||
) | ||
|
||
// Wrapper around http.Handler which adds suitable response compression based | ||
// on the client's Accept-Encoding headers. | ||
type compressedResponseWriter struct { | ||
http.ResponseWriter | ||
writer io.Writer | ||
} | ||
|
||
// Writes HTTP response content data. | ||
func (c *compressedResponseWriter) Write(p []byte) (int, error) { | ||
return c.writer.Write(p) | ||
} | ||
|
||
// Closes the compressedResponseWriter and ensures to flush all data before. | ||
func (c *compressedResponseWriter) Close() { | ||
if zstdWriter, ok := c.writer.(*zstd.Encoder); ok { | ||
zstdWriter.Flush() | ||
} | ||
if snappyWriter, ok := c.writer.(*snappy.Writer); ok { | ||
snappyWriter.Flush() | ||
} | ||
if zlibWriter, ok := c.writer.(*zlib.Writer); ok { | ||
zlibWriter.Flush() | ||
} | ||
if gzipWriter, ok := c.writer.(*gzip.Writer); ok { | ||
gzipWriter.Flush() | ||
} | ||
if closer, ok := c.writer.(io.Closer); ok { | ||
defer closer.Close() | ||
} | ||
} | ||
|
||
// Constructs a new compressedResponseWriter based on client request headers. | ||
func newCompressedResponseWriter(writer http.ResponseWriter, req *http.Request) *compressedResponseWriter { | ||
encodings := strings.Split(req.Header.Get(acceptEncodingHeader), ",") | ||
for _, encoding := range encodings { | ||
switch strings.TrimSpace(encoding) { | ||
case zstdEncoding: | ||
encoder, err := zstd.NewWriter(writer) | ||
if err == nil { | ||
writer.Header().Set(contentEncodingHeader, zstdEncoding) | ||
return &compressedResponseWriter{ResponseWriter: writer, writer: encoder} | ||
} | ||
case snappyEncoding: | ||
writer.Header().Set(contentEncodingHeader, snappyEncoding) | ||
return &compressedResponseWriter{ResponseWriter: writer, writer: snappy.NewBufferedWriter(writer)} | ||
case gzipEncoding: | ||
writer.Header().Set(contentEncodingHeader, gzipEncoding) | ||
return &compressedResponseWriter{ResponseWriter: writer, writer: gzip.NewWriter(writer)} | ||
case deflateEncoding: | ||
writer.Header().Set(contentEncodingHeader, deflateEncoding) | ||
return &compressedResponseWriter{ResponseWriter: writer, writer: zlib.NewWriter(writer)} | ||
} | ||
} | ||
return &compressedResponseWriter{ResponseWriter: writer, writer: writer} | ||
} | ||
|
||
// CompressionHandler is a wrapper around http.Handler which adds suitable | ||
// response compression based on the client's Accept-Encoding headers. | ||
type CompressionHandler struct { | ||
Handler http.Handler | ||
} | ||
|
||
// ServeHTTP adds compression to the original http.Handler's ServeHTTP() method. | ||
func (c CompressionHandler) ServeHTTP(writer http.ResponseWriter, req *http.Request) { | ||
compWriter := newCompressedResponseWriter(writer, req) | ||
c.Handler.ServeHTTP(compWriter, req) | ||
compWriter.Close() | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change means that we need a two phase deployment for querier to return this header for Query Frontend to properly apply the response size limit. Otherwise, the value is 0.
I think we should find a way (maybe a flag) to properly maintain the previous behavior during rollout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't quite get it, do you mean during deployment if Query Frontend is updated first then the limit wouldn't apply because the header is not being returned from Querier?
The old behaviour only works for gzip which is why I changed it. I can reverse it to use the old behaviour when gzip is used and the new one for other compression types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. If we use gzip then this limiter won't work until Querier is updated, which is a breaking behavior.
We just need a way to fallback to the old behavior if it is gzip. Other types are fine since they are new
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If header is missing we now fallback to applying limit using size of the response after decoding it. I think this is a cleaner way to still apply the limit and avoid a breaking change if the header is not there.
Applying the limit before decoding response was an optimization in the old behaviour, but I don't think it is considered breaking if we are still applying the limit.