Re: SSI patch version 12

> Heikki Linnakangas  wrote:
> Setting the high bit in OldSetXidAdd() seems a bit weird. How about
> just using UINT64_MAX instead of 0 to mean no conflicts? Or 1, and
> start the sequence counter from 2.
Sure.  I think I like reserving 1 and starting at 2 better.  Will do.
> ReleasePredicateLocks() reads ShmemVariableCache->nextXid without
> XidGenLock. Maybe it's safe, we assume that TransactionIds are
> atomic elsewhere, but at least there needs to be a comment
> explaining it. But it probably should use ReadNewTransactionId()
> instead.
Hmm...  We don't want to *assign* a new xid here -- we just want to
know what the next one *will be*.  I will review locking around this
area, but I think it's sound.  If that checks out OK on review, I'll
add comments.
> Attached is a patch for some trivial changes, mostly typos.
Wow.  I transpose letters a lot don't I?  Thanks for finding those.
Will fix.
> Overall, this is very neat and well-commented code. All the data
> structures make my head spin, but I don't see anything unnecessary
> or have any suggestions for simplification. There's a few remaining
> TODO comments in the code, which obviously need to be resolved one
> way or another, but as soon as you're finished with any outstanding
> issues that you know of, I think this is ready for commit.
OK.  I may need to bounce some questions off the list to resolve some
of them.  The biggest, in my mind, is whether MySerializableXact
needs to be declared volatile.  I don't have my head around the
issues on that as well as I might.  Any thoughts on it?
Thanks for the multiple reviews and all the suggestions.  The code is
clearly better for your attention.
-Kevin