Re: pgsql: Avoid duplicate XIDs at recovery when building initial snapshot

On Fri, 30 Nov 2018 at 23:08, Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> On Fri, Nov 30, 2018 at 02:55:47PM +0000, Simon Riggs wrote:
> > 1df21ddb looks OK to me and was simple enough to backpatch safely.
>
> Thanks for the feedback!
>
> > Seems excessive to say that the WAL record is corrupt, it just contains
> > duplicates, just as exported snapshots do. There's a few other imprecise
> > things around in WAL, that is why we need the RunningXact data in the
> first
> > place. So we have a choice of whether to remove the duplicates eagerly or
> > lazily.
> >
> > For GetRunningTransactionData(), we can do eager or lazy, since its not a
> > foreground process. I don't object to changing it to be eager in this
> path,
> > but this patch is more complex than 1df21ddb and I don't think we should
> > backpatch this change, assuming it is acceptable.
>
> Yes, I would avoid a backpatch for this more complicated one, and
> we need a solution for already-generated WAL.

Yes, that is an important reason not to backpatch.

> It is not complicated to
> handle duplicates for xacts and subxacts however holding ProcArrayLock
> for a longer time stresses me as it is already a bottleneck.
>

I hadn't realised this patch holds ProcArrayLock while removing duplicates.
Now I know I vote against applying this patch unless someone can show that
the performance effects of doing so are negligable, which I doubt.

--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services