Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)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. 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.
If people really think that route is better I can put a patch
together so that the two approaches can be compared side-by-side.
If we go that way, it seemed that maybe we should move
LastSxactCommitSeqNo to VariableCacheData, right below
latestCompletedXid. We need some way to communicate that the
commitSeqNo has been set -- probably a volatile boolean in local
memory, and we need to acquire ProcArrayLock in some places we
currently don't within the SSI code to get at the value. Off-hand,
I don't see how that can be done without holding
SerializableXactHashLock while grabbing ProcArrayLock; and if we
need to do that to grab the assigned value, I'm not sure how much
we're buying. Do you see some way to avoid that?
The other thing we need to do if we go with the looser semantics is
be pessimistic -- in all tests for dangerous structures we need to
assume that any transaction which is prepared *may* also be
committed, and may have committed before any other transaction which
is prepared or committed. It would be good to put some bounds on
this. I guess any monotonically increasing number in the system
which can be grabbed just before the commit would do.
Unless you've come up with some clever approach we missed, I fear
that a patch based on the looser semantics will be bigger and more
fragile.
Do you agree that the patch Dan and I posted *does not add any LW
locking* to a normal (not prepared) transaction if it either has no
XID or is not SERIALIZABLE? And that ProcArrayLock is not held
during COMMIT PREPARED or ROLLBACK PREPARED unless the transaction
is SERIALIZABLE? I'm a bit concerned that we're headed down this
other path because people aren't clear about these facts.
-Kevin