On Sat, Nov 17, 2012 at 03:20:20PM +0100, Andres Freund wrote:
> On 2012-11-16 22:31:51 -0500, Noah Misch wrote:
> > On Fri, Nov 16, 2012 at 05:31:12PM +0100, Andres Freund wrote:
> > > On 2012-11-16 13:17:47 -0300, Alvaro Herrera wrote:
> > > > Andres is on the verge of convincing me that we need to support
> > > > singleton FOR SHARE without multixacts due to performance concerns.
> > >
> > > I don't really see any arguments against doing so. We aren't in a that
> > > big shortage of flags and if we need more than available I think we can
> > > free some (e.g. XMAX/XMIN_INVALID).
> >
> > The patch currently leaves two unallocated bits. Reclaiming currently-used
> > bits means a binary compatibility break. Until we plan out such a thing,
> > reclaimable bits are not as handy as never-allocated bits. I don't think we
> > should lightly expend one of the final two.
>
> Not sure what you mean with a binary compatibility break?
> pg_upgrade'ability?
Yes. If we decide HEAP_XMIN_INVALID isn't helping, we can stop adding it to
tuples anytime. Old tuples may continue to carry the bit, with no particular
deadline for their demise. To reuse that bit in the mean time, we'll need to
prove that no tuple writable by PostgreSQL 8.3+ will get an unacceptable
interpretation under the new meaning of the bit. Alternately, build the
mechanism to prove that all such old bits are gone before using the bit in the
new way. This keeps HEAP_MOVED_IN and HEAP_MOVED_OFF unavailable today.
> What I previously suggested somewhere was:
>
> #define HEAP_XMAX_SHR_LOCK 0x0010
> #define HEAP_XMAX_EXCL_LOCK 0x0040
> #define HEAP_XMAX_KEYSHR_LOCK (HEAP_XMAX_SHR_LOCK|HEAP_XMAX_EXCL_LOCK)
> /*
> * Different from _LOCK_BITS because it doesn't include LOCK_ONLY
> */
> #define HEAP_LOCK_MASK (HEAP_XMAX_SHR_LOCK|HEAP_XMAX_EXCL_LOCK)
>
> #define HEAP_XMAX_IS_SHR_LOCKED(tup) \
> (((tup)->t_infomask & HEAP_LOCK_BITS) == HEAP_XMAX_SHR_LOCK)
> #define HEAP_XMAX_IS_EXCL_LOCKED(tup) \
> (((tup)->t_infomask & HEAP_LOCK_BITS) == HEAP_XMAX_EXCL_LOCK)
> #define HEAP_XMAX_IS_KEYSHR_LOCKED(tup) \
> (((tup)->t_infomask & HEAP_LOCK_BITS) == HEAP_XMAX_KEYSHR_LOCK)
>
> It makes checking for locks sightly more more complicated, but its not
> too bad...
Agreed; that seems reasonable.
