Thread: handling out of memory conditions when fetching row descriptions

Hi there,

using the latest git source code, I found that libpq will let the
connection stall when getRowDescriptions breaks on an out of memory
condition. I think this should better be handled differently to allow
application code to handle such situations gracefully.

For now, I changed libpq to close the connection when this happens
(please see the attached patch). This approach works fine for me so
far.

Please inform me if there are better ways to handle this.

Thanks.

Best regards,

Isidor Zeuner

"'Isidor Zeuner'" <postgresql@quidecco.de> writes:
> using the latest git source code, I found that libpq will let the
> connection stall when getRowDescriptions breaks on an out of memory
> condition. I think this should better be handled differently to allow
> application code to handle such situations gracefully.

The basic assumption in there is that if we wait and retry, eventually
there will be enough memory.  I agree that that's not ideal, since the
application may not be releasing memory elsewhere.  But what you propose
doesn't seem like an improvement: you're converting a maybe-failure into
a guaranteed-failure, and one that's much more difficult to recover from
than an ordinary query error.

Also, this patch breaks async operation, in which a failure return from
getRowDescriptions normally means that we have to wait for more data
to arrive.  The test would really need to be inserted someplace else.

In any case, getRowDescriptions is really an improbable place for an
out-of-memory to occur: it would be much more likely to happen while
absorbing the body of a large query result.  There already is some logic
in getAnotherTuple for dealing with that case, which I suggest is a
better model for what to do than "break the connection".  But probably
making things noticeably better here would require going through all
the code to check for other out-of-memory cases, and developing some
more uniform method of representing an already-known-failed query
result.  (For instance, it looks like getAnotherTuple might not work
very well if it fails to get memory for one tuple and then succeeds
on later ones.  We probably ought to have some explicit state that
says "we are absorbing the remaining data traffic for a query result
that we already ran out of memory for".)

            regards, tom lane

> "'Isidor Zeuner'" <postgresql@quidecco.de> writes:
> > using the latest git source code, I found that libpq will let the
> > connection stall when getRowDescriptions breaks on an out of memory
> > condition. I think this should better be handled differently to allow
> > application code to handle such situations gracefully.
>
> The basic assumption in there is that if we wait and retry, eventually
> there will be enough memory.

I think the greatest problem with that approach is that there is no
(at least no documented) way for the application to find out that it
should be releasing memory.

>  I agree that that's not ideal, since the
> application may not be releasing memory elsewhere.  But what you propose
> doesn't seem like an improvement: you're converting a maybe-failure into
> a guaranteed-failure, and one that's much more difficult to recover from
> than an ordinary query error.
>

I think it was an improvement considering that it puts the application
code back in control. Given that the application has some way to
handle connection and query errors, it can do something reasonable
about the situation. Before, the application had no way to find out
there is a (maybe-)failure at all.

> Also, this patch breaks async operation, in which a failure return from
> getRowDescriptions normally means that we have to wait for more data
> to arrive.  The test would really need to be inserted someplace else.
>
> In any case, getRowDescriptions is really an improbable place for an
> out-of-memory to occur: it would be much more likely to happen while
> absorbing the body of a large query result.

My assumption was that there is not much logic to handle such
situations because it is improbable.

I am currently using PostGreSQL under memory-constrained conditions,
so I might be getting back with more such cases if they surface.

>  There already is some logic
> in getAnotherTuple for dealing with that case, which I suggest is a
> better model for what to do than "break the connection".  But probably
> making things noticeably better here would require going through all
> the code to check for other out-of-memory cases, and developing some
> more uniform method of representing an already-known-failed query
> result.  (For instance, it looks like getAnotherTuple might not work
> very well if it fails to get memory for one tuple and then succeeds
> on later ones.  We probably ought to have some explicit state that
> says "we are absorbing the remaining data traffic for a query result
> that we already ran out of memory for".)
>

I like this approach. I changed the out-of-memory handling to switch
to a PGASYNC_MEMORY_FULL state, which will skip all messages until the
command is complete. Patch is attached.

Best regards,

Isidor Zeuner