Re: libpq compression

On 2/9/19 3:14 PM, Andres Freund wrote:
> Hi,
> 
> On 2019-02-08 23:38:12 +0100, Tomas Vondra wrote:
>> On 2/8/19 11:10 PM, Konstantin Knizhnik wrote:
>>> Does it mean that it is necessary to support multiple compression
>>> algorithms and make it possible to perform switch between them at
>>> runtime?
>>
>> IMHO the negotiation should happen at connection time, i.e. the server
>> should support connections compressed by different algorithms.
> 
> Exactly. And support client libraries with support for different
> compression algorithms. We have large forward and backward compatibility
> with libpq and even moreso with the wire protocol, we therefore
> shouldn't require exactly matching client / server versions or
> compilation options.
> 

IMHO the main reason to want/need this is not as much backward/forward
compatibility (in the sense of us adding/removing supported algorithms).
A much bigger problem (IMHO) is that different systems may not have some
of the libraries needed, or maybe the packager decided not to enable a
particular library, etc. That's likely far more dynamic.

> 
>> AFAICS this is quite close to how negotiation of encryption algorithms
>> works, in TLS and so on. Client specifies supported algorithms, server
>> compares that to list of supported algorithms, deduces the encryption
>> algorithm and notifies the client.
>>
>> To allow fall-back to uncompressed connection, use "none" as algorithm.
>> If there's no common algorithm, fail.
> 
> I'm somewhat inclined to think that not compressing is
> preferrable. There's some reason to think that there's some
> cryptographical issues around compression, therefore it could make a ton
> of sense for specific servers to disable compression centrally.
> 

I agree compression is not the same as crypto in this case, so fallback
to uncompressed connection might be sensible in some cases. But in other
cases I probably want to be notified ASAP that the compression does not
work, before pushing 10x the amount of data through the network. That's
why I think why I proposed to also have "none" as compression algorithm,
to allow/disallow the fallback.

> 
>>> Right now compression algorithm is linked statically.
>>> Negotiation of compression type is currently performed but it only
>>> checks that server and client are implementing the same algorithm and
>>> disables compression if it is not true.
> 
>> I don't think we should automatically fall-back to disabled compression,
>> when a client specifies compression algorithm.
> 
> Why?
> 

Because it's pretty difficult to realize it happened, particularly for
non-interactive connections. Imagine you have machines where transfer
between them is not free - surely you want to know when you suddenly get
10x the traffic, right?

But as I said above, this should be configurable. IMHO having "none"
algorithm to explicitly enable this seems like a reasonable solution.

> 
>>> If we are going to support multiple compression algorithms, do we need
>>> dynamic loading of correspondent compression libraries or static linking
>>> is ok? In case of dynamic linking we need to somehow specify information
>>> about available compression algorithms.
>>> Some special subdirectory for them so that I can traverse this directory
>>> and try to load correspondent libraries?
>>>
>>> Only I find it too complicated for the addressed problem?
>>>
>>
>> I don't think we need dynamic algorithms v1, but IMHO it'd be pretty
>> simple to do - just add a shared_preload_library which registers it in a
>> list in memory.
> 
> I personally don't think that's something we need to support. There's a
> lot of issues around naming registries that need to synchronized between
> client/server.
> 

Don't we have that issue even without the dynamic registration? Let's
say you have custom driver implementing the protocol (but not based on
libpq). Surely that assumes the algorithm names match what we have?

In the worst case the decompression fails (which may happen already
anyway) and the connection dies. Not a big deal, no?

But I agree v1 should not include this dynamic registration.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services