RE: libpq compression

From: "Iwata, Aya" <iwata(dot)aya(at)jp(dot)fujitsu(dot)com>
To: 'Konstantin Knizhnik' <k(dot)knizhnik(at)postgrespro(dot)ru>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "rharwood(at)redhat(dot)com" <rharwood(at)redhat(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, "g(dot)smolkin(at)postgrespro(dot)ru" <g(dot)smolkin(at)postgrespro(dot)ru>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, 'Dmitry Dolgov' <9erthalion6(at)gmail(dot)com>
Subject: RE: libpq compression
Date: 2019-02-08 07:01:01
Message-ID: 71E660EB361DF14299875B198D4CE5423DED67E5@g01jpexmbkw25
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

I am sorry for my late reply.

> I fixed all issues you have reported except using list of supported compression
> algorithms.
Sure. I confirmed that.

> It will require extra round of communication between client and server to
> make a decision about used compression algorithm.
In beginning of this thread, Robbie Harwood said that no extra communication needed.
I think so, too.

> I still not sure whether it is good idea to make it possible to user to
> explicitly specify compression algorithm.
> Right now used streaming compression algorithm is hardcoded and depends on
> --use-zstd ort --use-zlib configuration options.
> If client and server were built with the same options, then they are able
> to use compression.
I understand that compression algorithm is hardcoded in your proposal.
However given the possibility of future implementation, I think
it would be better for it to have a flexibility to choose compression library.

src/backend/libpq/pqcomm.c :
In current Postgres source code, pq_recvbuf() calls secure_read()
and pq_getbyte_if_available() also calls secure_read().
It means these functions are on the same level.
However in your change, pq_getbyte_if_available() calls pq_recvbuf(),
and pq_recvbuf() calls secure_read(). The level of these functions is different.

I think the purpose of pq_getbyte_if_available() is to get a character if it exists and
the purpose of pq_recvbuf() is to acquire data up to the expected length.
In your change, pq_getbyte_if_available() may have to do unnecessary process waiting or something.

So how about changing your code like this?
The part that checks whether it is compressed is implemented as a #define macro(like fe_misc.c). And pq_recvbuf() and pq_getbyte_if_available() modify little, like this;

- r = secure_read(MyProcPort, PqRecvBuffer + PqRecvLength,
- PQ_RECV_BUFFER_SIZE - PqRecvLength);
+ r = SOME_DEFINE_NAME_();

configure:
Adding following message to the top of zlib in configure
```
{$as_echo "$as_me:${as_lineno-$LINENO}:checking whethere to build with zstd support">&5
$as_echo_n "checking whether to build with zstd suppor... ">&6;}
```

Regards,
Aya Iwata

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-02-08 07:08:28 Re: libpq compression
Previous Message Amit Langote 2019-02-08 06:51:43 Re: speeding up planning with partitions