Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs - Mailing list pgsql-hackers
From | Jelte Fennema-Nio |
---|---|
Subject | Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs |
Date | |
Msg-id | CAGECzQRCR5uJ5iww2sTcDEY72vrHTaZchgiuatqwriXqAyT_kw@mail.gmail.com Whole thread Raw |
In response to | Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs (Robert Haas <robertmhaas@gmail.com>) |
Responses |
Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs
|
List | pgsql-hackers |
On Fri, 5 Apr 2024 at 18:48, Robert Haas <robertmhaas@gmail.com> wrote: > Maybe we'd be better off adding a libpq connection > option that forces the use of a specific minor protocol version, but > then we'll need backward-compatibility code in libpq basically > forever. But maybe we need that anyway to avoid older and newer > servers being unable to communicate. I think this would be good because it makes testing easy and, like you said, I think we'll need this backward-compatibility code in libpq anyway to be able to connect to old servers. To have even better and more realistic test coverage though, I think we might also want to actually test new libpq against old postgres servers and vice-versa in a build farm animal though. > Plus, you've got all of the consequences for non-core drivers, which > have to both add support for the new wire protocol - if they don't > want to seem outdated and eventually obsolete - and also test that > they're still compatible with all supported server versions. I think for clients/drivers, the work would generally be pretty minimal. For almost all proposed changes, clients can "support" the protocol version update by simply not using the new features, e.g. a client can "support" the ParameterSet feature, by simply never sending the ParameterSet message. So binding it to a protocol version bump doesn't make it any harder for that client to support that protocol version. I'm not saying that is the case for all protocol changes, but based on what's being proposed so far that's definitely a very common theme. Overall, I think this is something to discuss for each protocol change in isolation: i.e. how to make supporting the new feature as painless as possible for clients/drivers. > Connection poolers have the same set of problems. For connection poolers this is indeed a bigger hassle, because they at least need to be able to handle all the new message types that a client can send and maybe do something special for them. But I think if we're careful to keep connection poolers in mind when designing the features themselves then I think this isn't necessarily a problem. And probably indeed for the features that we think are hard for connection poolers to implement, we should be using protocol extension parameter feature flags. But I think a lot of protocol would be fairly trivial for a connection pooler to support. > The whole thing is > almost a hole with no bottom. Keeping up with core changes in this > area could become a massive undertaking for lots and lots of people, > some of whom may be the sole maintainer of some important driver that > now needs a whole bunch of work. I agree with Dave here, if you want to benefit from new features there's some expectation to keep up with the changes. But to be clear, we'd still support old protocol versions too. So we wouldn't break connecting using those clients, they simply wouldn't benefit from some of the new features. I think that's acceptable. > I'm not sure how much it improves things if we imagine adding feature > flags to the existing protocol versions, rather than whole new > protocol versions, but at least it cuts down on the assumption that > adopting new features is mandatory, and that such features are > cumulative. If a driver wants to support TDE but not protocol > parameters or protocol parameters but not TDE, who are we to say no? > If in supporting those things we bump the protocol version to 3.2, and > then 3.3 fixes a huge performance problem, are drivers going to be > required to add support for features they don't care about to get the > performance fixes? I think there's an important trade-off here. On one side we don't want to make maintainers of clients/poolers do lots of work to support features they don't care about. And on the other side it seems quite useful to limit the amount of feature combinations that are used it the wild (both for users and for us) e.g. the combinations of backwards compatibility testing you were talking about would explode if every protocol change was a feature flag. I think this trade-off is something we should be deciding on based on the specific protocol change. But if work needed to "support" the feature is "minimal" (to-be-defined exactly what we consider minimal), I think making it part of a protocol version bump is reasonable. > I see some benefit in bumping the protocol version > for major changes, or for changes that we have an important reason to > make mandatory, or to make previously-optional features for which > support has become in practical terms universal part of the base > feature set. But I'm very skeptical of the idea that we should just > handle as many things as possible via a protocol version bump. We've > been avoiding protocol version bumps like the plague since forever, > and swinging all the way to the other extreme doesn't sound like the > right idea to me. I think there's two parts to a protocol version bump: 1. The changes that cause us to consider a protocol bump 2. The actual act of bumping the protocol version I think 1 is a thing we should be careful about every time (especially regarding impact on clients/poolers). But 2 shouldn't be something that we should consider dangerous/scary. I think that every change that we make to the protocol (no matter how minor or backwards compatible it is), should be accompanied with a protocol version bump. This isn't what has happened in the past, and it makes it quite hard to understand what "supporting" a specific protocol version actually means. e.g. PgBouncer currently supports protocol 3.0, but doesn't support the NegotiateProtocolVersion message (I'm working on fixing that). To take it to the extreme: I think we should get to a state, where if we bump the protocol version at the client and server side without actually making any protocol changes, everything should continue to work fine. If we'd do that right now, then libpq wouldn't be able to connect to old postgres versions anymore. > I'm not sure what I think about this. Do we need these new GUCs to be > both PGC_PROTOCOL *and also* live in a separate namespace? I see the > need for the former pretty clearly: if these kinds of things are to be > part of the GUC system (which wasn't my initial bias, but whatever) > then they need to have some important behavioral differences from > other GUCs and so we need a flag to signal that. But what problem are > we solving by also giving them special-looking names, and are we sure > we wouldn't rather solve that problem some other way? Clients might want to allow the user of the client to change regular parameters using ParameterSet (e.g. so that a connection pooler can intercept those ParameterSet messages and change its own behaviour if the parameter name is pgbouncer.pool_mode). But they wouldn't want a user to set any parameters that change the wire-protocol this way. And because an old client might connect to a new server a simple hard-coded list of parameters at the client side is not sufficient. I can see two ways around this: 1. Using a well-known prefix or namespace for parameters that change the wire protocol. (exact prefix to be bikeshedded on) 2. Using a hard-coded list at the client AND disallow changing PGC_PROTOCOL parameters at the server if the negotiated protocol version is lower than the version this parameter was introduced in AND bump the protocol version whenever we add a new PGC_PROTOCOL parameter. I think 1 is easier to implement at the client side, as it only requires a prefix comparison instead of keeping track of a list.
pgsql-hackers by date: