Re: BackendKeyData is mandatory?

On 11/08/2025 09:58, Jelte Fennema-Nio wrote:
> On Fri, 8 Aug 2025 at 17:50, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> I think any of the other options would be
>> better. There's no guarantee that more data will ever arrive, the
>> connection might be used just to wait for the notification.
> 
> Yeah, I think that's the important thing to realize here. The "try
> again later" only makes sense if we need more data to try again. If we
> don't then we now start waiting on data that might never come.

Here's a new set of patches, to disconnect on OOM instead of hanging or 
silently discarding messages:

v4-0001-libpq-Don-t-hang-on-out-of-memory.patch

This is a very minimal fix for the "hang on OOM" issue. It changes the 
behavior to silently skip over the message, i.e. discard the async 
notification, or continue without cancellation key.

I think we should backpatch this.

v4-0002-libpq-Handle-OOMs-by-disconnecting-instead-of-sil.patch

This changes the behavior of all of the problematic places where we 
silently discarded things on OOM to disconnect instead. That is, when 
processing a Notify, BackendKeyData, or ParameterStatus message.

Now that I look at this again, we should probably forget about patch #1 
and commit and backpatch this straight away. It's a little more changes 
than patch #1, but not that much.

Note that there are still places where we discard things on OOM, like 
when parsing an error 'E' message. Those are a little iffy too, but 
they're less problematic because you still get an error, and the error 
is clearly associated with a query, unlike these Notify, BackendData and 
ParameterStatus messages.

v4-0003-libpq-Be-strict-about-cancel-key-lengths.patch

Your patch to add more checks, rebased over the first two.

- Heikki