Skip to content
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

Unusable outcome when two sessions do inserts that jointly will violate a constraint. #1985

Open
bllewell opened this issue Aug 7, 2019 · 4 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue
Projects

Comments

@bllewell
Copy link
Contributor

bllewell commented Aug 7, 2019

Jira Link: DB-2546
The test uses a table created thus:

\set AUTOCOMMIT on
create table t(
  a integer
    constraint t_a_nn not null
    constraint t_a_unq unique,
  b integer
    constraint t_b_nn not null
    constraint t_b_unq unique);

Now, in Session 1, do this:

\set AUTOCOMMIT on
insert into t(a, b) values(17, 42);

and then in Session 2 do this:

begin;
insert into t(a, b) values(18, 42);

The insert causes the error duplicate key value violates unique constraint "t_b_unq". So issue rollback now. Notice that the error text includes the name of the violated constraint. This is useful because application code can parse the text and generate a useful response for the human user like "The username that you chose is already taken. Please make another choice."

So far, the behavior is identical in PostgreSQL Version 11.2 and YugaByte DB Version 1.3.0.

Now come the two-session tests.

First, PostgreSQL Version 11.2.

In Session 1:

delete from t;
begin;
set transaction isolation level repeatable read;
insert into t(a, b) values(17, 42);

but don't commit yet. Then, in Session 2:

begin;
set transaction isolation level repeatable read;
insert into t(a, b) values(18, 42);

The insert hangs until Session 1 issues commit. When it does, Session 2 gets this error: duplicate key value violates unique constraint "t_b_unq". So we still get a usefully parsable error message, even in the two session case. Alternatively, if Session 1 issues rollback, possibly because of some later error in the same transaction, then Session 2's insert simply goes ahead with no error.

Now, YugaByte DB Version 1.3.0.

In Session 1:

delete from t;
begin;
set transaction isolation level repeatable read;
insert into t(a, b) values(17, 42);

but don't commit yet. Then, in Session 2:

begin;
set transaction isolation level repeatable read;
insert into t(a, b) values(18, 42);

The insert fails immediately with the error " Operation failed. Try again.: Conflicts with higher priority transaction". So now the application code has nothing to parse to determine which constraint has been violated.

Notice that this will make the application programming complicated. There must be two exception handlers, one (set) for the duplicate key value violates unique constraint "t_b_unq" error (and similar for any other constraints that might be violated). And one for the " Operation failed. Try again.: Conflicts with higher priority transaction" error.

It's even worse than this. Using PostgreSQL, you may, or may not, get the one (class of) error in Session 2 according to whether Session 1 eventually issues commit or (is forced to) issue rollback. And when you get the error, there's no retry possibility and so no retry code to write. But in YugaByte DB, you get the error prematurely in Session 2 so that you might succeed on retry (in an infinite loop with a sleep and a timeout). This makes for tortuous application code and poor usability for the end user: either an unhelpful error message, or an unpredictable delay that might resolve quietly or might resolve with the aforementioned unhelpful error message.

Can we make YugaByte DB behave the same as PstgreSQL?

@ndeodhar
Copy link
Contributor

ndeodhar commented Aug 7, 2019

Note that there is subtle difference in the two examples:
In the first example, the INSERT is already committed and so we get a clean error like duplicate key value violates unique constraint.
In the second example, there is another transaction (lets call it transaction A) that has written the same row and we detect a potential conflict while writing the same row in transaction B. However, it's possible that transaction A gets aborted, in which case if transaction B retries, it will succeed in writing the row. So Try Again error is not incorrect here.

But agree with the general sentiment that the error message can be more useful by providing details like what constraint it violates and hinting that it may succeed if tried again.

@ndeodhar ndeodhar self-assigned this Aug 7, 2019
@ndeodhar ndeodhar added the area/ysql Yugabyte SQL (YSQL) label Aug 7, 2019
@ndeodhar ndeodhar added this to To do in YSQL via automation Aug 7, 2019
@bllewell
Copy link
Contributor Author

bllewell commented Aug 7, 2019

Thanks, Neha. The "subtle" difference was very much part of the design of my experiment. To recap:

— when PostgreSQL detects that a concurrent session has an uncommitted txn that might be incompatible with the present session's DML attempt, the present session patiently waits until the other session issues commit or rollback. Then the present session knows where it stands and can either go ahead without error or raise the error that it would have raised had the other session done what it did yesterday.

— when YugaByte DB detects that a concurrent session has an uncommitted txn that might be incompatible with the present session's DML attempt, the present session immediately raises a "potential conflict, please retry" error.

The PostgreSQL model allows much cleaner, and shorter, application code than does the YugaByte DB model. And it allows more helpful messages to be composed for the human end-user.

So here is the key question:

— Can the YugaByte DB model be changed to be the same as the PostgreSQL model? This would also ease the intellectual task for programmers who are experienced with PostgreSQL and are new to YugaByte DB.

— Or is the presently observed YugaByte DB model non-negotiably determined by the difference between how its persistence "lower half" works compared to how PostgreSQL's "lower half" works?

@pkj415
Copy link
Contributor

pkj415 commented Aug 20, 2021

@bllewell this is a golden example!

  1. The issue of a txn not waiting on another one to commit/rollback will be resolved by the pessimistic locking work - [YSQL] Support wait-on-conflict concurrency control #5680
  2. About the error message - agree that we should fix it. Once we have pessimistic locking, we can do some plumbing to differentiate between a constraint violation and other errors.

I am adding this issue to tracking in #5683

@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jun 9, 2022
@pkj415
Copy link
Contributor

pkj415 commented Sep 5, 2023

Circling back here, I looked at this issue again and paged in the context.

We have implemented Wait-on-Conflict concurrency control, which can be turned on using enable_wait_queues=true (we earlier used to incorrectly call this mode pessimistic locking). With this mode, the issue where the second txn doesn't wait for the first one to complete, becomes void.

The second issue, where we throw a 'Operation failed. Try again.: Conflicts with higher priority transaction' instead of 'duplicate key value violates unique constraint "t_b_unq"', is still valid. To repro it, use Bryn's second example where the transactions are concurrent (ensure to keep yb_enable_read_committed_isolation=false if #12494 has still not been fixed).

The fix needs to be in Pg: we should intercept the kConflict and direct it to run Pg's code which throws the duplicate key error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue
Projects
Status: No status
YSQL
  
Backlog
Development

No branches or pull requests

4 participants