Received: from malur.postgresql.org ([2a02:16a8:dc51::56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fW39z-00043X-Dw for pgsql-hackers@arkaria.postgresql.org; Thu, 21 Jun 2018 17:14:15 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1fW39x-0003Ub-AK for pgsql-hackers@arkaria.postgresql.org; Thu, 21 Jun 2018 17:14:13 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fW39x-0003UT-0P for pgsql-hackers@lists.postgresql.org; Thu, 21 Jun 2018 17:14:13 +0000 Received: from mx1.redhat.com ([209.132.183.28]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fW39t-0004m4-Mq for pgsql-hackers@lists.postgresql.org; Thu, 21 Jun 2018 17:14:11 +0000 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F101B80F7A; Thu, 21 Jun 2018 17:14:07 +0000 (UTC) Received: from localhost (ovpn-112-58.rdu2.redhat.com [10.10.112.58]) by smtp.corp.redhat.com (Postfix) with ESMTP id A89688BB1E; Thu, 21 Jun 2018 17:14:07 +0000 (UTC) From: Robbie Harwood To: Konstantin Knizhnik , Thomas Munro Cc: Grigory Smolkin , PostgreSQL Hackers Subject: Re: libpq compression In-Reply-To: References: <429cf55b-a8bf-c4ba-196d-5b45420145a1@postgrespro.ru> <36db5d02-45e6-b863-1418-825ef20ae4c6@postgrespro.ru> <3b0260da-fe47-2341-da01-51383fe9172d@postgrespro.ru> <15419613-35b5-0ff6-6261-834abf73330c@postgrespro.ru> <9b3a735c-06c2-631f-8e97-e3982e575210@postgrespro.ru> Date: Thu, 21 Jun 2018 13:14:05 -0400 Message-ID: MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 21 Jun 2018 17:14:08 +0000 (UTC) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --=-=-= Content-Type: text/plain Konstantin Knizhnik writes: > On 21.06.2018 17:56, Robbie Harwood wrote: >> Konstantin Knizhnik writes: >>> On 20.06.2018 23:34, Robbie Harwood wrote: >>>> Konstantin Knizhnik writes: >>>> >>>> Well, that's a design decision you've made. You could put lengths >>>> on chunks that are sent out - then you'd know exactly how much is >>>> needed. (For instance, 4 bytes of network-order length followed by >>>> a complete payload.) Then you'd absolutely know whether you have >>>> enough to decompress or not. >>> >>> Do you really suggest to send extra header for each chunk of data? >>> Please notice that chunk can be as small as one message: dozen of >>> bytes because libpq is used for client-server communication with >>> request-reply pattern. >> >> I want you to think critically about your design. I *really* don't >> want to design it for you - I have enough stuff to be doing. But >> again, the design I gave you doesn't necessarily need that - you just >> need to properly buffer incomplete data. > > Right now secure_read may return any number of available bytes. But in > case of using streaming compression, it can happen that available > number of bytes is not enough to perform decompression. This is why we > may need to try to fetch additional portion of data. This is how > zpq_stream is working now. No, you need to buffer and wait until you're called again. Which is to say: decompress() shouldn't call secure_read(). secure_read() should call decompress(). > I do not understand how it is possible to implement in different way > and what is wrong with current implementation. The most succinct thing I can say is: absolutely don't modify pq_recvbuf(). I gave you pseudocode for how to do that. All of your logic should be *below* the secure_read/secure_write functions. I cannot approve code that modifies pq_recvbuf() in the manner you currently do. >>>> My idea was the following: client want to use compression. But >>>> server may reject this attempt (for any reasons: it doesn't support >>>> it, has no proper compression library, do not want to spend CPU for >>>> decompression,...) Right now compression algorithm is >>>> hardcoded. But in future client and server may negotiate to choose >>>> proper compression protocol. This is why I prefer to perform >>>> negotiation between client and server to enable compression. Well, >>>> for negotiation you could put the name of the algorithm you want in >>>> the startup. It doesn't have to be a boolean for compression, and >>>> then you don't need an additional round-trip. >>> >>> Sorry, I can only repeat arguments I already mentioned: >>> - in future it may be possible to specify compression algorithm >>> - even with boolean compression option server may have some reasons to >>> reject client's request to use compression >>> >>> Extra flexibility is always good thing if it doesn't cost too >>> much. And extra round of negotiation in case of enabling compression >>> seems to me not to be a high price for it. >> >> You already have this flexibility even without negotiation. I don't >> want you to lose your flexibility. Protocol looks like this: >> >> - Client sends connection option "compression" with list of >> algorithms it wants to use (comma-separated, or something). >> >> - First packet that the server can compress one of those algorithms >> (or none, if it doesn't want to turn on compression). >> >> No additional round-trips needed. > > This is exactly how it works now... Client includes compression > option in connection string and server replies with special message > ('Z') if it accepts request to compress traffic between this client > and server. No, it's not. You don't need this message. If the server receives a compression request, it should just turn compression on (or not), and then have the client figure out whether it got compression back. This is of course made harder by your refusal to use packet framing, but still shouldn't be particularly difficult. Thanks, --Robbie --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEA5qc6hnelQjDaHWqJTL5F2qVpEIFAlsr3N0ACgkQJTL5F2qV pEIjKhAAj9/lg9LMx9IUXqmh+qnScX0kFBW0RH0w96MoS8H17aEiBQFt40UL5L6U C1b4m/fo+AXVMIN4gKaccW2VpEqLvPVeN6o+tneQNWSC4RWVhmEB0DrxfaHq6uRb oLLPZyCNjfBVopnZOmRTDU7phRAdnV4mkP7IVHvkYLywpceM5KpL2BLuKgf+YGzx ezi5dQv0idxAlOH0hBSF7PjoYiOv3WWBe85uyVwwb9aSwQRQU8kWVvcQGNJOr0un F/rxQZiU1tFjFeTuCE3faIrKRWKz4rCvotymB4AxYFANI7k7YE+xrbfgJ4DLSAF+ Sp9ucWryhDz7If+GZ0985HITn8FBNcAuBXJgxqvsuHXLWpad/I/qyK3nolafN368 hYENFp8y77mSFPNBJhIh9fqde27pMAcXvlWZl+L19lj4cKe1mNjUc0hUbvfGS12c lG1F39uU9KlcIT7n8o+hUTFTwuMBpGKmzJY6eWxByu7MB38SUi+2af5wOZguJgPA ULg9fijOOcj/PHHnJPI5liMrlHqfJsR1e0HOr/a/LngAJujSu/Ri4/VNsHEBAi0x XFKJmOeHMR3hW0h+yJ0d2gycmvqiLQ5A2rQB0MpEz09jcp86scoKT14+R/sFZEhr 0tcY6fQeC5T9aCA6cKts+zuy6H/r3vs3r51ei2VHn+zwlsAmzk0= =ulnL -----END PGP SIGNATURE----- --=-=-=--