On 07.07.2011 21:50, Heikki Linnakangas wrote:
On 07.07.2011 19:41, Kevin Grittner wrote:
Heikki Linnakangas<heikki.linnakan...@enterprisedb.com> wrote:
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.

This comment in the patch actually suggests a stronger
requirement:

+ * For correct SERIALIZABLE semantics, the commitSeqNo must
appear to be set
+ * atomically with the work of the transaction becoming visible
to other
+ * transactions.

So, is it enough for the commitSeqNos to be set in the order the
transactions become visible to others? I'm assuming 'yes' for now,
as the approach being discussed to assign commitSeqNo in
ProcArrayEndTransaction() without also holding
SerializableXactHashLock is not going to work otherwise, and if I
understood correctly you didn't see any correctness issue with
that. Please shout if I'm missing something.

Hmm. The more strict semantics are much easier to reason about and
ensure correctness.

True.

I think that the looser semantics can be made
to work, but there needs to be more fussy logic in the SSI code to
deal with the fact that we don't know whether the visibility change
has occurred. Really what pushed us to the patch using the stricter
semantics was how much the discussion of how to cover the edge
conditions with the looser semantics made our heads spin. Anything
that confusing seems more prone to subtle bugs.

Let's step back and analyse a bit closer what goes wrong with the
current semantics. The problem is that commitSeqNo is assigned too late,
sometime after the transaction has become visible to others.

Looking at the comparisons that we do with commitSeqNos, they all have
conservative direction that we can err to, when in doubt. So here's an
idea:

Let's have two sequence numbers for each transaction: prepareSeqNo and
commitSeqNo. prepareSeqNo is assigned when a transaction is prepared (in
PreCommit_CheckForSerializableConflicts), and commitSeqNo is assigned
when it's committed (in ReleasePredicateLocks). They are both assigned
from one counter, LastSxactCommitSeqNo, so that is advanced twice per
transaction, and prepareSeqNo is always smaller than commitSeqNo for a
transaction. Modify operations that currently use commitSeqNo to use
either prepareSeqNo or commitSeqNo, so that we err on the safe side.

That yields a much smaller patch (attached). How does this look to you,
am I missing anything?

Committed, so that we get this into beta3. We had some quick offlist discussion with Keving and Dan about this, Kevin pointed out a few more places that needed to use prepareSeqNo instead of commitSeqNo, out of which one seemed legitimate to me, and was included in the committed patch.

Kevin and Dan also pointed out that a 2PC transaction can stay in prepared state for a long time, and we could optimize that by setting prepareSeqNo only at the final COMMIT PREPARED. I objected to that, for the reason that it's currently very convenient for testing purposes that a transaction prepared with PREPARE TRANSACTION is in the exact same state as regular transaction that's going through regular commit processing.

--
  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