From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | pgsql-hackers(at)postgreSQL(dot)org |
Subject: | Rethinking autovacuum.c memory handling |
Date: | 2017-09-22 21:09:03 |
Message-ID: | 13849.1506114543@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I notice that autovacuum.c calls autovacuum_do_vac_analyze, and
thereby vacuum(), in TopTransactionContext. This doesn't seem
like a terribly great idea, because it doesn't correspond to what
happens during a manually-invoked vacuum. TopTransactionContext
will go away when vacuum() commits the outer transaction, whereas
in non-autovac usage, we call vacuum() in a PortalHeapMemory
context that is not a child of TopTransactionContext and is not
at risk of being reset multiple times during the vacuum(). This'd
be a hazard if autovacuum_do_vac_analyze or vacuum did any palloc's
before getting to the main loop. More generally, I'm not aware of
other cases where we invoke a function in a context that we know
that function will destroy as it executes.
I don't see any live bug associated with this in HEAD, but this behavior
requires a rather ugly (and memory-leaking) workaround in the proposed
patch to allow multiple vacuum target rels.
What I think we should do instead is invoke autovacuum_do_vac_analyze
in the PortalContext that do_autovacuum has created, which we already
have a mechanism to reset once per table processed in do_autovacuum.
The attached patch does that, and also modifies perform_work_item()
to use the same approach. Right now perform_work_item() has a
copied-and-pasted MemoryContextResetAndDeleteChildren(PortalContext)
call in its error recovery path, but that seems a bit out of place
given that perform_work_item() isn't using PortalContext otherwise.
Comments, objections?
regards, tom lane
PS: I was disappointed to find out that perform_work_item() isn't
exercised at all in the standard regression tests.
Attachment | Content-Type | Size |
---|---|---|
use-portalcontext-in-autovacuum.patch | text/x-diff | 2.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2017-09-22 21:09:09 | Re: [Proposal] Make the optimiser aware of partitions ordering |
Previous Message | Peter Eisentraut | 2017-09-22 20:48:23 | Re: Rewriting the test of pg_upgrade as a TAP test |