Re: Make PQgetResult() not return NULL on out-of-memory error

From: Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Make PQgetResult() not return NULL on out-of-memory error
Date: 2025-11-12 07:20:13
Message-ID: 20251112162013.104098c112a485e8d8bd6ae1@sraoss.co.jp
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 11 Nov 2025 20:16:11 +0900
Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:

> On Tue, Nov 11, 2025 at 2:43 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
> > > To address this, callers need a way to distinguish between PGRES_FATAL_ERROR
> > > and OOM. Functions that loop until PQgetResult() returns NULL should continue
> > > if the result is PGRES_FATAL_ERROR, but should break if it's an OOM.
> >
> > Not sure about that. We might or might not be able to make progress
> > if the caller keeps calling PQgetResult. But if it stops after an
> > OOM report then we might as well just close the connection, because
> > there is no hope of getting back in sync.
> >
> > I'm inclined to think that it's correct that we should return
> > OOM_result not NULL if we couldn't make a PGresult, but further
> > analysis is needed to make sure that libpq can make progress
> > afterwards. I don't think we should expect applications to
> > involve themselves in that recovery logic, because they won't,
> > and most certainly won't test it carefully.
> >
> > The whole project might be doomed to failure really. I think
> > that one very likely failure if we are approaching OOM is that
> > we can't enlarge libpq's input buffer enough to accept all of
> > a (long) incoming server message. What hope have we got of
> > getting out of that situation?
>
> When pqCheckInBufferSpace() fails to enlarge the input buffer and PQgetResult()
> returns PGRES_FATAL_ERROR, I noticed that PQgetResult() sets asyncStatus to
> PGASYNC_IDLE. This means subsequent calls to PQgetResult() will return NULL
> even in OOM case, so functions looping until NULL won't end up in an
> infinite loop.
> Of course, issuing another query on the same connection afterward could be
> problematic, though.
>
> I'm still not sure this behavior is correct, but I'm just wondering if
> asyncStatus should be reset to PGASYNC_IDLE also when OOM_result is returned.

I agree that PQgetResult() should return NULL after returning OOM_result, and
resetting asyncStatus to PGASYNC_IDLE would work in the normal mode.

I also agree that continuing to use the same connection seems problematic,
especially in pipeline mode; the pipeline status remains PQ_PIPELINE_OK, PQgetResult()
never returns PGRES_PIPELINE_SYNC, and the sent commands are left in the command queue.

Therefore, I wonder about closing the connection and resetting the status
when OOM_result is retunred, by callling pqDropConnection() as handleFatalError() does.
In this case, the connection status should also be set to CONNECTINO_BAD.

This would prevent applications from continueing to use potentially problamatic
connection without providing the way to distinguish OOM from other errors.

What do you think?

I've attached an updated patch in this direction.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>

Attachment Content-Type Size
v2-0001-Make-PQgetResult-not-return-NULL-on-out-of-memory.patch text/x-diff 2.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Kukushkin 2025-11-12 07:28:15 Re: Issue with logical replication slot during switchover
Previous Message Xuneng Zhou 2025-11-12 07:19:53 Re: Implement waiting for wal lsn replay: reloaded