On 10/31/17 15:38, Peter Eisentraut wrote:
> 2) SPI needs some work. It thinks that it can clean everything away at
> transaction end. I have found that instead of TopTransactionContext one
> can use PortalContext and get a more suitable life cycle for the memory.
> I have played with some variants to make this configurable (e.g.,
> argument to SPI_connect()), but that didn't seem very useful. There are
> some comments indicating that there might not always be a PortalContext,
> but the existing tests don't seem to mind. (There was a thread recently
> about making a fake PortalContext for autovacuum, so maybe the current
> idea is that we make sure there always is a PortalContext.) Maybe we
> need another context like StatementContext or ProcedureContext.
This could use more specific discussion, as it is a critical point.
One general theme in this patch is to use PortalContext instead of
TopTransactionContext (or similar). In SPI_connect(), we have
/*
* Create memory contexts for this procedure
*
* XXX it would be better to use PortalContext as the parent
context, but
* we may not be inside a portal (consider deferred-trigger execution).
* Perhaps CurTransactionContext would do? For now it doesn't matter
* because we clean up explicitly in AtEOSubXact_SPI().
*/
_SPI_current->procCxt = AllocSetContextCreate(TopTransactionContext,
"SPI Proc",
and my patch changes that to PortalContext in defiance of that comment.
So either the comment is incorrect or we have insufficient test coverage
or something in between.
ISTM that in the normal case, at the time deferred triggers are
executed, we are in the portal that executes the COMMIT command, so that
should work. There are some cases that call CommitTransactionCommand()
internally, but they don't run in cases when triggers are pending (e.g.,
VACUUM). Although logical apply workers might be a problem, but they
clearly postdate that comment.
In any case, the precedent in autovacuum appears to be to make a fake
PortalContext if needed, so we could do that. Can we think of other
cases where that might be necessary, so I can construct some test cases?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
