Re: Out-of-memory error reports in libpq

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Peter Geoghegan <pg(at)bowt(dot)ie>
Subject: Re: Out-of-memory error reports in libpq
Date: 2021-07-28 11:20:43
Message-ID: a19a180e-5012-053d-8d4d-4d46db0f600a@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 7/27/21 6:40 PM, Tom Lane wrote:
> While cleaning out dead branches in my git repo, I came across an
> early draft of what eventually became commit ffa2e4670 ("In libpq,
> always append new error messages to conn->errorMessage"). I realized
> that it contained a good idea that had gotten lost on the way to that
> commit. Namely, let's reduce all of the 60-or-so "out of memory"
> reports in libpq to calls to a common subroutine, and then let's teach
> the common subroutine a recovery strategy for the not-unlikely
> possibility that it fails to append the "out of memory" string to
> conn->errorMessage. That recovery strategy of course is to reset the
> errorMessage buffer to empty, hopefully regaining some space. We lose
> whatever we'd had in the buffer before, but we have a better chance of
> the "out of memory" message making its way to the user.
>
> The first half of that just saves a few hundred bytes of repetitive
> coding. However, I think that the addition of recovery logic is
> important for robustness, because as things stand libpq may be
> worse off than before for OOM handling. Before ffa2e4670, almost
> all of these call sites did printfPQExpBuffer(..., "out of memory").
> That would automatically clear the message buffer to empty, and
> thereby be sure to report the out-of-memory failure if at all
> possible. Now we might fail to report the thing that the user
> really needs to know to make sense of what happened.
>
> Therefore, I feel like this was an oversight in ffa2e4670,
> and we ought to back-patch the attached into v14.
>
> cc'ing the RMT in case they wish to object.
>
>

I'm honored you've confused me with Alvaro :-)

This seems sensible, and we certainly shouldn't be worse off than
before, so let's do it.

I'm fine with having two functions for call simplicity, but I don't feel
strongly about it.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2021-07-28 11:26:23 Re: perlcritic: prohibit map and grep in void conext
Previous Message Andrew Dunstan 2021-07-28 11:10:29 Re: perlcritic: prohibit map and grep in void conext