search

agroal / pgagroal

High-performance connection pool for PostgreSQL
https://agroal.github.io/pgagroal/
BSD 3-Clause "New" or "Revised" License
684 stars 61 forks source link

Management protocol incompatibilites due to version numbers #447

Open fluca1978 opened 6 months ago

fluca1978 commented 6 months ago

Now that the pgagroal-cli and friends send the application version and pgagroal answer with its version number (see work in #392 ), it is possible to make the management protocol to raise an error if the version numbers between sides are incompatible. Surely incompatibilities must be raised between major versions, while it can also be added for minor versions as well.

fluca1978 commented 6 months ago

Self assigning to myself, if someone else wants to work on this, please advice.

fluca1978 commented 2 months ago

I'm slowly working on this, here are some thought.

If a message comes from an incompatible protocol version, the reply is assumed to be an error. Compatibility means that at max 2 units differ in the minor version, assuming the major one is the same. So for instance 1.5.1 and 1.7.12 are compatible, 2.0.0 and 1.9.99 are not, 1.5.0 and 1.8.0 are not.

In order to distinguish if the error comes from a low level problem (e.g., a network error) or a protocol version mismatch, a new exit code must be introduced. In other words, $? from a command will indicate the case of failure. The JSON output for a command must also specify the cause of the error, i.e., the protocol mismatch, but I'm thinking to add a dedicated field like protocol_mismatch to quickly allow for parsing of a JSON output and get a glance at the main root problem (JSON already has an error boolean field).

When pgagroal receive an incompatible message, it will reply with a special message with an header that spells the incompatibility and will drop the request, so no action is going to be taken on the daemon side. On the other hand, if pgagroal-cli (and friends) receive a message from an incompatible server, an error has to be reported so that the content of the message is not "parsed".

jesperpedersen commented 2 months ago

I don't think we should spend too much time on this, as we know that the next version will be 2.0 which will break everything in I/O layer and management layer as well.

2.0 will use a JSON based message format, where the header will have the client side version, and the response will have the server side version. Then we can look at what we can do for each message type.

Having a simple JSON format as the core of the management layer will allow us to make changes quicker even though there is a larger overhead compared to the existing binary protocol

fluca1978 commented 2 months ago

Ok, I'll postpone this work to the new JSON infrastructure.