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(at)sss(dot)pgh(dot)pa(dot)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