Kindly review this patch that implements the proposal for pgwire protocol 
negotiation described in this thread.

A big thanks to @Satyanarayana 
Narlapuram<mailto:satyanarayana.narlapu...@microsoft.com> for his help and 
guidance in implementing the patch.

Please note:
1. Pgwire protocol v3.0 with negotiation is called v3.1.
2. There are 2 patches for the change: a BE-specific patch that will be 
backported and a FE-specific patch that is only for pg10 and above.

/* Feature In A Nutshell */
This feature enables a newer FE to connect to an older BE using a protocol 
negotiation phase in the connection setup. The FE sends its protocol version 
and a list of startup parameters to the BE to start a connection. If the FE 
pgwire version is newer than the BE's or if the BE could not recognize some of 
the startup parameters,
the BE simply ignores the parameters that it did not recognize and sends a 
ServerProtocolVersion message containing the minimum and maximum versions it 
supports as well as a list of the parameters that it did honour. The FE then 
decides whether it wants to continue connecting- in the patch, the FE continues 
only if the parameters that the BE did not honour were optional, where optional 
parameters are denoted by a proper prefix of "_pq_" in their names.

/* BE Patch */

Files modified:

              └── backend/
                           └── postmaster/
                               └── postmaster.c
                           └── utils/misc/
                               └── guc.c

              └── include/postmaster/
                           └── postmaster.h

Design decisions:

              1.           The BE sends a message type of ServerProtocolVersion 
if the FE is newer or the FE is not using v3.0
                           1.a Message structure: [ 'Y' | msgLength | 
min_version | max_version | param_list_len | list of param names ]
                           1.b 'Y' was selected to denote ServerProtocolVersion 
messages as it is the first available character from the bottom of the list of 
available characters that is not being used to denote any message type at 

              2.           Added support for BE to accept optional parameters
                           2.a An optional parameter is a startup parameter 
with "_pq_" as a proper prefix in its name. The check to add a placeholder in 
/src/backend/utils/misc/guc.c was modified to allow optional parameters.
                           2.b The string comparison in is_optional() is 
encoding-aware as the BE's encoding might be different from the FE's.

/* FE Patch */

Files modified:

              └── Makefile

              └── Makefile.global.in

              └──       bin/
                           └── pg_dump/
                               └── pg_dump.c
                           └── scripts/
                               ├── clusterdb.c
                               ├── createuser.c
                               ├── reindexdb.c
                               └── vacuumdb.c

              └── common/
                  └── Makefile

              └── fe_utils/
                  └── Makefile
                           └── simple_list.c

              └── include/
                           └── fe_utils/
                               └── simple_list.h
                           └── libpq/
                               └── pqcomm.h

              └── interfaces/libpq/
                           ├── fe-connect.c
                           ├── fe-protocol3.c
                           ├── libpq-fe.h
                           └── libpq-int.h

              └── tools/msvc/
                           └── Mkvcbuild.pm

Design decisions:

              1.           Added mechanism to send startup parameters to BE:
                           SimpleStringList startup_parameters in 
/src/interfaces/libpq/fe-connect.c enables sending parameters to the BE. The 
startup_parameters list is parsed in /src/interfaces/libpq/fe-protocol3.c and 
the arguments are sent in the startup packet to the BE.

                           This is mostly used for testing at this point- one 
would send additional startup parameters like so in PQconnectStartParams():

simple_string_list_append(&startup_parameters, "_pq_A");
simple_string_list_append(&startup_parameters, "1");
                                         startup_parameters.immutable = true; 
// PQconnectStartParams() is called twice for the same connection; the 
immutable flag ensures that the same parameter is not added twice

              2.           Added a CONNECTION_NEGOTIATING state in 
/src/interfaces/libpq/fe-connect.c to parse the ServerProtocolVersion message 
from the BE
                           2.a The FE terminates the connection if BE rejected 
non-optional parameters.

              3.           The changes to the following files were due to the 
addition of the immutable field in the SimpleStringList struct
                           3.a /src/bin/pg_dump/pg_dump.c
                           3.b /src/bin/scripts/clusterdb.c
                           3.c /src/bin/scripts/createuser.c
                           3.d /src/bin/scripts/reindexdb.c
                           3.e /src/bin/scripts/vacuumdb.c

              4.           Added some dependencies to the libpq project to be 
able to use SimpleStringList in fe-connect.c.
                           4.a Visual C compiler
                                         4.a.1 /src/tools/msvc/Mkvcbuild.pm was 
