> 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
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
