Re: libpq compression - Mailing list pgsql-hackers
From | Andrew Dunstan |
---|---|
Subject | Re: libpq compression |
Date | |
Msg-id | 8d9386be-a567-c526-2fc1-153d6ffd3507@2ndQuadrant.com Whole thread Raw |
In response to | Re: libpq compression (Robert Haas <robertmhaas@gmail.com>) |
Responses |
Re: libpq compression
|
List | pgsql-hackers |
On 08/13/2018 02:47 PM, Robert Haas wrote: > On Fri, Aug 10, 2018 at 5:55 PM, Andrew Dunstan > <andrew.dunstan@2ndquadrant.com> wrote: >> This thread appears to have gone quiet. What concerns me is that there >> appears to be substantial disagreement between the author and the reviewers. >> Since the last thing was this new patch it should really have been put back >> into "needs review" (my fault to some extent - I missed that). So rather >> than return the patch with feedfack I'm going to set it to "needs review" >> and move it to the next CF. However, if we can't arrive at a consensus about >> the direction during the next CF it should be returned with feedback. > I agree with the critiques from Robbie Harwood and Michael Paquier > that the way in that compression is being hooked into the existing > architecture looks like a kludge. I'm not sure I know exactly how it > should be done, but the current approach doesn't look natural; it > looks like it was bolted on. I agree with the critique from Peter > Eisentraut and others that we should not go around and add -Z as a > command-line option to all of our tools; this feature hasn't yet > proved itself to be useful enough to justify that. Better to let > people specify it via a connection string option if they want it. I > think Thomas Munro was right to ask about what will happen when > different compression libraries are in use, and I think failing > uncleanly is quite unacceptable. I think there needs to be some > method for negotiating the compression type; the client can, for > example, provide a comma-separated list of methods it supports in > preference order, and the server can pick the first one it likes. In > short, I think that a number of people have provided really good > feedback on this patch, and I suggest to Konstantin that he should > consider accepting all of those suggestions. > > Commit ae65f6066dc3d19a55f4fdcd3b30003c5ad8dbed tried to introduce > some facilities that can be used for protocol version negotiation as > new features are added, but this patch doesn't use them. It looks to > me like it instead just breaks backward compatibility. The new > 'compression' option won't be recognized by older servers. If it were > spelled '_pq_.compression' then the facilities in that commit would > cause a NegotiateProtocolVersion message to be sent by servers which > have that commit but don't support the compression option. I'm not > exactly sure what will happen on even-older servers that don't have > that commit either, but I hope they'll treat it as a GUC name; note > that setting an unknown GUC name with a namespace prefix is not an > error, but setting one without such a prefix IS an ERROR. Servers > which do support compression would respond with a message indicating > that compression had been enabled or, maybe, just start sending back > compressed-packet messages, if we go with including some framing in > the libpq protocol. > Excellent summary, and well argued recommendations, thanks. I've changed the status to waiting on author. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
pgsql-hackers by date: