Re: Allocations in critical section (was Re: [HACKERS] WAL format and API changes (9.5))

2014-04-04 Thread Tom Lane
Heikki Linnakangas hlinnakan...@vmware.com writes:
 Ok, I fixed the issues that the assertion fixed. I also committed a 
 patch to add the assertion itself; let's see if the buildfarm finds more 
 cases that violate the rule.

 It ignores the checkpointer, because it's known to violate the rule,

... uh, isn't that a bug to be fixed?

 and 
 allocations in ErrorContext, which is used during error recovery, e.g if 
 you indeed PANIC while in a critical section for some other reason.

Yeah, I realized we'd have to do something about elog's own allocations.
Not sure if a blanket exemption for ErrorContext is the best way.  I'd
been thinking of having a way to turn off the complaint once processing
of an elog(PANIC) has started.

 I didn't backpatch this.

Agreed.

BTW, I'm pretty sure you added some redundant assertions in mcxt.c.
eg, palloc does not need its own copy.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: Allocations in critical section (was Re: [HACKERS] WAL format and API changes (9.5))

2014-04-04 Thread Heikki Linnakangas

On 04/04/2014 04:56 PM, Tom Lane wrote:

Heikki Linnakangas hlinnakan...@vmware.com writes:

Ok, I fixed the issues that the assertion fixed. I also committed a
patch to add the assertion itself; let's see if the buildfarm finds more
cases that violate the rule.



It ignores the checkpointer, because it's known to violate the rule,


... uh, isn't that a bug to be fixed?


Yes. One step a time ;-).


and
allocations in ErrorContext, which is used during error recovery, e.g if
you indeed PANIC while in a critical section for some other reason.


Yeah, I realized we'd have to do something about elog's own allocations.
Not sure if a blanket exemption for ErrorContext is the best way.  I'd
been thinking of having a way to turn off the complaint once processing
of an elog(PANIC) has started.


Hmm. PANIC processing should avoid allocations too, except in 
ErrorContext, because otherwise you might get an out-of-memory during 
PANIC processing.


ErrorContext also covers elog(DEBUG2, ...). I presume we'll want to 
ignore that too. Although I also tried without the exemption for 
ErrorContext at first, and didn't get any failures from the regression 
tests, so I guess we never do that in a critical section. I was a bit 
surprised by that.




BTW, I'm pretty sure you added some redundant assertions in mcxt.c.
eg, palloc does not need its own copy.


palloc is copy-pasted from MemoryContextAlloc - it does need its own copy.

- Heikki


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers