On Thu, Feb 4, 2016 at 7:48 AM, Robert Haas <robertmh...@gmail.com> wrote: > On Sun, Jan 31, 2016 at 5:19 PM, Thomas Munro > <thomas.mu...@enterprisedb.com> wrote: >> As described in a recent Reddit discussion and bug report 9301, >> there are scenarios where overlapping concurrent read-write sequences >> produce serialization failures without constraints, but produce >> constraint violations when there is a unique constraint. A simple >> example is deciding on a new value for a primary key by first checking >> the existing contents of a table. >> >> This makes life difficult if you're trying to build systems that >> automatically retry SERIALIZABLE transactions where appropriate, >> because you have to decide how and when to handle unique constraint >> violations too. For example, people have experimented with automatic >> retry-on-40001 at the level of HTTP requests for Django applications >> when using the middleware that maps HTTP requests to database >> transactions, and the same opportunities presumably exist in Java >> application servers and other web service technologies, but unique >> constraint violations get in the way of that. >> >> Here is an experimental patch to report such SSI conflicts. I had to >> make a change to aminsert_function so that the snapshot could be >> available to btree insert code (which may be unpopular). The >> questions on my mind now are: Are there still conflicting schedules >> that would be missed, or significant new spurious conflict reports, or >> other theoretical soundness problems? Is that PredicateLockPage call >> sound and cheap enough? It is the only new code that isn't in a path >> already doomed to ereport, other than the extra snapshot propagation, >> and without it read-write-unique-3.spec (taken from bug report 9301) >> doesn't detect the conflict. >> >> Thoughts? > > I don't feel qualified to have an opinion on whether this is an > improvement. I'm a little skeptical of changes like this on general > principle because sometimes one clientele wants error A to be reported > rather than error B and some other clientele wants the reverse. > Deciding who is right is above my pay grade.
I don't see it as a difficult choice between two reasonable alternatives. It quacks suspiciously like a bug. The theoretical problem with the current behaviour is that by reporting a unique constraint violation in this case, we reach an outcome that is neither a serialization failure nor a result that could occur in any serial ordering of transactions. The overlapping transactions both observed that the key they planned to insert was not present before inserting, and therefore they can't be untangled: there is no serial order of the transactions where the second transaction to run wouldn't see the key already inserted by the first transaction and (presumably) take a different course of action. (If it *does* see the value already present in its snapshot, or doesn't even look first before inserting and it turns out to be present, then it really *should* get a unique constraint violation when trying to insert.) The practical problem with the current behaviour is that the user has to work out whether a unique constraint violation indicates: 1. A programming error -- something is wrong that retrying probably won't fix 2. An unfavourable outcome in the case that you speculatively inserted without checking whether the value was present so you were expecting a violation to be possible, in which case you know what you're doing and you can figure out what to do next, probably retry or give up 3. A serialization failure that has been masked because our coding happens to check for unique constraint violations without considering SSI conflicts first -- retrying will eventually succeed. It's complicated for a human to work out how to distinguish the third category errors in each place where they might occur (and even to know that they are possible, since AFAIK the manual doesn't point it out), and impossible for an automatic retry-on-40001 framework to handle in general. SERIALIZABLE is supposed to be super easy to use (and devilishly hard to implement...). Here's an example. Suppose you live in a country that requires invoices to be numbered without gaps starting at one for each calendar year. You write: BEGIN ISOLATION LEVEL SERIALIZABLE; SELECT COALESCE(MAX(invoice_number) + 1, 1) FROM invoice WHERE year = 2016; INSERT INTO invoice VALUES (2016, $1, ...); -- using value computed above COMMIT; I think it's pretty reasonable to expect that to either succeed or ereport SQLSTATE 40001, and I think it's pretty reasonable to want to be able to give the code that runs that transaction to a mechanism that will automatically retry the whole thing if 40001 is reported. Otherwise the promise of SERIALIZABLE is thwarted: you should be able to forget about concurrency and write simple queries that assume they are the only transaction in the universe. The activities of other overlapping transactions shouldn't change the outcome of your transaction, other than potentially triggering 40001. If you really want unique constraint violations for this case, then with this patch you can remove the SELECT and get the UCV, or use a lower isolation level since you are in fact depending on a hole in the isolation between transactions. Or you could consider the new SSI conflict to be spurious (though it isn't!) and just retry, which doesn't seem to have a downside since you already have to handle those. I added the (German?) invoice numbering example above as specs/read-write-unique-4.spec in the attached new patch, with three interesting permutations. I am not sure what to think about the third permutation yet -- an overlap between two transactions where one of them just writes the key, and the other reads then writes -- the outcome is possibly wrong there and I don't know off the top of my head how to fix it if it is. My thinking was that this case should be covered by the new predicate lock I introduced, because even though s1 didn't explicitly SELECT anything, by successfully passing the unique check phase it really has read something, so it's a transaction that has read (the absence of the value) and written (the value). This is the part of the problem that I'm least sure of, but am hoping to work out with some SSI expert help. -- Thomas Munro http://www.enterprisedb.com
Description: Binary data
-- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers