Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs

From: Jelte Fennema-Nio <me(at)jeltef(dot)nl>
To: Jacob Burroughs <jburroughs(at)instructure(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Dave Cramer <davecramer(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Andrey M(dot) Borodin" <x4mmm(at)yandex-team(dot)ru>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>
Subject: Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs
Date: 2023-12-30 17:57:03
Message-ID: CAGECzQT1p=-REEJLBTgMmGwDL0xL0f-gysAT77mjnS=3KaYo2Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, 30 Dec 2023 at 16:05, Jacob Burroughs
<jburroughs(at)instructure(dot)com> wrote:
> What if we allowed the client to send `ParameterStatus` messages to
> the server to reconfigure protocol extensions that were requested at
> startup?

Sending ParameterStatus from the frontend is not possible, because
Sync already claimed the 'S' message type on the frontend side. (and
even if that weren't the case I think the name ParameterStatus is not
descriptive of what the message would do when you'd send it from the
frontend.

> Such processing would be independent of the transaction
> lifecycle since protocol-level options aren't related to transactions.
> Any errors in the set value would be handled with an `ErrorResponse`
> (including if an extension was not reconfigurable after connection
> startup), and success with a new `ReadyForQuery` message.

If we only consider modifying protocol extension parameters with
ParameterSet, then I think I like the idea of reusing ReadyForQuery
instead of introducing ParamaterSetComplete. I like that it implicitly
makes clear that you should not send ParameterStatus while you already
have an open pipeline for protocol extension parameters. If we go this
approach, we should probably explicitly disallow sending ParameterSet
while a pipeline.

However, if we also allow using ParameterSet to change regular runtime
parameters, then I think this approach makes less sense. Because then
you might want to batch regular runtime parameter together with other
actions in a pipeline, and use the expected "ignore everything after
the first error"

> The actual
> effect of an extension change must be delayed until after the
> ReadyForQuery has been transmitted, though I don't know if that is
> feasible or if we would need to instead implicitly assume changes were
> successful and just close the connection on error.

I'm trying to think about how a client would want to handle a failure
when changing a protocol extension. Is there really an action it can
reasonably take at that point? I guess it might depend on the exact
extension, but I do think that in many cases closing the connection is
the only reasonable response. Maybe the server should even close the
connection with a FATAL error when it receives a ParameterSet for a
protocol extension but it fails to apply it.

> We wouldn't need
> to bump the protocol version since a client wouldn't (shouldn't) send
> these messages unless it successfully negotiated the relevant protocol
> extension(s) at startup.

I think I'd still prefer to bump the minor version, even if it's just
so that we've done it for the first time and we fixed all the libpq
issues with it. But I also think it makes sense from a versioning
perspective, if there's new message types that can be sent by the
client, which do not correspond to a protocol extension, then I think
the only reasonable thing is to update the version number. Otherwise
you'd basically need to define the ParameterSet message to be a part
of every new protocol extension that you would add. That seems more
confusing than saying that version 3.1 supports this new ParameterSet
message type.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2023-12-30 19:39:47 Re: pg_stat_statements: more test coverage
Previous Message Joe Conway 2023-12-30 17:50:08 Re: SET ROLE x NO RESET