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] Enable lazy evaluation in functions in READ COMMITTED isolation level #12959

Open
pkj415 opened this issue Jun 20, 2022 · 1 comment
Open
Assignees
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 Jun 20, 2022

Jira Link: DB-2693

Description

Lazy evaluation is disabled in YSQL for READ COMMITTED isolation level as part of #13428. This issue tracks work that is needed to allow lazy evaluation and reap its benefits.

In a nutshell, what we need to do is to move management of a consistent read point from the pg_client_session.cc proxy (in tserver process) to the YSQL backend.

@pkj415 pkj415 added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage 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 pkj415 added pg-compatibility Label for issues that result in differences b/w YSQL and Pg semantics and removed kind/bug This issue is a bug priority/medium Medium priority issue status/awaiting-triage Issue awaiting triage labels Jun 20, 2022
pkj415 added a commit that referenced this issue Jun 27, 2022
…ackfill read time

Summary:
This refactor is needed to ease understanding of a larger change that is being
planned next that will allow a backend to switch back and forth between
snapshots (i.e., consistent read points) in READ COMMITTED isolation.

Till now, the read time required by backfill and the in txn limit logic were using
the same statement_read_time variable in exec parameters. This is confusing
and might lead to bugs later. This diff separates both.

Test Plan:
./yb_build.sh --cxx-test pgwrapper_pg_index_backfill-test --gtest_filter PgIndexBackfillTest.CreateUniqueIndexWriteAfterSafeTime
./yb_build.sh --cxx-test pgwrapper_pg_index_backfill-test --gtest_filter PgIndexBackfillTest.ReadTime

Reviewers: jason, dmitry

Reviewed By: dmitry

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D17770
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jul 9, 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] Allow saving a snapshot, switching the another and reusing the saved snapshot in YSQL (similar to PostgreSQL) [YSQL] Support lazy evaluation Jul 13, 2022
@pkj415 pkj415 changed the title [YSQL] Support lazy evaluation [YSQL] Support lazy evaluation in functions in READ COMMITTED isolation level Jul 13, 2022
@pkj415 pkj415 changed the title [YSQL] Support lazy evaluation in functions in READ COMMITTED isolation level [YSQL] Enable lazy evaluation in functions in READ COMMITTED isolation level Aug 8, 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 labels Oct 25, 2022
pkj415 added a commit that referenced this issue Mar 2, 2023
…mit and index backfill read time

Summary:
This refactor is needed to ease understanding of a larger change that is being
planned next that will allow a backend to switch back and forth between
snapshots (i.e., consistent read points) in READ COMMITTED isolation.

Till now, the read time required by backfill and the in txn limit logic were using
the same statement_read_time variable in exec parameters. This is confusing
and might lead to bugs later. This diff separates both.

Original commit: 893cb65 / D17770

Test Plan:
Jenkins: urgent
./yb_build.sh --cxx-test pgwrapper_pg_index_backfill-test --gtest_filter PgIndexBackfillTest.CreateUniqueIndexWriteAfterSafeTime
./yb_build.sh --cxx-test pgwrapper_pg_index_backfill-test --gtest_filter PgIndexBackfillTest.ReadTime

Reviewers: dmitry, jason

Reviewed By: jason

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D23232
@pao214
Copy link
Contributor

pao214 commented Aug 23, 2024

@pkj415 Does edbd06b resolve this issue?

@pao214 pao214 closed this as completed Aug 23, 2024
@pao214 pao214 reopened this Aug 23, 2024
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/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
Status: In Progress
Development

No branches or pull requests

3 participants