modified to add the reference for visual c compiler
                           4.b Non-windows compilers: the following files were 
                                         4.b.1 /src/Makefile
                                         4.b.2 /src/Makefile.global.in
                                         4.b.3 /src/common/Makefile
                                         4.b.4 /src/fe_utils/Makefile

              5.           Bumped max_pg_protocol version from 3.0 to 3.1 in 

/* Testing */

An outline of the testing framework used to validate the code:

              1.           Newer FE can connect to older BE
                           1.a Change FE protocol version in line 1811 of 
                                         1.a.1 conn->pversion = PG_PROTOCOL(3, 
1); succeeds now whereas it would have failed before: a result of bumping 
max_pg_protocol from 3.0 to 3.1
                                         1.a.2 conn->pversion = PG_PROTOCOL(4, 
0); succeds now whereas it would have failed before: older BE is capable of 
handling newer FE now

              2.           Older drivers work as expected
                           2.a Change FE protocol version in line 1811 of 
                                         2.a.1 conn->pversion = PG_PROTOCOL(1, 
9); fails now and failed before as well as it is below 
min_pg_protocol=PG_PROTOCOL(2, 0)
                                         2.a.2 conn->pversion = PG_PROTOCOL(2, 
0); succeeds now and succeeded before as well as it is in the supported range 
                                         2.a.3 conn->pversion = PG_PROTOCOL(3, 
0); succeeds now and succeeded before as well as it is in the supported range 

              3.           BE support for optional parameters
                           3.a Accept names with "_pq_" as proper prefix, eg 
"_pq_A" would succeed
                           3.b Reject all others without crashing
                                         3.b.1 Any string not like "_pq_A" 
should fail

Looking forward to your feedback,
Thank you,
Badrul Chowdhury

From: pgsql-hackers-ow...@postgresql.org 
[mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas
Sent: Sunday, July 2, 2017 3:45 PM
To: Satyanarayana Narlapuram <satyanarayana.narlapu...@microsoft.com>
Cc: Tom Lane <t...@sss.pgh.pa.us>; Craig Ringer <cr...@2ndquadrant.com>; Peter 
Eisentraut <pete...@gmx.net>; Magnus Hagander <mag...@hagander.net>; 
PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: protocol version negotiation (Re: Libpq PGRES_COPY_BOTH - version 

On Thu, Jun 29, 2017 at 7:29 PM, Satyanarayana Narlapuram
> -----Original Message-----

The formatting of this message differs from the style normally used on
this mailing list, and is hard to read.

> 2. If the client version is anything other than 3.0, the server responds with 
> a ServerProtocolVersion indicating the highest version it supports, and 
> ignores any pg_protocol.<whatever> options not known to it as being either 
> third-party extensions or something from a future version. If the initial 
> response to the startup message is anything other than a 
> ServerProtocolVersion message, the client should assume it's talking to a 3.0 
> server. (To make this work, we would back-patch a change into existing 
> releases to allow any 3.x protocol version and ignore any 
> pg_protocol.<whatever> options that were specified.)
> We can avoid one round trip if the server accepts the startupmessage as is 
> (including understanding all the parameters supplied by the client), and in 
> the cases where server couldn’t accept the startupmessage / require 
> negotiation, it should send ServerProtocolVersion message that contains both 
> MIN and MAX versions it can support. Providing Min version helps server 
> enforce the client Min protocol version, and provides a path to deprecate 
> older versions. Thoughts?

With this latest proposal, there are no extra round-trips anyway. I
don't much see the point of having the server advertise a minimum
supported version. The idea of new minor protocol versions is to add
*optional* features, so there shouldn't be an issue with the client
being too old to talk to the server altogether. Of course, the server
might be configured to reject the client unless some particular new
feature is in use, but that's best handled by a message about the
specific problem at hand rather than a generic complaint.

> Does the proposal also include the client can negotiate the protocol version 
> on the same connection rather than going through connection setup process 
> again? The state machine may not sound simple with this proposal but helps 
> bringing down total time taken for the login.

Nothing in that proposal involved an extra connection setup process;
if that's not clear, you might want to reread it.

Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Sent via pgsql-hackers mailing list 
To make changes to your subscription:

Attachment: be_pgwire3.1.patch
Description: be_pgwire3.1.patch

Attachment: fe_pgwire3.1.patch
Description: fe_pgwire3.1.patch

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to