Re: libpq compression

From: Konstantin Knizhnik <k(dot)knizhnik(at)postgrespro(dot)ru>
To: Michael Paquier <michael(at)paquier(dot)xyz>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
Cc: Grigory Smolkin <g(dot)smolkin(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: libpq compression
Date: 2018-06-05 15:58:42
Message-ID: 3db573ae-5862-5f77-c0d0-d1dcd4be5dab@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 05.06.2018 10:09, Michael Paquier wrote:
> On Tue, Jun 05, 2018 at 06:04:21PM +1200, Thomas Munro wrote:
>> On Thu, May 17, 2018 at 3:54 AM, Konstantin Knizhnik
>> Speaking of configuration, are you planning to support multiple
>> compression libraries at the same time? It looks like the current
>> patch implicitly requires client and server to use the same configure
>> option, without any attempt to detect or negotiate. Do I guess
>> correctly that a library mismatch would produce an incomprehensible
>> corrupt stream message?
> I just had a quick look at this patch, lured by the smell of your latest
> messages... And it seems to me that this patch needs a heavy amount of
> work as presented. There are a couple of things which are not really
> nice, like forcing the presentation of the compression option in the
> startup packet to begin with. The high-jacking around secure_read() is
> not nice either as it is aimed at being a rather high-level API on top
> of the method used with the backend. On top of adding some
> documentation, I think that you could get some inspiration from the
> recent GSSAPI encription patch which has been submitted again for v12
> cycle, which has spent a large amount of time designing its set of
> options.
> --
> Michael
Thank you for feedback,
I have considered this patch mostly as prototype to estimate efficiency
of libpq  protocol compression and compare it with SSL compression.
So I agree with you that there are a lot of things which should be improved.

But can you please clarify what is wrong with "forcing the presentation
of the compression option in the startup packet to begin with"?
Do you mean that it will be better to be able switch on/off compression
during session?

Also I do not completely understand what do you mean by "high-jacking
around secure_read()".
I looked at GSSAPI patch. It does injection in secure_read:

+#ifdef ENABLE_GSS
+    if (port->gss->enc)
+    {
+        n = be_gssapi_read(port, ptr, len);
+        waitfor = WL_SOCKET_READABLE;
+    }
+    else

But the main difference between encryption and compression is that
encryption is not changing data size, while compression does.
To be able to use streaming compression, I need to specify some function
for reading data from the stream. I am using secure_read for this purpose:

       PqStream = zpq_create((zpq_tx_func)secure_write,
(zpq_rx_func)secure_read, MyProcPort);

Can you please explain what is the problem with it?

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2018-06-05 16:06:47 Re: Code of Conduct plan
Previous Message David Fetter 2018-06-05 15:50:27 Re: Spilling hashed SetOps and aggregates to disk