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

[YSQL] Non-transactional side-effects can occur more than once when a conflict/ read restart error in functions/ procedures is handled by read committed isolation level #12958

Open
pkj415 opened this issue Jun 20, 2022 · 0 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug pg-compatibility Label for issues that result in differences b/w YSQL and Pg semantics priority/medium Medium priority issue

Comments

@pkj415
Copy link
Contributor

pkj415 commented Jun 20, 2022

Jira Link: DB-2692

Description

Right now, statements are internally retried in READ COMMITTED isolation level with exponential backoff if a kConflict or kReadRestart error is seen at the query layer. They are retried on a new latest snapshot for kConflict and with restart read time for kReadRestart errors.

However the following known issue can occur -

create table t(k serial primary key, v varchar(100) not null);
insert into t(v) values ('dog');

create or replace procedure f(new_v in text)
  language sql
as $body$
  deallocate dummy_query;
  insert into t(v) values ('cat'); -- if this faces a kConflict and the whole function is retried because the top level client statement is retried
$body$;

Session 1: begin; insert into t(v) values ('cat');
Session 2: prepare dummy_query as select * from t; call f();

call f() would fail because deallocation on dummy_query would be attempted more than once. The error wouldn't occur if only the statement that faced the conflict in the procedure was retried. This is not an issue in Postgres because it handles conflicts with eval plan qual handling (see #11573).

This issue would be solved for kConflicts once #11573 is fixed. It might be solved for kReadRestart too in case the solution for #11572 gets rid of statement retries for kReadRestart errors using some new scheme to get rid of kReadRestart errors.

@pkj415 pkj415 added area/ysql Yugabyte SQL (YSQL) pg-compatibility Label for issues that result in differences b/w YSQL and Pg semantics labels Jun 20, 2022
@pkj415 pkj415 self-assigned this Jun 20, 2022
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jun 20, 2022
pkj415 added a commit that referenced this issue Jul 11, 2022
…DL work

Summary:
Till now, we supported READ COMMITTED semantics only for SELECT/ INSERT/ UPDATE
and DELETE statements. This is done by restarting the statement after rolling
back work done by the failed execution of the statement.

With this diff, the logic is allowed for all other statements except DDLs.

TODOs for a later diff:
(1) Move read committed retries to a finer level in functions and procedures i.e., per query (#12958).
This is required for multiple reasons:

(i) Performance - in read committed isolation we can retry just the statement that faced a
kConflict or kReadRestart in the function/ procedure. We don't have to restart the whole
procedure or function.

(ii) We should not be redoing non-transactional side-effects in functions and procedures.
This will be resolved automatically once we perform retries at a statement level in the
func/ proc.

(2) Allow saving a snapshot, switching the another and reusing the saved snapshot in YSQL
Detailed use case in #12959

Test Plan:
./yb_build.sh --java-test org.yb.pgsql.TestPgIsolationRegress#isolationRegress
./yb_build.sh --java-test org.yb.pgsql.TestPgReadCommittedVolatileFuncs#testFunctionSemantics

Reviewers: alex, mtakahara

Reviewed By: mtakahara

Subscribers: lnguyen, bogdan, zyu, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D17440
pkj415 added a commit that referenced this issue Jul 13, 2022
…s for all non-DDL work

Summary:
Till now, we supported READ COMMITTED semantics only for SELECT/ INSERT/ UPDATE
and DELETE statements. This is done by restarting the statement after rolling
back work done by the failed execution of the statement.

With this diff, the logic is allowed for all other statements except DDLs.

TODOs for a later diff:
(1) Move read committed retries to a finer level in functions and procedures i.e., per query (#12958).
This is required for multiple reasons:

(i) Performance - in read committed isolation we can retry just the statement that faced a
kConflict or kReadRestart in the function/ procedure. We don't have to restart the whole
procedure or function.

(ii) We should not be redoing non-transactional side-effects in functions and procedures.
This will be resolved automatically once we perform retries at a statement level in the
func/ proc.

(2) Allow saving a snapshot, switching the another and reusing the saved snapshot in YSQL
Detailed use case in #12959

Original commit: a224db2 / D17440

Test Plan:
./yb_build.sh --java-test org.yb.pgsql.TestPgIsolationRegress#isolationRegress
./yb_build.sh --java-test org.yb.pgsql.TestPgReadCommittedVolatileFuncs#testFunctionSemantics

Reviewers: alex, mtakahara

Reviewed By: mtakahara

Subscribers: yql, zyu, bogdan, lnguyen

Differential Revision: https://phabricator.dev.yugabyte.com/D18252
@pkj415 pkj415 changed the title [YSQL] Move READ COMMITTED retries to finer per statement level in functions and procedures [YSQL] Non-transactional side-effects can occur more than once when a conflict/ read restart retry occurs in functions/ procedures Jul 28, 2022
pkj415 added a commit to pkj415/yugabyte-db that referenced this issue Feb 8, 2023
… statement level in functions and procedures

Summary:
[WIP]
This diff is to address the first TODO in a224db2

Test Plan: ./yb_build.sh --java-test org.yb.pgsql.TestPgIsolationRegress#isolationRegress

Reviewers: alex, mtakahara

Subscribers: yql, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D18296
@pkj415 pkj415 changed the title [YSQL] Non-transactional side-effects can occur more than once when a conflict/ read restart retry occurs in functions/ procedures [YSQL] Non-transactional side-effects can occur more than once when a conflict/ read restart error in functions/ procedures is handled by read committed isolation level Aug 29, 2023
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 pg-compatibility Label for issues that result in differences b/w YSQL and Pg semantics priority/medium Medium priority issue
Projects
None yet
Development

No branches or pull requests

2 participants