On Fri, 30 Nov 2018 at 23:08, Michael Paquier <mich...@paquier.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
