[HACKERS] Re: protocol version negotiation (Re: Libpq PGRES_COPY_BOTH - versioncompatibility)

Hello,

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

A big thanks to @Satyanarayana Narlapuram 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:

              src/

              └── backend/

                           └── postmaster/

                               └── postmaster.c

                           └── utils/misc/

                               └── guc.c

              src/

              └── 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 present.

              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:

              src/

              └── Makefile

              src/

              └── Makefile.global.in

              src/

              └──       bin/

                           └── pg_dump/

                               └── pg_dump.c

                           └── scripts/

                               ├── clusterdb.c

                               ├── createuser.c

                               ├── reindexdb.c

                               └── vacuumdb.c

              src/

              └── common/

                  └── Makefile

              src/

              └── fe_utils/

                  └── Makefile

                           └── simple_list.c

              src/

              └── include/

                           └── fe_utils/

                               └── simple_list.h

                           └── libpq/

                               └── pqcomm.h

              src/

              └── interfaces/libpq/

                           ├── fe-connect.c

                           ├── fe-protocol3.c

                           ├── libpq-fe.h

                           └── libpq-int.h

              src/

              └── 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 modified

                                         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 /src/include/libpq/pqcomm.h

/***********/

/* 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 /src/interfaces/libpq/fe-connect.c

                                         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 /src/interfaces/libpq/fe-connect.c

                                         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 PG_PROTOCOL(2, 0) to PG_PROTOCOL(3, 1)

                                         2.a.3 conn->pversion = PG_PROTOCOL(3, 0); succeeds now and succeeded before as well as it is in the supported range PG_PROTOCOL(2, 0) to PG_PROTOCOL(3, 1)

              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

On Thu, Jun 29, 2017 at 7:29 PM, Satyanarayana Narlapuram
<Satyanarayana.Narlapuram@microsoft.com> wrote:
> -----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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers