pgagroal icon indicating copy to clipboard operation
pgagroal copied to clipboard

Add application version within the management protocol

Open fluca1978 opened this issue 1 year ago • 11 comments

While working on #391 I got the idea that, since pgagroal-cli and pragroal are communicating over a socket thru the management protocol, chances are that the two applications are not at the very same version. In order to better diagnose problems and even receive a better and more constant behavior, we should transmit the version of pgagroal-cli and pgagroal thru the socket, so that both parties have information about who they are talking to. Moreover, this could be a chance to even block request made by older versions of any of the two to the other part.

My idea is to add an header-like block to any request sent out from pgagroal-cli, having pgagroal to get it in the begin of the message (so, at a constant offset) and viceversa, having pgagroal to send out the same information as the first part of the communication protocol.

This should not require a lot of changes in the management protocol implementation. However, I would postpone this to #391 because importing JSON output will speed up the debugging of this implementation.

fluca1978 avatar Dec 09 '23 14:12 fluca1978

:+1: to the idea.

We just have to figure out what we want in that header... could also be stuff like the entire size of the management invocation

jesperpedersen avatar Dec 09 '23 16:12 jesperpedersen

👍 to the idea.

We just have to figure out what we want in that header... could also be stuff like the entire size of the management invocation

Well, since we are going to add stuff, I think there is room for placing a size of the request. At this point, I would also put the client (e.g., pgagroal-cli ) that did the request.

fluca1978 avatar Dec 11 '23 10:12 fluca1978

Yeah, the name of the client plus its version number would be a good idea.

The response should include the version of the pgagroal instance.

An idea could be to be to create a variable length header where each "section" is identified by a unique char - like ...cpgagroal-cli\0v1.6.0\0... with a response of ...Spgagroal\0V1.6.0\0...

jesperpedersen avatar Dec 11 '23 13:12 jesperpedersen

An idea could be to be to create a variable length header where each "section" is identified by a unique char - like ...cpgagroal-cli\0v1.6.0\0... with a response of ...Spgagroal\0V1.6.0\0...

I think that changing write_header https://github.com/agroal/pgagroal/blob/master/src/libpgagroal/management.c#L1488 (and consequently, pgagroal_managament_read_header https://github.com/agroal/pgagroal/blob/master/src/libpgagroal/management.c#L66) to interact with the extended headers. But in my opinion, sections should be identified by a fixed letter, for example F as from (the fact that is a server or a client depends on the side we read the data), V for version, etc. Dealing with headers means that the headaers have to be accessible in some way to the "client" methods, like pgagroal_management_read_something, so I'm still thinking if instead of dealing with headers we should deal with custom payloads.

fluca1978 avatar Dec 18 '23 15:12 fluca1978

Sounds good - a prototype of it will likely give a clearer indication of what is the best solution

jesperpedersen avatar Dec 19 '23 10:12 jesperpedersen

Sounds good - a prototype of it will likely give a clearer indication of what is the best solution

Ok, I think this will be done in the next year, and after #391 has completed.

fluca1978 avatar Dec 19 '23 10:12 fluca1978

:+1:

jesperpedersen avatar Dec 19 '23 10:12 jesperpedersen

is this a possible GSoC 2024 project?

HazemRawi avatar Jan 11 '24 23:01 HazemRawi

is this a possible GSoC 2024 project?

I don't think so: this is a quite short (even if not so trivial) change to the communication protocol. @jesperpedersen could add more on this, and has the very last word (obviously).

fluca1978 avatar Jan 17 '24 11:01 fluca1978

Sounds good - a prototype of it will likely give a clearer indication of what is the best solution

Ok, I think this will be done in the next year, and after #391 has completed.

Meanwhile, I was thinking that either this change to the communication protocol can either broke backward compatibility or not. I think the message should include information about the client/server version (and other details) in the beginning of the message, i.e., before the actual payload. Doing so, however, would break backward compatibility, because a new pgagroal-cli cannot talk anymore with an old pgagroal and viceversa. One possible solution could be to make a fail path, so that if the "new" information is not found, the protocol is assumed to be old and we proceed immediatly to the payload. But I don't like this idea, since it exposes the risk of misinterpreting a piece of payload as "new" information. Probably this poses the question about putting a protocol version number into the messages, so that pgagroal and pgagroal-cli could possibly decide if they can handle the message correctly. But I see backward compatibility breaks in here.

On the other hand, we could add the "new" information at the very end of the message, but this would prevent a possible compatibility check (e.g., pgagroal-cli 1.6 talking to pgagroal 2), so I don't like the idea neither.

fluca1978 avatar Jan 17 '24 12:01 fluca1978

Getting new ideas into the project should not block the progress.

If we decide to change the CLI and management protocol that is enough for a version 2.0 -- it doesn't have to be at a pool level.

And yes, changing the management protocol isn't "large" enough for a GSoC 2024 project, even for a short project.

jesperpedersen avatar Jan 18 '24 23:01 jesperpedersen