On Thu, Oct 18, 2012 at 04:58:20PM -0300, Alvaro Herrera wrote:
> Here is version 22 of this patch. This version contains fixes to issues
> reported by Andres, as well as a rebase to latest master.
I scanned this for obvious signs of work left to do. It contains numerous XXX
and FIXME comments. Many highlight micro-optimization opportunities and the
like; those can stay. Others preclude commit, either highlighting an unsolved
problem or wrongly highlighting a non-problem:
> + /*
> + * XXX we do not lock this tuple here; the theory is that it's sufficient
> + * with the buffer lock we're about to grab. Any other code must be able
> + * to cope with tuple lock specifics changing while they don't hold buffer
> + * lock anyway.
> + */
> * so they will be uninteresting by the time our next transaction starts.
> * (XXX not clear that this is correct --- other members of the MultiXact
> * could hang around longer than we did. However, it's not clear what a
> ! * better policy for flushing old cache entries would be.) FIXME actually
> ! * this is plain wrong now that multixact's may contain update Xids.
> ! nmembers = GetMultiXactIdMembers(multi, &members, true);
> ! /*
> ! * XXX we don't have the infomask to run the required consistency check
> ! * here; the required notational overhead seems excessive.
> ! */
> /* We assume the cache entries are sorted */
> ! /* XXX we assume the unused bits in "status" are zeroed */
> ! if (memcmp(members, entry->members, nmembers * sizeof(MultiXactMember)) == 0)
> ! * XXX do we have any issues with needing to checkpoint here?
> */
> ! static void
> ! TruncateMultiXact(void)
> {
> + /* FIXME what should we initialize this to? */
> + newFrozenMulti = ReadNextMultiXactId();
> + * FIXME -- the XMAX_IS_MULTI test is a bit wrong .. it's possible to
> + * have tuples with that bit set that are dead. However, if that's
> + * changed, the RawXmax() call below should probably be researched as well.
> */
> if (tuple->t_infomask &
> ! (HEAP_XMAX_INVALID | HEAP_XMAX_LOCK_ONLY | HEAP_XMAX_IS_MULTI))
> return false;
