Re: SSI patch version 14

Thanks for the review, Jeff!

Dan Ports wrote:
> On Tue, Jan 25, 2011 at 01:07:39AM -0800, Jeff Davis wrote:
>> At a high level, there is a nice conceptual simplicity. Let me
>> try to summarize it as I understand it:
>> * RW dependencies are detected using predicate locking.
>> * RW dependencies are tracked from the reading transaction (as an
>> "out") conflict; and from the writing transaction (as an "in"
>> conflict).
>> * Before committing a transaction, then by looking only at the RW
>> dependencies (and predicate locks) for current and past
>> transactions, you can determine if committing this transaction
>> will result in a cycle (and therefore a serialization anomaly);
>> and if so, abort it.
>
> This summary is right on. I would add one additional detail or
> clarification to the last point, which is that rather than
> checking for a cycle, we're checking for a transaction with both
> "in" and "out" conflicts, which every cycle must contain.

Yep. For the visual thinkers out there, the whole concept can be
understood by looking at the jpeg file that's in the Wiki page:

http://wiki.postgresql.org/images/e/eb/Serialization-Anomalies-in-Snapshot-Isolation.png

>> * In RegisterSerializableTransactionInt(), for a RO xact, it
>> considers any concurrent RW xact a possible conflict. It seems
>> like it would be possible to know whether a RW transaction may
>> have overlapped with any committed RW transaction (in
>> finishedLink queue), and only add those as potential conflicts.
>> Would that work? If so, that would make more snapshots safe.
>
> Interesting idea. That's worth some careful thought. I think it's
> related to the condition that the RW xact needs to commit with a
> conflict out to a transaction earlier than the RO xact. My first
> guess is that this wouldn't make more transactions safe, but could
> detect safe snapshots faster.

Yes, that would work. It would lower one type of overhead a little
and allow RO transactions to be released from SSI tracking earlier.
The question is how to determine it without taking too much time
scanning the finished transaction list for every active read write
transaction every time you start a RO transaction. I don't think
that it's a trivial enough issue to consider for 9.1; it's certainly
one to put on the list to look at for 9.2.

>> * When a transaction finishes, then PID should probably be set to
>> zero. You only use it for waking up a deferrable RO xact waiting
>> for a snapshot, right?
>
> Correct. It probably wouldn't hurt to clear that field when
> releasing the transaction, but we don't use it after.

I thought they might show up in the pid column of pg_locks, but I
see they don't. Should they? If so, should we still see the pid
after a commit, for as long as the lock survives?

-Kevin

On Tue, 2011-01-25 at 09:41 -0600, Kevin Grittner wrote:
> Yep. For the visual thinkers out there, the whole concept can be
> understood by looking at the jpeg file that's in the Wiki page:
>
> http://wiki.postgresql.org/images/e/eb/Serialization-Anomalies-in-Snapshot-Isolation.png

Yes, that helped a lot throughout the review process. Good job keeping
it up-to-date!

> Yes, that would work. It would lower one type of overhead a little
> and allow RO transactions to be released from SSI tracking earlier.
> The question is how to determine it without taking too much time
> scanning the finished transaction list for every active read write
> transaction every time you start a RO transaction. I don't think
> that it's a trivial enough issue to consider for 9.1; it's certainly
> one to put on the list to look at for 9.2.

It's OK to leave it to 9.2. But if it's a RO deferrable transaction,
it's just going to go to sleep in that case anyway; so why not look for
an opportunity to get a safe snapshot right away?

Regards,
Jeff Davis

Jeff Davis <pgsql(at)j-davis(dot)com> wrote:

> It's OK to leave it to 9.2. But if it's a RO deferrable
> transaction, it's just going to go to sleep in that case anyway;
> so why not look for an opportunity to get a safe snapshot right
> away?

If you're talking about doing this only for DEFERRABLE transactions
it *might* make sense for 9.1. I'd need to look at what's involved.
We make similar checks for all read only transactions, so they can
withdraw from SSI while running, if their snapshot *becomes* safe.
I don't think I'd want to consider messing with that code at this
point.

-Kevin