On Wed, Jun 18, 2014 at 6:25 PM, Abhijit Menon-Sen <ams@2ndquadrant.com> wrote:
At 2014-06-18 18:10:34 +0530, rahilasyed90@gmail.com wrote: > > palloc() is disallowed in critical sections and we are already in CS > while executing this code. So we use malloc().
Are these allocations actually inside a critical section? It seems to me that the critical section starts further down, but perhaps I am missing something.
ISTM XLogInsert() itself is called from other critical sections. See heapam.c for example.
Second, as Andres says, you shouldn't malloc() inside a critical section either; and anyway, certainly not without checking the return value.
I was actually surprised to see Andreas comment. But he is right. OOM inside CS will result in a PANIC. I wonder if we can or if we really do enforce that though. The code within #ifdef WAL_DEBUG in the same function is surely doing a palloc(). That will be caught since there is an assert inside palloc(). May be nobody tried building with WAL_DEBUG since that assert was added.
May be Rahila can move that code to InitXLogAccess or even better check for malloc() return value and proceed without compression. There is code in snappy.c which will need similar handling, if we decide to finally add that to core.
> I am not sure if the change will be a significant improvement from > performance point of view except it will save few condition checks.
Moving that allocation out of the outer for loop it's currently in is *nothing* to do with performance, but about making the code easier to read.