[YSQL] Enable DDL related tests to run with read committed isolation #19975
Assignees
Labels
Comments
pkj415
added
area/ysql
yugabyte-ci
added
kind/bug
samiahmedsiddiqui
pushed a commit
to samiahmedsiddiqui/yugabyte-db
that referenced
this issue
Nov 23, 2023
…version
Summary:
To reproduce the bug:
1. Create a cluster and convert it into per-database catalog version mode
```
yugabyte=# select * from pg_yb_catalog_version;
db_oid | current_version | last_breaking_version
--------+-----------------+-----------------------
1 | 1 | 1
13242 | 1 | 1
13243 | 1 | 1
13245 | 1 | 1
13246 | 1 | 1
(5 rows)
```
The above query output indicates that the cluster is running in per-database
catalog version mode.
2. Run the following DDL global-impact DDL statement
```
yugabyte=# ALTER ROLE yugabyte SUPERUSER;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
```
In the PG log:
```
F1122 01:04:40.396021 15991 pg_txn_manager.cc:346] Check failed: !IsDdlMode() READ COMMITTED semantics don't apply to DDL transactions
```
This happens when we call `yb_increment_all_db_catalog_versions` to increment
catalog versions for all databases. This is done inside function
`YBDecrementDdlNestingLevel` which first does
```
--ddl_transaction_state.nesting_level;
```
and then if `ddl_transaction_state.nesting_level == 0` it calls
`yb_increment_all_db_catalog_versions` if the current DDL statement has global
impact. In `GetTransactionSnapshot` we have
```
if (IsYBReadCommitted() && YBGetDdlNestingLevel() == 0)
```
The function `yb_increment_all_db_catalog_versions` is still called as part of
the DDL statement, but `ddl_transaction_state.nesting_level` is already 0 so
`YBGetDdlNestingLevel()` returns 0. As a result the above `if` condition is true
and the DDL statement is treated as a non-DDL statement, eventually leading to
the assertion failure.
The DDL mode is already transferred to pggate when we call
`YBCPgEnterSeparateDdlTxnMode`. So I added `YBCPgIsDdlMode()` and uses it to
replace `YBGetDdlNestingLevel() == 0`.
I reverted some work arounds in `pg_catalog_version-test.cc` which used snapshot
isolation because otherwise those unit tests would fail under read committed
isolation.
Test Plan: ./yb_build.sh debug --cxx-test pg_catalog_version-test
Reviewers: tvesely, pjain
Reviewed By: pjain
Subscribers: yql
Differential Revision: https://phorge.dev.yugabyte.com/D30417
pkj415
added a commit
that referenced
this issue
Nov 30, 2023
Summary: TestPgRegressIndex rarely fails with the following errror if READ COMMITTED isolation is enabled: ``` jenkins@jenkins-master ~/jobs/github-yugabyte-db-alma8-master-gcc11-fastdebug/builds$ find -name '*fatal_failure_details*txt' | xargs grep -l 'Restarting a DDL transaction not supported' ./4258/archive/java/yb-pgsql/target/surefire-reports_org.yb.pgsql.TestPgRegressIndex__testPgRegressIndex/org.yb.pgsql.TestPgRegressIndex.testPgRegressIndex.fatal_failure_details.ts-1.127.52.22.121-port13683.2023-11-19T05_58_16.pid13287.txt ./4248/archive/java/yb-pgsql/target/surefire-reports_org.yb.pgsql.TestPgRegressIndex__testPgRegressIndex/org.yb.pgsql.TestPgRegressIndex.testPgRegressIndex.fatal_failure_details.ts-1.127.29.54.114-port15898.2023-11-18T00_42_03.pid223695.txt ./4262/archive/java/yb-pgsql/target/surefire-reports_org.yb.pgsql.TestPgRegressIndex__testPgRegressIndex/org.yb.pgsql.TestPgRegressIndex.testPgRegressIndex.fatal_failure_details.ts-1.127.206.118.77-port16906.2023-11-20T11_42_24.pid40994.txt ``` Disabling read committed in this test to avoid this. Added a TODO to re-enable read committed for this test as part of #19975. Test Plan: ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressIndex' Reviewers: jason, tvesely Reviewed By: tvesely Subscribers: tvesely, yql Differential Revision: https://phorge.dev.yugabyte.com/D30509
myang2021
added a commit
that referenced
this issue
Dec 7, 2023
…atalog version
Summary:
To reproduce the bug:
1. Create a cluster and convert it into per-database catalog version mode
```
yugabyte=# select * from pg_yb_catalog_version;
db_oid | current_version | last_breaking_version
--------+-----------------+-----------------------
1 | 1 | 1
13242 | 1 | 1
13243 | 1 | 1
13245 | 1 | 1
13246 | 1 | 1
(5 rows)
```
The above query output indicates that the cluster is running in per-database
catalog version mode.
2. Run the following DDL global-impact DDL statement
```
yugabyte=# ALTER ROLE yugabyte SUPERUSER;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
```
In the PG log:
```
F1122 01:04:40.396021 15991 pg_txn_manager.cc:346] Check failed: !IsDdlMode() READ COMMITTED semantics don't apply to DDL transactions
```
This happens when we call `yb_increment_all_db_catalog_versions` to increment
catalog versions for all databases. This is done inside function
`YBDecrementDdlNestingLevel` which first does
```
--ddl_transaction_state.nesting_level;
```
and then if `ddl_transaction_state.nesting_level == 0` it calls
`yb_increment_all_db_catalog_versions` if the current DDL statement has global
impact. In `GetTransactionSnapshot` we have
```
if (IsYBReadCommitted() && YBGetDdlNestingLevel() == 0)
```
The function `yb_increment_all_db_catalog_versions` is still called as part of
the DDL statement, but `ddl_transaction_state.nesting_level` is already 0 so
`YBGetDdlNestingLevel()` returns 0. As a result the above `if` condition is true
and the DDL statement is treated as a non-DDL statement, eventually leading to
the assertion failure.
The DDL mode is already transferred to pggate when we call
`YBCPgEnterSeparateDdlTxnMode`. So I added `YBCPgIsDdlMode()` and uses it to
replace `YBGetDdlNestingLevel() == 0`.
I reverted some work arounds in `pg_catalog_version-test.cc` which used snapshot
isolation because otherwise those unit tests would fail under read committed
isolation.
Original commit: 417e2de / D30417
Test Plan: ./yb_build.sh debug --cxx-test pg_catalog_version-test
Reviewers: tvesely, pjain
Reviewed By: tvesely
Subscribers: yql
Tags: #jenkins-ready
Differential Revision: https://phorge.dev.yugabyte.com/D30840
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Jira Link: DB-8949
Description
There are many DDL related tests marked with this gh issue number which have been disabled to run with read committed isolation:
PgCatalogVersionTest.FixCatalogVersionTabledue to reason (1), see belowPgCatalogVersionTest.DBCatalogVersionGlobalDDLdue to reason (1)PgIndexBackfillTest.CreateIndexSimultaneouslydue to reason (2)PgPackedRowTest.AlterTable/*PgLibPqTest.ConcurrentInsertTruncateForeignKeydue to reason (3), only reproable in debug builds on devserverPgDDLConcurrencyTest.IndexCreationdue to reason (4), only reproable in debug builds on devserverPgDdlAtomicityParallelDdlTest.TestParallelDdldue to reason (4), only reproable in debug builds on devserverYBBackupTest.TestYSQLRestoreBackupToDBCatalogVersionModedue to reason (4)pg_ddl_atomicity-testdue to reason (5)Reasons for failure:
(1)
Fails withREAD COMMITTED semantics don't apply to DDL transactionsbecausePgTxnManager::ResetTransactionReadPointis called while performing catalog version increment in the final stage of a DDL.(2) Fails because DDL is retried in RC which is not expected by the test
(3)
Check failed: !options.ddl_mode() Restarting a DDL transaction not supported(4)
Check failed: !IsDdlMode() READ COMMITTED semantics don't apply to DDL transactions(5)
"Transaction for catalog table write operation 'pg_type' not found"Issue (1) can be solved by moving
--ddl_transaction_state.nesting_level;just beforeHandleYBStatus(YBCPgExitSeparateDdlTxnMode());inYBDecrementDdlNestingLevel(). This is done in pg_yb_utils.c in https://phorge.dev.yugabyte.com/D29278?vs=148742&id=152090#toc. However, this results in issue (5) cropping up in some of the other DDL tests.Issue Type
kind/bug
Warning: Please confirm that this issue does not contain any sensitive information
The text was updated successfully, but these errors were encountered: