-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[YSQL] ON CONFLICT (k) DO UPDATE can still throw conflict errors #1974
Labels
Comments
frozenspider
changed the title
[YSQL] ON CONFLICT (k) DO UPDATE can still throw conflict errors
Aug 6, 2019
on conflict (k) do update
can still throw conflict errors
frozenspider
added
kind/bug
This issue is a bug
and removed
community/request
Issues created by external users
labels
Aug 6, 2019
This might be related to #1611! |
Re-tested this under @nocaway 's recent partial fix for #1611 (commit ca3ebdb), this bug is still reproducible. |
@nocaway discovered this bug to be unrelated to #1611
|
spolitov
added a commit
that referenced
this issue
Aug 19, 2019
Summary: The transaction (T1) could be aborted while a transaction (T2) with higher priority was writing intents. In this a case, T1 becomes invalid and could read invalid values, for instance, it could read values from T2, and decide that a duplicate key value violates the unique constraint, which would not happen with non-aborted transaction. From the point of view of consistency, there is no problem, because such aborted transaction would not be able to commit. But it could provide an incorrect error message to the user, saying that constraint is violated instead of "Transaction aborted" This diff addresses the issue by adding checks for aborted state, while the transaction is writing intents. Provided fix should not affect performance, since all checks are done locally, i.e. without RPC calls. Also added caching and preliminary rejection of the operation, so in the high contention scenario performance should only increase. Test Plan: ybd --gtest_filter PgLibPqTest.OnConflict Reviewers: alex, timur, mikhail Reviewed By: timur Subscribers: ybase, bogdan Differential Revision: https://phabricator.dev.yugabyte.com/D7063
spolitov
added a commit
that referenced
this issue
Aug 20, 2019
Summary: As shown by Jepsen log investigation, the initial version of D7063 ( ee7bc3e ) was enough to fix #1974. The extra check that was added to WriteOperationCompletionCallback is superfluous and also contains a bug for the case where this operation is the first operation of the transaction on this tablet. In such a case, this check always decides that the transaction has been aborted, while (obviously) some transactions don't get aborted. The reason the call to TransactionParticipant::CheckAborted returns an aborted status for the first operation of a transaction on a particular tablet, in this case, is that the transaction metadata should be written by the operation that ended up failing. Just to clarify, if an operation fails, we don't write metadata for this transaction on the transaction participant. And the failure of the operation is what initiates this check that eventually calls CheckAborted. Just as a reminder, absent metadata for a particular transaction id in TransactionParticipant is indistinguishable from the situation when this transaction that has been aborted and cleaned up. This revision removes the extra check. Test Plan: ybd --gtest_filter PgLibPqTest.OnConflict -n 100 Reviewers: alex, neha, mikhail Reviewed By: mikhail Subscribers: ybase, bogdan Differential Revision: https://phabricator.dev.yugabyte.com/D7090
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
With version 1.3.1.0, using Jepsen ed8ecfb9146816cb99e458c9cb92f59e19a7b78f, the append test uses
insert ... on conflict (k) do update ...
to insert or update records in place. This table has two columns: a primary integer keyk
, and a textv
.You would think that when there exists a row with some key
k
, this would update the associatedv
usingCONCAT
, and indeed it does--some of the time. Other times, however, it throws:See, for instance, 20190806T095424.000-0400.zip
This happens whether we use
on conflict (k)
or explicitly specify the index viaon conflict on constraint append1_pkey
", and it happens regardless of whetherk
is a primary key, or simply a unique column. You can reproduce it with any append test, e.g.The text was updated successfully, but these errors were encountered: