Hi, Justin!
First of all, thanks for the detailed review. I’ve applied your patches to the current version.
> On 1 Jan 2022, at 22:25, Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> I wonder whether the asymmetric compression idea is useful. The only
> application I can see for this is that a server might allow to *decompress*
> data from a client, but may not want to *compress* data to a client.
In the current patch state, there is no option to forbid the server-to-client or client-to-server
compression only. The decision of whether to compress or not is based only on the message length. In
the future, we can implement more precise control without the need for any protocol changes.
> On 1 Jan 2022, at 22:25, Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> I'm not sure it's needed to allow the compression to be changed dynamically,
> and the generality to support a different compression mthod for each libpq
> message type seems excessive. Maybe it's enough to check that the message type
> is one of VALID_LONG_MESSAGE_TYPE and its length is long enough.
Yes, as stated above, in the current patch version protocol message type is ignored but can be potentially
be taken into account. All messages will be compressed using the first available compressor. For
example, if postgresql.conf contains “libpq_compression = zlib,zstd” and has "compression = zlib,zstd”
in its connection string, zlib will be used to compress all messages with length more than the threshold.
> On 1 Jan 2022, at 22:25, Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> What happenes if the compression doesn't decrease the message size ? I don't
> see anything that allows sending the original, raw data. The advantage would
> be that the remote side doesn't incur the overhead of decompression.
Because of the streaming compression, we can’t just throw away some compressed data because future
compressed messages might back-reference it. There are two ways to solve this:
1. Do not use the streaming compression
2. Somehow reset the compression context to the state before the message has been compressed
I’ll look into it. Also, if anyone has any thoughts on this, I’ll appreciate hearing your opinions.
> On 1 Jan 2022, at 22:25, Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> I hit this assertion with PGCOMPRESSION=lz4 (but not zlib), and haven't yet
> found the cause.
Thanks for reporting this. Looks like LZ4 implementation is not polished yet and needs some
additional effort. I'll look into it too.
> On 12 Jan 2022, at 19:15, Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> zlib still causes check-world to get stuck. I first mentioned this last March:
> 20210319062800.GI11765@telsasoft.com
>
> Actually all the compression methods seems to get stuck with
> time make check -C src/bin/pg_rewind
> time make check -C src/test/isolation
>
> For CI purposes, there should be an 0003 patch which enables compression by
> default, for all message types and maybe all lengths.
>
> I removed the thread from the CFBOT until that's resolved.
I’ve fixed the failing tests, now they should pass.
Thanks,
Daniil Zakhlystov