Make query result HTTP response compression configurable

[pettyjamesm]

Before this change, query result JSON responses were generally compressed (assuming the response met the minimum size threshold and passed the user agent checks), so that behavior is still the default. However, disabling GZIP compression can significantly improve throughput of sending query results, especially over localhost links where the overhead of compressing the response and then uncompressing it again on the client side is never worth the bandwidth savings.

Clients are allowed to opt-out of compression, but not request compression from a server which has decided to disable compressed query result responses. Both sides ultimately negotiate the result based on their Accept-Encoding or Content-Encoding headers and the way that the gzip compression middleware interprets them.

For queries that are bound only by result processing throughput (eg: SELECT * FROM <large table>) execution time can reduced by 20-50% when submitted over a localhost connection with compression disabled.

== RELEASE NOTES ==

General Changes
* Add support for disabling query result compression via client and server-side configuration properties. Clients
can disable compressed responses using the ``--disable-compression`` flag or ``disableCompression`` driver property. Compression can be disabled server-wide by using the configuration property: ``query-results.compression-enabled=false``

[shixuan-fan]

LGTM.

[shixuan-fan]

Can we instead use compressionEnabled? Disabled would incur one more flip in mind :)

[pettyjamesm]

The phrasing is a little weird, but the mechanism here doesn't actually let you force compression- only disable compression. Same thing with the server side logic. Default negotiation happens inside of jetty around the client Accept-Encoding header and server's chosen mime-type and Content-Encoding headers.

[shixuan-fan]

I see. But we could probably just flip the boolean when it is used? I'm just worried this might be error-prone.

[pettyjamesm]

In general I’m worried about the impression that “enabled” will give. To me, that sounds like the client has ultimate control of the decision when in fact, they do not. The client only as the option to opt-out of compression.

[shixuan-fan]

Sorry took the last week off so coming back late. I'm wondering if it actually makes sense if we use an enum for "compression algorithm" (so we could use identity to disable compression), and use gzip as default value? This way we could avoid the enabled/disabled dilemma, and could potentially enable more compression algorithms if available.

[pettyjamesm]

Sorry took the last week off so coming back late. I'm wondering if it actually makes sense if we use an enum for "compression algorithm" (so we could use identity to disable compression), and use gzip as default value? This way we could avoid the enabled/disabled dilemma, and could potentially enable more compression algorithms if available.

I think the problem with that is, as currently implemented, we have no control over the compression algorithm (or indeed, compression level) used by the jetty middleware without some much more significant work. It’s probably possible but it’s much more involved and presumes that different algorithms would actually provide a meaningful middle ground between no compression and gzip (which I doubt, but can’t say for sure without setting up the experiment). If you wanted to add that option in the future it would have to accommodate encoding type negotiation between the client and server which also adds complexity.

[mbasmanova]

@pettyjamesm James, this is a nice change, but I don't see this functionality documented in https://prestodb.io/docs/current/develop/client-protocol.html Any chance you could help update the documentation?

[pettyjamesm]

@pettyjamesm James, this is a nice change, but I don't see this functionality documented in https://prestodb.io/docs/current/develop/client-protocol.html Any chance you could help update the documentation?

I'm happy to update the docs, but I'm not 100% sure where a high level description of the change should go. Fundamentally, the changes here don't depend on any presto-specific headers, it's just leveraging standard HTTP semantics for negotiating the encoding of the response built into the client and server to control whether responses will be gzipped when sent or not so it's not so much a "client protocol" concern as it is maybe a client configuration flag property instead? Would you suggest just adding a documentation entry to the JDBC properties doc?