Re: libpq compression

Hi, thanks for your review!

> On Mar 19, 2021, at 11:28 AM, Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:
>
> This needs some significant effort:
>
> - squish commits;
> * maybe make an 0001 commit supporting zlib, and an 0002 commit adding zstd;
> * add an 0003 patch to enable zlib compression by default (for CI - not for merge);
> - rebase to current master;

I’ve rebased the chunked compression branch and attached it to this message as two diff patches:
0001-Add-zlib-and-zstd-streaming-compression.patch - this patch introduces general functionality for zlib and zstd streaming compression
0002-Implement-libpq-compression.patch - this patch introduces libpq chunked compression

> - compile without warnings;
> - assign OIDs at the end of the ranges given by find-unused-oids to avoid
> conflicts with patches being merged to master.

Done

> Currently, make check-world gets stuck in several places when I use zlib.
>
> There's commit messages claiming that the compression can be "asymmetric"
> between client-server vs server-client, but the commit seems to be unrelated,
> and the protocol documentation doesn't describe how this works.
>
> Previously, I suggested that the server should have a "policy" GUC defining
> which compression methods are allowed. Possibly including compression "level".
> For example, the default might be to allow zstd, but only up to level 9.

Support for different compression methods in each direction is implemented in zpq_stream.c,
the only thing left to do is to define the GUC settings to control this behavior and make adjustments to the compression handshake process.

Earlier in the thread, I’ve discussed the introduction of compress_algorithms and decompress_algorithms GUC settings with Robert Haas.
The main issue with the decompress_algorithms setting is that the receiving side can’t effectively enforce the actual compression level chosen by the sending side.

So I propose to add the two options to the client and server GUC:
1. compress_algorithms setting with the ability to specify the exact compression level for each algorithm
2. decompress_algorithms to control which algorithms are supported for decompression of the incoming messages

For example:

Server
compress_algorithms = ’zstd:2; zlib:5’ // use the zstd with compression level 2 or zlib with compression level 5 for outgoing messages
decompress_algorithms = ‘zstd; zlib’ // allow the zstd and zlib algorithms for incoming messages

Client
compress_algorithms = ’zstd; zlib:3’ // use the zstd with default compression level (1) or zlib with compression level 3 for outgoing messages
decompress_algorithms = ‘zstd’ // allow the zstd algorithm for incoming messages

Robert, If I missed something from our previous discussion, please let me know. If this approach is OK, I'll implement it.

> This:
> + /* Initialise values and NULL flags arrays */
> + MemSet(values, 0, sizeof(values));
> + MemSet(nulls, 0, sizeof(nulls));
>
> can be just:
> + bool nulls[PG_STAT_NETWORK_TRAFFIC_COLS] = {false};
> since values is fully populated below.
>

The current implementation matches the other functions in pgstatfuncs.c so I think that it is better not to change it.

> typo: aslogirhm

Fixed

> I wrote about Solution.pm earlier in this thread, and I see that it's in
> Konstantin's repo since Jan 12, but it's not in yours (?) so I think windows
> build will fail.
>
> Likewise, commit 1a946e14e in his branch adds compression into to psql
> \connninfo based on my suggestion, but it's missing in your branch.

I've rebased the patch. Now it includes Konstantin's fixes.

—
Daniil Zakhlystov