On Wed, May 22, 2019 at 5:47 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, May 22, 2019 at 7:17 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > +/* Extract xid from a value comprised of epoch and xid */
> > > > +#define GetXidFromEpochXid(epochxid) \
> > > > + ((uint32) (epochxid) & 0XFFFFFFFF)
> > > > +
> > > > +/* Extract epoch from a value comprised of epoch and xid */
> > > > +#define GetEpochFromEpochXid(epochxid) \
> > > > + ((uint32) ((epochxid) >> 32))
> > > > +
> > >
> > > Why do these exist?
> > >
> >
> > We don't need the second one (GetEpochFromEpochXid), but the first one
> > is required. Basically, the oldestXidHavingUndo computation does
> > consider oldestXmin (which is still a TransactionId) as we can't
> > retain undo which is 2^31 transactions old due to other limitations
> > like clog/snapshots still has a limit of 4-byte transaction ids.
> > Slightly unrelated, but we do want to improve the undo retention in a
> > subsequent version such that we won't allow pending undo for
> > transaction whose age is more than 2^31.
>
> The point is that we now have EpochFromFullTransactionId and
> XidFromFullTransactionId. You shouldn't be inventing your own version
> of that infrastructure. Use FullTransactionId, not a uint64, and then
> use the functions for dealing with full transaction IDs from
> transam.h.
>
Okay, I misunderstood the comment. I'll change accordingly. Thanks
for pointing out.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
