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.

Sign up for GitHub

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

Jump to bottom

[YSQL] READ COMMITTED semantics should apply to all non-DDL statements (not just INSERT, UPDATE, DELETE, and SELECT) #12254

Closed
pkj415 opened this issue Apr 25, 2022 · 1 comment
Closed

[YSQL] READ COMMITTED semantics should apply to all non-DDL statements (not just INSERT, UPDATE, DELETE, and SELECT) #12254

pkj415 opened this issue Apr 25, 2022 · 1 comment
Assignees
@pkj415
Labels
area/ysql Yugabyte SQL (YSQL) kind/enhancement This is an enhancement of an existing feature 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 Apr 25, 2022 •
edited
Loading

Jira Link: DB-498

Description

This issue tracks work for extending READ COMMITTED semantics to all non-DDL statements, not just the 4 basic statements i.e., insert, update, delete and select
@pkj415 pkj415 added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Apr 25, 2022
@pkj415 pkj415 self-assigned this Apr 25, 2022
@pkj415 pkj415 changed the title [YSQL] READ COMMITTED semantics don't apply to statement with EXPLAIN [YSQL] READ COMMITTED semantics don't apply to EXPLAIN and DO statements Apr 25, 2022
@pkj415 pkj415 mentioned this issue Apr 29, 2022
Open
@pkj415 pkj415 added the pg-compatibility Label for issues that result in differences b/w YSQL and Pg semantics label May 24, 2022
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels May 24, 2022
@pkj415 pkj415 changed the title [YSQL] READ COMMITTED semantics don't apply to EXPLAIN and DO statements [YSQL] READ COMMITTED semantics don't apply to EXPLAIN, DO and CALL statements Jun 6, 2022
pkj415 added a commit to pkj415/yugabyte-db that referenced this issue Jun 17, 2022
@pkj415
[yugabyte#12254] [yugabyte#11640] [YSQL] Support read committed seman…
7de0e16 
…tics 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 statements except DDLs.

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

Reviewers: alex, mtakahara

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D17440
pkj415 added a commit to pkj415/yugabyte-db that referenced this issue Jul 9, 2022
@pkj415
[yugabyte#12254] [YSQL] Support read committed semantics for all non-…
0fff550 
…DDL work

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.
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 is be resolved automatically once we perform retries at a statement level in the
func/ proc.

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

Reviewers: alex, mtakahara

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D17440
pkj415 added a commit that referenced this issue Jul 11, 2022
@pkj415
[#12254, #11640] YSQL: Support read committed semantics for all non-D…
a224db2 
…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
@pkj415
[BACKPORT 2.14][#12254, #11640] YSQL: Support read committed semantic…
fd49967 
…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] READ COMMITTED semantics don't apply to EXPLAIN, DO and CALL statements [YSQL] READ COMMITTED semantics should apply to all non-DDL statements (not just INSERT, UPDATE, DELETE, and SELECT) Jul 25, 2022
@yugabyte-ci yugabyte-ci added kind/enhancement This is an enhancement of an existing feature and removed kind/bug This issue is a bug status/awaiting-triage Issue awaiting triage labels Jul 27, 2022
@m-iancu
Copy link
Contributor

m-iancu commented Jul 28, 2022

Fixed by a224db2.

@m-iancu m-iancu closed this as completed Jul 28, 2022
@pkj415 pkj415 mentioned this issue Aug 9, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pkj415 pkj415

Labels
area/ysql Yugabyte SQL (YSQL) kind/enhancement This is an enhancement of an existing feature pg-compatibility Label for issues that result in differences b/w YSQL and Pg semantics priority/medium Medium priority issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@m-iancu @pkj415 @yugabyte-ci