From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Cary Huang <cary(dot)huang(at)highgo(dot)ca>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Avoid memory leaks during base backups |
Date: | 2022-10-19 15:53:51 |
Message-ID: | CALj2ACXK85qiL92=iexHyHiWx96dTBuQRutJxkqZeaGP7uWypw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Oct 19, 2022 at 8:10 PM Robert Haas <robertmhaas(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2022-10-19 16:23:28 | Re: Mingw task for Cirrus CI |
Previous Message | Ants Aasma | 2022-10-19 15:50:09 | Standby recovers records from wrong timeline |