Re: Avoid memory leaks during base backups - Mailing list pgsql-hackers

From Bharath Rupireddy
Subject Re: Avoid memory leaks during base backups
Date
Msg-id CALj2ACXK85qiL92=iexHyHiWx96dTBuQRutJxkqZeaGP7uWypw@mail.gmail.com
Whole thread Raw
In response to Re: Avoid memory leaks during base backups  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: Avoid memory leaks during base backups  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
List pgsql-hackers
On Wed, Oct 19, 2022 at 8:10 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> Well this still touches postgres.c. And I still think it's an awful
> lot of machinery for a pretty trivial problem. As a practical matter,
> nobody is going to open a connection and sit there and try to start a
> backup over and over again on the same connection. And even if someone
> wrote a client that did that -- why? -- they'd have to be awfully
> persistent to leak any amount of memory that would actually matter. So
> it is not insane to just think of ignoring this problem entirely.

I understand that the amount of memory allocated by pg_backup_start()
is small compared to the overall RAM, however, I don't think we can
ignore the problem and let postgres cause memory leaks.

> But if we want to fix it, I think we should do it in some more
> localized way.

Agreed.

> One option is to just have do_pg_start_backup() blow
> away any old memory context before it allocates any new memory, and
> forget about releasing anything in PostgresMain(). That means memory
> could remain allocated after a failure until you next retry the
> operation, but I don't think that really matters. It's not a lot of
> memory; we just don't want it to accumulate across many repetitions.

This seems reasonable to me.

> Another option, perhaps, is to delete some memory context from within
> the TRY/CATCH block if non-NULL, although that wouldn't work nicely if
> it might blow away the data we need to generate the error message.

Right.

> A third option is to do something useful inside WalSndErrorCleanup() or
> WalSndResourceCleanup() as I suggested previously.

These functions will not be called for SQL-callable backup functions
pg_backup_start() and pg_backup_stop(). And the memory leak problem
we're trying to solve is for SQL-callable functions, but not for
basebaskups as they already have a memory context named "Replication
command context" that gets deleted in PostgresMain().

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com



pgsql-hackers by date:

Previous
From: Ants Aasma
Date:
Subject: Standby recovers records from wrong timeline
Next
From: Andres Freund
Date:
Subject: Re: Mingw task for Cirrus CI