On Sat, May 4, 2019 at 1:34 PM Jeff Janes <jeff.ja...@gmail.com> wrote:
> On Thu, Mar 28, 2019 at 1:30 AM Thomas Munro <thomas.mu...@gmail.com> > wrote: > >> On Thu, Mar 28, 2019 at 1:48 AM Heikki Linnakangas <hlinn...@iki.fi> >> wrote: >> > Once we have the FullTransactionId type and basic macros in place, I'm >> > sure we could tidy up a bunch of code by using them. > > > Thanks for the reviews! Pushed. >> > > I think that this might be broken. > > We have this change: > > @@ -73,7 +75,8 @@ GetNewTransactionId(bool isSubXact) > > LWLockAcquire(XidGenLock, LW_EXCLUSIVE); > > - xid = XidFromFullTransactionId(ShmemVariableCache->nextFullXid); > + full_xid = ShmemVariableCache->nextFullXid; > + xid = XidFromFullTransactionId(full_xid); > > But then later on in an little-used code path around line 164: > > /* Re-acquire lock and start over */ > LWLockAcquire(XidGenLock, LW_EXCLUSIVE); > xid = XidFromFullTransactionId(ShmemVariableCache->nextFullXid); > > full_xid does not get updated, but then later on full_xid gets returned in > lieu of xid. > > Is there a reason that this is OK? > > Ah, sorry for the noise. I see that this has been fixed already. I wasn't looking at HEAD, or at the other email thread, when I "discovered" this. Sorry for the noise. Cheers, Jeff