Re: Serializable snapshot isolation patch

> Jeff Davis  wrote:
> On Mon, 2010-10-18 at 13:26 -0500, Kevin Grittner wrote:
> I assume here that you mean that you _did_ see the failure
> (serialization error) and therefore did not see the problem?
Yeah.
> Also, are you sure it was using the GiST index for the searches and
> didn't just get a full table lock or full index lock?
I think so, but I'm at home and don't have details here.  Will check
tomorrow.
> I'll try to narrow it down. It could be a problem with PERIOD, or
> maybe it wasn't using the GiST index for a search when I thought it
> was (I didn't run EXPLAIN at every point, so I'll double-check).
> Clearly, a problem exists somewhere though.
Hmmm...  When Joe was looking at the patch he exposed an intermittent
problem with btree indexes which turned out to be related to improper
handling of the predicate locks during index page clean-up caused by a
vacuum.  Easy to fix once found, but it didn't always happen, even
with identical runs.  (I'm guessing that was due to the randomness in
picking a page to split when inserting into a group of identical
keys.)  Perhaps a similar bug lurks in the GiST predicate locking.
> Eventually, we may need to keep statistics about the number of
> conflicts happening, and start to rate-limit new serializable
> transactions (effectively reducing parallelism) to ensure that
> reasonable progress can be made (hopefully faster than serial
> execution).
Ah, you've exposed just how self-serving my interest in an admission
control policy mechanism is!  ;-)
http://archives.postgresql.org/pgsql-hackers/2009-12/msg02189.php
>> I also *really* hope to add the "SERIALIZABLE READ ONLY
>> DEFERRABLE" mode so that pg_dump and other read-only transactions
>> don't push things into a state where the rollback rate spikes:
>>
>> http://archives.postgresql.org/pgsql-hackers/2010-09/msg01771.php
>
> One potential issue there is starvation. I don't see how you would
> guarantee that there will ever be a point to grab a safe snapshot,
> even if all of the other transactions are completing.
After mulling it over in greater detail the previously, I see your
point.  I'll think about it some more, but this particular idea might
be a dead end.
>>> a. We should probably have a compatibility GUC that makes
>>> SERIALIZABLE equal to REPEATABLE READ. My opinion is that this
>>> should be only for compatibility, and should default to "off"
>>> (i.e. SSI code enabled) either in 9.1 or soon after.
>>
>> I'm inclined to agree with you, with the strong preference for a
>> 9.1 setting of off. This was previously discussed, and there were
>> people who felt that we should avoid a "behavior-changing" GUC
>> like that, so I didn't add it. It wouldn't be hard to put it in,
>> if that's the community consensus.
>
> I think that was me, but I'm OK with behavior-changing GUCs as long
> as they are there for compatibility and we intend to push people
> toward the new behavior in the long run (like
> standard_conforming_strings, but hopefully not as long-lived).
>
> Maybe it's not necessary at all, because your patch offers strictly
> better guarantees (at the cost of more serialization failures), and
> if they want the old behavior then REPEATABLE READ is already
> there.
Anyone else with an opinion on this?
> So a lock can't ever cover itself?
No.  If a transaction requests an SIRead lock identical to one it
already holds, we ignore the request.
> When using locks in an unconventional way, it would be helpful to
> describe the invalid schedules that you're preventing. Perhaps an
> example if you think it would be reasonably simple? Also some
> indication of how another process is intended to modify the list
> without walking it.
I will review that comment and add something along those lines.
-Kevin