Re: Out-of-memory error reports in libpq

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