Subject: Re: protocol-level wait-for-LSN

From: Peter Eisentraut <[email-masked]>
Date: Tue, 29 Oct 2024 07:28:51 +0100

On 29.10.24 06:06, Tatsuo Ishii wrote:
>> The patch adds a protocol extension called _pq_.wait_for_lsn as well
>> as a libpq connection option wait_for_lsn to activate the same.  (Use
>> e.g., psql -d 'wait_for_lsn=1'.)
>>
>> With this protocol extension, two things are changed:
>>
>> - The ReadyForQuery message sends back the current LSN.
> 
> If other protocol extension X tries to add something to the
> ReadyForQuery message too, what would happen?

I think one would have to define that somehow.  If it's useful, the 
additional fields of both extensions could be appended, in some defined 
order.  But this is an interesting question to think about.

---

From: Tatsuo Ishii <[email-masked]>
Date: Wed, 30 Oct 2024 15:49:19 +0900 (JST)

> additional fields of both extensions could be appended, in some
> defined order.  But this is an interesting question to think about.

I think this kind of extension, which adds new filed to an existing
message type, should be implemented as v4 protocol.

Best reagards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

---

From: Jelte Fennema-Nio <[email-masked]>
Date: Wed, 30 Oct 2024 09:41:03 +0100

> message type, should be implemented as v4 protocol.

Could you explain why you think a major version bump is needed? In
what situation do you care about this. Because for my usecases (client
implementations & pgbouncer) I don't think that would be necessary. If
a client doesn't send the _pq_.wait_for_lsn protocol parameter, it
will never receive this new version.

I don't really see a problem with having two protocol parameters
change the same message. Yes, you have to define what the result of
their combination is, but that seems trivial to do for additions of
fields. You either define the first protocol parameter that was added
to the spec, to add its field before the second. Or you could do it
based on something non-time-dependent, like the alphabetic order of
the protocol parameter, or the alphabetic order of the fields that
they add.

The main guarantees I'd like to uphold are listed here:
https://www.postgresql.org/message-id/[email-masked]

---

From: Tatsuo Ishii <[email-masked]>
Date: Wed, 30 Oct 2024 20:34:55 +0900 (JST)

> will never receive this new version.

Yes, if there's only one extension for a message type, it would not be
a big problem. But if there's more than one extensions that want to
change the same type, problem arises as I have already discussed them
upthread.

> the protocol parameter, or the alphabetic order of the fields that

That sounds far from trivial. So each extension needs to check if any
other extension which modifies the same message type is activated?
That requires each extension implementation to have built-in knowledge
about any conflicting extension. Moreover each extension may not be
added at once.  If extension Y is added after extension X is defined,
then implementation of X needs to be changed because at the time when
X is defined, it did not need to care about Y. Another way to deal
with the problem could be defining a new protocol message which
describes those conflict information so that each extensions do not
need to have such information built-in, but maybe it is too complex.

Best reagards,

---

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 30 Oct 2024 13:53:27 +0200

> need to have such information built-in, but maybe it is too complex.

Note that the "protocol extension" mechanism is *not* meant for 
user-defined extensions. That's not the primary purpose anyway. It 
allows evolving the protocol in core code in a backwards compatible way, 
but indeed the different extensions will need to be coordinated so that 
they don't clash with each other. If they introduced new message types 
for example, they better use different message type codes.

We might have made a mistake by calling this mechanism "protocol 
extensions", because it makes people think of user-defined extensions.

With user-defined extensions, yes, you have exactly the problem you 
describe.We have no rules on how a protocol extension is allowed to 
change the protocol. It might add fields, it might add messages, or it 
might change the meaning of existing messages. Or encapsulate the whole 
protocol in XML.

So yes, each protocol extension needs to know about all the other 
protocol extensions that it can be used with. In practice we'll avoid 
doing crazy stuff so that the protocol extensions are orthogonal, but if 
user-defined extensions get involved, there's not much we can do to 
ensure that.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

---

From: Jelte Fennema-Nio <[email-masked]>
Date: Wed, 30 Oct 2024 15:01:27 +0100

> extensions", because it makes people think of user-defined extensions.

I think this is a real problem, that's probably worth fixing. I
created a separate thread to address this[1]

> doing crazy stuff so that the protocol extensions are orthogonal

Just as an example, let's say we add a server-side query time to the
protocol (which honestly seems like a pretty useful feature). So that
ReadyForQuery now returns the query time if the query_time protocol.
For clients it isn't difficult at all to support any combination of
query_time & wait_for_lsn options. As long as we define that the
wait_for_lsn field is before the query_time field if both exist, then
two simple if statements like this would do the trick:

if (wait_for_lsn_enabled) {
    // interpret next field as LSN
}
if (query_time_enabled) {
    // interpret next field as query time
}

[1]: https://www.postgresql.org/message-id/CAGECzQQoc%2BV94TrF-5cMikCMaf-uUnU52euwSCtQBeDYqXnXyA%40mail.gmail.com

---

From: Tatsuo Ishii <[email-masked]>
Date: Thu, 31 Oct 2024 07:44:55 +0900 (JST)

>     // interpret next field as query time

But

doesn't work, right? I don't like clients need to know the exact order
of each protocol extensions.

BTW,

> ReadyForQuery now returns the query time if the query_time protocol.

Probaby it's better CommandComplete returns the query time because
there could be multiple query-time in multi-statement query or
extended query protocol.

Best reagards,