Re: BUG #12330: ACID is broken for unique constraints

Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com> wrote:
> On 12/29/14, 10:53 AM, Kevin Grittner wrote:
>> Merlin Moncure <mmoncure(at)gmail(dot)com> wrote:
>>
>>> Serialization errors only exist as a concession to concurrency
>>> and performance. Again, they should be returned as sparsely as
>>> possible because they provide absolutely (as Tom pointed
>>> out) zero detail to the application.
>>
>> That is false. They provide an *extremely* valuable piece of
>> information which is not currently available when you get a
>> duplicate key error -- whether the error occurred because of a
>> race condition and will not fail for the same cause if retried.
>
> As for missing details like the duplicated key value, is there a
> reasonable way to add that as an errdetail() to a serialization
> failure error?

Sure. What we've been discussing is the case where a unique index
is checked for a duplicate with visibility which would cause it to
fail; whether, at transaction isolation level of serializable, to
check to see if the committed transaction which created the
existing entry overlaps the entry checking for duplicates, and use
a different SQLSTATE. Anything available for display in the detail
currently would still be available. We give different detail
messages for different types of serialization failures; it would
seem a little silly not to mention the table and the duplicate
values for this type of serialization failure. My guess is that in
production this would be completely ignored by the software, but
logging it might be helpful in tuning.

> We do still have to be careful here though; you could still have
> code using our documented upsert methodology inside a serialized
> transaction. For example, via a 3rd party extension that can't
> assume you're using serialized transactions. Would it still be OK
> to treat that as a serialization failure and bubble that all the
> way back to the application?

If the current upsert function example were used, unmodified, by
serializable transactions with the suggested changes, it would not
loop back for another try within the function, but generate a
serialization failure for the transaction as a whole. If there was
a framework watching for that and retrying transactions when it is
seen, the transaction would start over and see the new state and
behave correctly. Without the change I'm proposing, I bet it would
loop endlessly, until interrupted. [tries it] Yep, there it is.
All the hand-wringing about how we might break existing
applications is entirely backwards. Under either repeatable read
or serializable that technique goes to 100% of one CPU until you
cancel it -- completely broken. If it generated a serialization
failure the code in our docs would throw the serialization failure
out to the client, which (in most environments using serializable
transactions, or even repeatable read) would retry the transaction
from the beginning, with much less pain. It makes me wonder
whether it would be better to generate the serialization failure
for both repeatable read and serializable, since those are the
cases where the sample code in our docs generates the endless loop.

> As part of this, we should probably modify our upsert example so
> it takes transaction_isolation into account...

Probably.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company