Re: Should XLogInsert() be done only inside a critical section?

Michael Paquier wrote:
> On Thu, Apr 21, 2016 at 5:44 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Anyway, I went through our tree and added START/END_CRIT_SECTION calls
> > around all XLogInsert calls that could currently be reached without one;
> > see attached.  Since this potentially breaks third-party code I would
> > not propose back-patching it, but I think it's reasonable to propose
> > applying it to HEAD.
> 
> +1 for sanitizing those code paths this way. This patch looks sane to
> me after having a look with some testing.
> 
> --- a/src/backend/access/brin/brin.c
> +++ b/src/backend/access/brin/brin.c
> @@ -610,15 +610,12 @@ brinbuild(Relation heap, Relation index,
> IndexInfo *indexInfo)
>         elog(ERROR, "index \"%s\" already contains data",
>              RelationGetRelationName(index));
> 
> -   /*
> -    * Critical section not required, because on error the creation of the
> -    * whole relation will be rolled back.
> -    */
> Perhaps Alvaro has a opinion to offer regarding this bit removed in brin.c?

I vaguely recall copying this comment from elsewhere, but I didn't see
any other such comment being removed by the patch; I probably copied
something else which got slowly mutated into what's there today during
development.

if we're adding the critical section then the comment should
certainly be removed too.  

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services