Re: SSI predicate locking on heap -- tuple or row?

On 26.05.2011 06:19, Kevin Grittner wrote:
>     /*
>      * Check whether the writer has become a pivot with an out-conflict
>      * committed transaction, while neither reader nor writer is committed. If
>      * the reader is a READ ONLY transaction, there is only a serialization
>      * failure if an out-conflict transaction causing the pivot committed
>      * before the reader acquired its snapshot.  (That is, the reader must not
>      * have been concurrent with the out-conflict transaction.)
>      */
>     if (!failure)
>     {
>         if (SxactHasSummaryConflictOut(writer))
>         {
>             failure = true;
>             conflict = NULL;
>         }
>         else
>             conflict = (RWConflict)
>                 SHMQueueNext(&writer->outConflicts,
>                              &writer->outConflicts,
>                              offsetof(RWConflictData, outLink));
>         while (conflict)
>         {
>             if (SxactIsCommitted(conflict->sxactIn)
>                 && (!SxactIsCommitted(reader)
>                     || conflict->sxactIn->commitSeqNo <= reader->commitSeqNo)
>                 && (!SxactIsCommitted(writer)
>                     || conflict->sxactIn->commitSeqNo <= writer->commitSeqNo)
>                 && (!SxactIsReadOnly(reader)
>                     || conflict->sxactIn->commitSeqNo <= reader->SeqNo.lastCommitBeforeSnapshot))
>             {
>                 failure = true;
>                 break;
>             }
>             conflict = (RWConflict)
>                 SHMQueueNext(&writer->outConflicts,
>                              &conflict->outLink,
>                              offsetof(RWConflictData, outLink));
>         }
>     }

The comment is not in sync with the code. The code is not checking that
"neither reader or writer has committed", but something more complicated.

Looking at OnConflict_CheckForSerializationFailure(), it's really hard
to see how it works, and why the conditions it checks are sufficient. I
find it helps tremendously to draw the dangerous structures being
checked, in addition to just explaining them in text. Ascii art is a bit
clunky, but I think we have to do it here.

I did some of that in the comments, and I think I understand it now. See
attached patch. Does that look right to you? (note that I swapped the
second and third check in the function, I think it's more
straightforward that way). I also added a couple of questions about the
conditions, marked with XXX comments. Can you answer those, please?

PS. Should we say "Cancelled on identification as pivot, during ...", or
"Cancelled on identification as a pivot, during ..." ? We seem to use
both in the error messages.

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