On 05.07.2011 20:03, Kevin Grittner wrote:
In reviewing the 2PC changes mentioned in a separate post, both Dan
and I realized that these were dependent on the assumption that
SSI's commitSeqNo is assigned in the order in which the transactions
became visible.  There is a race condition such that this is not
necessarily true.  It is a very narrow race condition, which would
come up very rarely in practice, but Murphy's Law being what it is,
I think we need to consider it a bug and fix it.

We considered a fix which would be contained within predicate.c code
and operate by making pessimistic assumptions, so that no false
negatives occurred.  The reason we didn't go that way is that the
code would be very convoluted and fragile.  The attached patch just
makes it atomic in a very direct way, and adjusts the predicate.c
code to use the right tests in the right places.  We were careful
not to add any LW locking to the path that a normal transaction
without an XID is terminating, since there had obviously been
significant work put into keeping locks out of that code path.

Hmm, I think it would be simpler to decide that instead of SerializableXactHashLock, you must hold ProcArrayLock to access LastSxactCommitSeqNo, and move the assignment of commitSeqNo to ProcArrayTransaction(). It's probably easiest to move LastSxactCommitSeqno to ShmemVariableCache too. There's a few places that would then need to acquire ProcArrayLock to read LastSxactCommitSeqno, but I feel it might still be much simpler that way.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to