From e8655932212724c19112e5ee065f8a9c24d40891 Mon Sep 17 00:00:00 2001 From: Minghui Yang Date: Fri, 22 Mar 2024 03:20:56 +0000 Subject: [PATCH] [BACKPORT 2024.1][#22097] YSQL: Enable DDL atomicity by default Summary: This diff enables DDL atomicity feature by default. (1) changing the default value of several gflags from false to true. --ysql_yb_ddl_rollback_enabled --report_ysql_ddl_txn_status_to_master --ysql_ddl_transaction_wait_for_ddl_verification (2) code cleanup related to (1), for example, some unit tests needed to explicitly enable one or more of these 3 gflags. Now that they are turned on by default, those code are removed. (3) other unit tests update related to (1). For example, in pg_packed_row-test.cc, the test output has changed from `PACKED_ROW[2]` to `PACKED_ROW[3]`. The number in the bracket represents the table schema version. With DDL atomicity, a DDL such as `ALTER TABLE test DROP COLUMN v2` causes the schema version of table test to bump by 2 after the DDL commits. Without DDL atomicity, the schema version of table test used to only bump up by 1 after the DDL commits. Jira: DB-11028 Original commit: 1bbe61925c8cd929767a7290396fc38184d05974 / D30471 Test Plan: Jenkins run Reviewers: hsunder Reviewed By: hsunder Subscribers: hsunder, ycdcxcluster Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D34636 --- .../org/yb/pgsql/TestPgDdlFaultTolerance.java | 30 +++++++++++-------- .../org/yb/pgsql/TestPgUniqueConstraint.java | 2 -- src/postgres/src/backend/utils/misc/guc.c | 2 +- .../expected/yb_alter_table_rewrite.out | 2 -- .../regress/sql/yb_alter_table_rewrite.sql | 2 -- src/yb/cdc/cdcsdk_producer.cc | 2 -- src/yb/client/client-internal.cc | 1 - src/yb/client/table_alterer.cc | 2 -- src/yb/client/table_creator.cc | 1 - src/yb/common/common_flags.cc | 2 +- src/yb/integration-tests/cdcsdk_test_base.cc | 4 --- src/yb/integration-tests/cdcsdk_test_base.h | 1 - src/yb/master/catalog_loaders.cc | 2 -- .../yb-backup/yb-backup-cross-feature-test.cc | 8 ----- src/yb/tserver/pg_client_session.cc | 5 ++-- src/yb/yql/pggate/ybc_pggate.cc | 2 -- src/yb/yql/pgwrapper/pg_cache_refresh-test.cc | 16 +++++----- src/yb/yql/pgwrapper/pg_ddl_atomicity-test.cc | 29 +++++++----------- src/yb/yql/pgwrapper/pg_drop_column_test.cc | 10 ------- src/yb/yql/pgwrapper/pg_libpq-test.cc | 5 ++-- src/yb/yql/pgwrapper/pg_packed_row-test.cc | 6 +++- 21 files changed, 48 insertions(+), 86 deletions(-) diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgDdlFaultTolerance.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgDdlFaultTolerance.java index 7b440bc74cab..685038b0640b 100644 --- a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgDdlFaultTolerance.java +++ b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgDdlFaultTolerance.java @@ -156,7 +156,7 @@ public void testDocDBMetadataFailuresForTable() throws Exception { // Alter should fail because DocDB cannot add the column. runInvalidQuery(statement, alterStmt, "Injected random failure for testing"); - // Enable syscatalog again. + // Enable syscatalog writes again. setDocDBSysCatalogWriteRejection(masterLeaderAddress, 0); // Alter should now succeed. @@ -175,18 +175,19 @@ public void testDocDBMetadataFailuresForTable() throws Exception { setDocDBSysCatalogWriteRejection(masterLeaderAddress, 100); String dropStmt = "DROP TABLE ddl_ft_table"; - // Drop the table -- should succeed with warning. - verifyStatementWarning(statement, dropStmt, "Injected random failure for testing"); + // Drop table with error injection. + runInvalidQuery(statement, dropStmt, "Injected random failure for testing"); - // Table should not be usable after drop. - runInvalidQuery(statement, String.format(insertSql, 7, 8, 9), - "relation \"ddl_ft_table\" does not exist"); - - runInvalidQuery(statement, selectStmt, "relation \"ddl_ft_table\" does not exist"); + // Table should still be usable. + statement.execute("INSERT INTO ddl_ft_table VALUES (11, '12', 13.0, 14)"); + expectedRows.add(new Row(11, "12", 13.0, 14)); + assertRowSet(statement, selectStmt, expectedRows); - // We should be able to create a table with the same name without issues. + // Enable syscatalog writes again. setDocDBSysCatalogWriteRejection(masterLeaderAddress, 0); + // Drop table should now succeed. + statement.execute(dropStmt); // Create table should now succeed. statement.execute(createStmt); @@ -264,18 +265,21 @@ public void testDocDBMetadataFailuresForIndex() throws Exception { // Disable syscatalog writes. setDocDBSysCatalogWriteRejection(masterLeaderAddress, 100); - // Drop index should succeed but with a warning. - verifyStatementWarning(statement, "DROP INDEX ddl_ft_indexed_table_idx", - "Injected random failure for testing"); + // Drop index should fail. + String dropStmt = "DROP INDEX ddl_ft_indexed_table_idx"; + runInvalidQuery(statement, dropStmt, "Injected random failure for testing"); - // Table should only have pkey index. + // Table should still have primary index and ddl_ft_indexed_table_idx expectedIdxs.clear(); expectedIdxs.add(new Row("ddl_ft_indexed_table_pkey")); + expectedIdxs.add(new Row("ddl_ft_indexed_table_idx")); assertRowSet(statement, pgClassQuery, expectedIdxs); // Enable syscatalog writes. setDocDBSysCatalogWriteRejection(masterLeaderAddress, 0); + // Drop index should succeed. + statement.execute(dropStmt); // We should be able to create a new index with the same name. statement.execute(createIndex); diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgUniqueConstraint.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgUniqueConstraint.java index f7c6fd80a57f..9ef1f19774de 100644 --- a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgUniqueConstraint.java +++ b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgUniqueConstraint.java @@ -404,8 +404,6 @@ public void createIndexViolatingUniqueness() throws Exception { for (HostAndPort hp : miniCluster.getTabletServers().keySet()) { assertTrue(miniCluster.getClient().setFlag(hp, "allowed_preview_flags_csv", "ysql_yb_ddl_rollback_enabled=true")); - assertTrue(miniCluster.getClient().setFlag(hp, - "ysql_yb_ddl_rollback_enabled", "false")); } try (Statement stmt = connection.createStatement()) { runInvalidQuery( diff --git a/src/postgres/src/backend/utils/misc/guc.c b/src/postgres/src/backend/utils/misc/guc.c index 4d5aa3991b8d..93c6d3543165 100644 --- a/src/postgres/src/backend/utils/misc/guc.c +++ b/src/postgres/src/backend/utils/misc/guc.c @@ -2488,7 +2488,7 @@ static struct config_bool ConfigureNamesBool[] = GUC_NOT_IN_SAMPLE }, &yb_ddl_rollback_enabled, - false, + true, NULL, NULL, NULL }, diff --git a/src/postgres/src/test/regress/expected/yb_alter_table_rewrite.out b/src/postgres/src/test/regress/expected/yb_alter_table_rewrite.out index ac2c77967ad0..b99e878ece2c 100644 --- a/src/postgres/src/test/regress/expected/yb_alter_table_rewrite.out +++ b/src/postgres/src/test/regress/expected/yb_alter_table_rewrite.out @@ -435,9 +435,7 @@ CREATE INDEX test_add_column_idx ON test_add_column(b ASC) SPLIT AT VALUES ((5), (10)); INSERT INTO test_add_column VALUES (1, 1); ALTER TABLE test_add_column ADD COLUMN c SERIAL; -SET yb_ddl_rollback_enabled = ON; ALTER TABLE test_add_column ADD COLUMN d SERIAL PRIMARY KEY; -SET yb_ddl_rollback_enabled = OFF; SELECT num_tablets, num_hash_key_columns FROM yb_table_properties('test_add_column'::regclass); num_tablets | num_hash_key_columns diff --git a/src/postgres/src/test/regress/sql/yb_alter_table_rewrite.sql b/src/postgres/src/test/regress/sql/yb_alter_table_rewrite.sql index 87cfeccd96f5..d2564d5cf11c 100644 --- a/src/postgres/src/test/regress/sql/yb_alter_table_rewrite.sql +++ b/src/postgres/src/test/regress/sql/yb_alter_table_rewrite.sql @@ -185,9 +185,7 @@ CREATE INDEX test_add_column_idx ON test_add_column(b ASC) SPLIT AT VALUES ((5), (10)); INSERT INTO test_add_column VALUES (1, 1); ALTER TABLE test_add_column ADD COLUMN c SERIAL; -SET yb_ddl_rollback_enabled = ON; ALTER TABLE test_add_column ADD COLUMN d SERIAL PRIMARY KEY; -SET yb_ddl_rollback_enabled = OFF; SELECT num_tablets, num_hash_key_columns FROM yb_table_properties('test_add_column'::regclass); SELECT yb_get_range_split_clause('test_add_column_idx'::regclass); diff --git a/src/yb/cdc/cdcsdk_producer.cc b/src/yb/cdc/cdcsdk_producer.cc index 6f809421eba8..383123234029 100644 --- a/src/yb/cdc/cdcsdk_producer.cc +++ b/src/yb/cdc/cdcsdk_producer.cc @@ -92,8 +92,6 @@ DEFINE_NON_RUNTIME_int64( DECLARE_bool(ysql_enable_packed_row); DECLARE_bool(ysql_yb_enable_replica_identity); -DECLARE_bool(ysql_yb_ddl_rollback_enabled); - namespace yb { namespace cdc { diff --git a/src/yb/client/client-internal.cc b/src/yb/client/client-internal.cc index 4bce558a6c37..d16d32ba7a21 100644 --- a/src/yb/client/client-internal.cc +++ b/src/yb/client/client-internal.cc @@ -119,7 +119,6 @@ DEFINE_RUNTIME_uint32(change_metadata_backoff_init_exponent, 1, DECLARE_int64(reset_master_leader_timeout_ms); DECLARE_string(flagfile); -DECLARE_bool(ysql_yb_ddl_rollback_enabled); namespace yb { diff --git a/src/yb/client/table_alterer.cc b/src/yb/client/table_alterer.cc index 278eaf9c725a..5c05c8fe1831 100644 --- a/src/yb/client/table_alterer.cc +++ b/src/yb/client/table_alterer.cc @@ -25,8 +25,6 @@ using std::string; -DECLARE_bool(ysql_yb_ddl_rollback_enabled); - namespace yb { namespace client { diff --git a/src/yb/client/table_creator.cc b/src/yb/client/table_creator.cc index e96b6d973502..86ca4d703567 100644 --- a/src/yb/client/table_creator.cc +++ b/src/yb/client/table_creator.cc @@ -36,7 +36,6 @@ using std::string; DECLARE_bool(client_suppress_created_logs); DECLARE_uint32(change_metadata_backoff_max_jitter_ms); DECLARE_uint32(change_metadata_backoff_init_exponent); -DECLARE_bool(ysql_yb_ddl_rollback_enabled); DEFINE_test_flag(bool, duplicate_create_table_request, false, "Whether a table creator should send duplicate CreateTableRequestPB to master."); diff --git a/src/yb/common/common_flags.cc b/src/yb/common/common_flags.cc index f1d864ecf6ba..d4ab5de1f7bb 100644 --- a/src/yb/common/common_flags.cc +++ b/src/yb/common/common_flags.cc @@ -59,7 +59,7 @@ DEFINE_NON_RUNTIME_bool(disable_deadlock_detection, false, TAG_FLAG(disable_deadlock_detection, advanced); TAG_FLAG(disable_deadlock_detection, hidden); -DEFINE_RUNTIME_PG_PREVIEW_FLAG(bool, yb_ddl_rollback_enabled, false, +DEFINE_RUNTIME_PG_FLAG(bool, yb_ddl_rollback_enabled, true, "If true, upon failure of a YSQL DDL transaction that affects the DocDB syscatalog, the " "YB-Master will rollback the changes made to the DocDB syscatalog."); diff --git a/src/yb/integration-tests/cdcsdk_test_base.cc b/src/yb/integration-tests/cdcsdk_test_base.cc index ce302d6d7aba..8a27e59a7f06 100644 --- a/src/yb/integration-tests/cdcsdk_test_base.cc +++ b/src/yb/integration-tests/cdcsdk_test_base.cc @@ -55,7 +55,6 @@ using std::string; DECLARE_bool(ysql_enable_pack_full_row_update); -DECLARE_bool(ysql_ddl_transaction_wait_for_ddl_verification); namespace yb { using client::YBClient; @@ -141,9 +140,6 @@ Status CDCSDKTestBase::SetUpWithParams( // Set max_replication_slots to a large value so that we don't run out of them during tests and // don't have to do cleanups after every test case. ANNOTATE_UNPROTECTED_WRITE(FLAGS_max_replication_slots) = 500; - ANNOTATE_UNPROTECTED_WRITE(FLAGS_allowed_preview_flags_csv) = "ysql_yb_ddl_rollback_enabled"; - ANNOTATE_UNPROTECTED_WRITE(FLAGS_ysql_yb_ddl_rollback_enabled) = true; - ANNOTATE_UNPROTECTED_WRITE(FLAGS_ysql_ddl_transaction_wait_for_ddl_verification) = true; MiniClusterOptions opts; opts.num_masters = num_masters; diff --git a/src/yb/integration-tests/cdcsdk_test_base.h b/src/yb/integration-tests/cdcsdk_test_base.h index e381adefb0bd..b76a7c8da653 100644 --- a/src/yb/integration-tests/cdcsdk_test_base.h +++ b/src/yb/integration-tests/cdcsdk_test_base.h @@ -48,7 +48,6 @@ DECLARE_uint32(cdcsdk_retention_barrier_no_revision_interval_secs); DECLARE_int32(cleanup_split_tablets_interval_sec); DECLARE_string(allowed_preview_flags_csv); DECLARE_bool(ysql_yb_enable_ddl_atomicity_infra); -DECLARE_bool(ysql_yb_ddl_rollback_enabled); DECLARE_bool(ysql_enable_pack_full_row_update); DECLARE_bool(ysql_yb_enable_replica_identity); DECLARE_bool(ysql_enable_packed_row_for_colocated_table); diff --git a/src/yb/master/catalog_loaders.cc b/src/yb/master/catalog_loaders.cc index ee236bb43e06..a5fc389a23fb 100644 --- a/src/yb/master/catalog_loaders.cc +++ b/src/yb/master/catalog_loaders.cc @@ -52,8 +52,6 @@ DEFINE_UNKNOWN_bool(master_ignore_deleted_on_load, true, DEFINE_test_flag(uint64, slow_cluster_config_load_secs, 0, "When set, it pauses load of cluster config during sys catalog load."); -DECLARE_bool(ysql_yb_ddl_rollback_enabled); - namespace yb { namespace master { diff --git a/src/yb/tools/yb-backup/yb-backup-cross-feature-test.cc b/src/yb/tools/yb-backup/yb-backup-cross-feature-test.cc index 00de24c8ed39..65763867b17e 100644 --- a/src/yb/tools/yb-backup/yb-backup-cross-feature-test.cc +++ b/src/yb/tools/yb-backup/yb-backup-cross-feature-test.cc @@ -1637,14 +1637,6 @@ INSTANTIATE_TEST_CASE_P( std::make_tuple(PackedRowsEnabled::kTrue, SourceDatabaseIsColocated::kFalse))); class YBDdlAtomicityBackupTest : public YBBackupTestBase, public pgwrapper::PgDdlAtomicityTestBase { - void UpdateMiniClusterOptions(ExternalMiniClusterOptions* options) override { - LibPqTestBase::UpdateMiniClusterOptions(options); - options->extra_tserver_flags.push_back( - "--allowed_preview_flags_csv=ysql_yb_ddl_rollback_enabled"); - options->extra_tserver_flags.push_back("--ysql_yb_ddl_rollback_enabled=true"); - options->extra_tserver_flags.push_back("--report_ysql_ddl_txn_status_to_master=true"); - } - public: Status RunDdlAtomicityTest(pgwrapper::DdlErrorInjection inject_error); }; diff --git a/src/yb/tserver/pg_client_session.cc b/src/yb/tserver/pg_client_session.cc index 9aad17f9fdaf..c3c154dc78d3 100644 --- a/src/yb/tserver/pg_client_session.cc +++ b/src/yb/tserver/pg_client_session.cc @@ -66,7 +66,7 @@ using std::string; using namespace std::chrono_literals; -DEFINE_RUNTIME_bool(report_ysql_ddl_txn_status_to_master, false, +DEFINE_RUNTIME_bool(report_ysql_ddl_txn_status_to_master, true, "If set, at the end of DDL operation, the TServer will notify the YB-Master " "whether the DDL operation was committed or aborted"); @@ -81,12 +81,11 @@ DEFINE_RUNTIME_string(ysql_sequence_cache_method, "connection", "Where sequence values are cached for both existing and new sequences. Valid values are " "\"connection\" and \"server\""); -DEFINE_RUNTIME_bool(ysql_ddl_transaction_wait_for_ddl_verification, false, +DEFINE_RUNTIME_bool(ysql_ddl_transaction_wait_for_ddl_verification, true, "If set, DDL transactions will wait for DDL verification to complete before " "returning to the client. "); DECLARE_bool(ysql_serializable_isolation_for_ddl_txn); -DECLARE_bool(ysql_yb_ddl_rollback_enabled); DECLARE_bool(ysql_yb_enable_ddl_atomicity_infra); DECLARE_bool(yb_enable_cdc_consistent_snapshot_streams); diff --git a/src/yb/yql/pggate/ybc_pggate.cc b/src/yb/yql/pggate/ybc_pggate.cc index 054c9e845535..44fed259ebd1 100644 --- a/src/yb/yql/pggate/ybc_pggate.cc +++ b/src/yb/yql/pggate/ybc_pggate.cc @@ -77,8 +77,6 @@ DECLARE_int32(delay_alter_sequence_sec); DECLARE_int32(client_read_write_timeout_ms); -DECLARE_bool(ysql_yb_ddl_rollback_enabled); - DEFINE_UNKNOWN_bool(ysql_enable_reindex, false, "Enable REINDEX INDEX statement."); TAG_FLAG(ysql_enable_reindex, advanced); diff --git a/src/yb/yql/pgwrapper/pg_cache_refresh-test.cc b/src/yb/yql/pgwrapper/pg_cache_refresh-test.cc index 4ba2f41ba2d6..faeb1cdb9778 100644 --- a/src/yb/yql/pgwrapper/pg_cache_refresh-test.cc +++ b/src/yb/yql/pgwrapper/pg_cache_refresh-test.cc @@ -172,8 +172,11 @@ TEST_F(PgCacheRefreshTest, NewConnectionTransparentRetry) { // In a different connection to a different node, run a DDL statement. testConcurrentDDLFromDifferentNode("col2"); - // TODO: Test commented out due to #14327 - // ASSERT_OK(executeInsert()); + + // Wait for heartbeat to propagate across all the TServers and invalidate the table cache + // across all nodes. + SleepFor(MonoDelta::FromMilliseconds(2 * FLAGS_heartbeat_interval_ms)); + ASSERT_OK(executeInsert()); // Cause schema version increment in a different connection to the same TServer. testConcurrentSchemaVersionIncrement("col3"); @@ -235,12 +238,9 @@ TEST_F(PgCacheRefreshTest, NewConnectionTransparentRetryTxn) { // across all nodes. SleepFor(MonoDelta::FromMilliseconds(2 * FLAGS_heartbeat_interval_ms)); - // Test DML transaction interleaved with an operation that only causes schema version mismatch - // through a connection to the same TServer. Here the alter operation causes table cache - // invalidation on the TServer, therefore no retry is needed. - testSuccessfulTxnSchemaVersionMismatch([this] { - testConcurrentSchemaVersionIncrement("col3"); - }); + // Cause a schema version increment operation through the same TServer so that the table schema + // gets cached. + testConcurrentSchemaVersionIncrement("col3"); // Test DML transaction interleaved with an operation that only causes schema version mismatch // through a connection to a different TServer. diff --git a/src/yb/yql/pgwrapper/pg_ddl_atomicity-test.cc b/src/yb/yql/pgwrapper/pg_ddl_atomicity-test.cc index df8ae097b51d..cd84d3e17a1d 100644 --- a/src/yb/yql/pgwrapper/pg_ddl_atomicity-test.cc +++ b/src/yb/yql/pgwrapper/pg_ddl_atomicity-test.cc @@ -57,9 +57,6 @@ using namespace std::literals; DECLARE_bool(TEST_hang_on_ddl_verification_progress); DECLARE_string(allowed_preview_flags_csv); -DECLARE_bool(ysql_yb_ddl_rollback_enabled); -DECLARE_bool(report_ysql_ddl_txn_status_to_master); -DECLARE_bool(ysql_ddl_transaction_wait_for_ddl_verification); DECLARE_bool(TEST_ysql_disable_transparent_cache_refresh_retry); namespace yb { @@ -156,6 +153,17 @@ TEST_F(PgDdlAtomicityTest, TestIndexTableGC) { ASSERT_OK(conn.Execute(CreateTableStmt(test_name))); // After successfully creating the first table, set flags to delay the background task. + // But do not let PG wait for the ddl verification to complete otherwise it will defeat the + // purpopse of the following 13000ms delay to start the ddl verification task: TestFailDdl + // does not finish until 13000ms have passed and the ddl verification has completed. On + // ddl verification completion master rolls back the index table and the next VerifyTableExists + // fails to see the table. Do not set report_ysql_ddl_txn_status_to_master for similar + // reason: on receiving PG reported status without polling master rolls back the index + // table. + ASSERT_OK(cluster_->SetFlagOnTServers( + "ysql_ddl_transaction_wait_for_ddl_verification", "false")); + ASSERT_OK(cluster_->SetFlagOnTServers( + "report_ysql_ddl_txn_status_to_master", "false")); ASSERT_OK(cluster_->SetFlagOnMasters("ysql_transaction_bg_task_wait_ms", "13000")); ASSERT_OK(conn.TestFailDdl(CreateIndexStmt(test_name_idx, test_name, "key"))); @@ -268,11 +276,6 @@ TEST_F(PgDdlAtomicityTest, FailureRecoveryTestWithAbortedTxn) { class PgDdlAtomicitySanityTest : public PgDdlAtomicityTest { protected: void UpdateMiniClusterOptions(ExternalMiniClusterOptions* options) override { - options->extra_tserver_flags.push_back( - "--allowed_preview_flags_csv=ysql_yb_ddl_rollback_enabled"); - options->extra_tserver_flags.push_back("--ysql_yb_ddl_rollback_enabled=true"); - options->extra_tserver_flags.push_back("--report_ysql_ddl_txn_status_to_master=true"); - options->extra_tserver_flags.push_back("--ysql_ddl_transaction_wait_for_ddl_verification=true"); // TODO (#19975): Enable read committed isolation options->extra_tserver_flags.push_back("--yb_enable_read_committed_isolation=false"); options->extra_master_flags.push_back("--vmodule=ysql_ddl_handler=5,ysql_transaction_ddl=5"); @@ -1381,11 +1384,7 @@ TEST_F(PgDdlAtomicitySnapshotTest, DdlRollbackListSnapshotTest) { class PgLibPqMatviewTest: public PgDdlAtomicitySanityTest { public: void UpdateMiniClusterOptions(ExternalMiniClusterOptions* options) override { - options->extra_tserver_flags.push_back( - "--allowed_preview_flags_csv=ysql_yb_ddl_rollback_enabled"); - options->extra_tserver_flags.push_back("--ysql_yb_ddl_rollback_enabled=true"); options->extra_master_flags.push_back("--vmodule=ysql_ddl_handler=3,ysql_transaction_ddl=3"); - options->extra_tserver_flags.push_back("--ysql_ddl_transaction_wait_for_ddl_verification=true"); } protected: void MatviewTest(); @@ -1469,8 +1468,6 @@ class PgLibPqTableRewrite: public: void UpdateMiniClusterOptions(ExternalMiniClusterOptions* options) override { PgDdlAtomicitySanityTest::UpdateMiniClusterOptions(options); - options->extra_tserver_flags.push_back( - "--allowed_preview_flags_csv=ysql_yb_ddl_rollback_enabled"); options->extra_tserver_flags.push_back("--ysql_enable_reindex=true"); if (!GetParam()) { options->extra_master_flags.push_back("--ysql_transaction_bg_task_wait_ms=10000"); @@ -1606,10 +1603,6 @@ TEST_P(PgLibPqTableRewrite, class PgDdlAtomicityMiniClusterTest : public PgMiniTestBase { protected: void SetUp() override { - ANNOTATE_UNPROTECTED_WRITE(FLAGS_allowed_preview_flags_csv) = "ysql_yb_ddl_rollback_enabled"; - ANNOTATE_UNPROTECTED_WRITE(FLAGS_ysql_yb_ddl_rollback_enabled) = true; - ANNOTATE_UNPROTECTED_WRITE(FLAGS_report_ysql_ddl_txn_status_to_master) = true; - ANNOTATE_UNPROTECTED_WRITE(FLAGS_ysql_ddl_transaction_wait_for_ddl_verification) = true; ANNOTATE_UNPROTECTED_WRITE(FLAGS_TEST_ysql_disable_transparent_cache_refresh_retry) = true; pgwrapper::PgMiniTestBase::SetUp(); } diff --git a/src/yb/yql/pgwrapper/pg_drop_column_test.cc b/src/yb/yql/pgwrapper/pg_drop_column_test.cc index f4afdb9f21db..16ea2cf23bfa 100644 --- a/src/yb/yql/pgwrapper/pg_drop_column_test.cc +++ b/src/yb/yql/pgwrapper/pg_drop_column_test.cc @@ -14,22 +14,12 @@ using std::string; -DECLARE_bool(ysql_yb_ddl_rollback_enabled); - namespace yb { namespace pgwrapper { const std::string table = "testtable"; class PgDropColumnSanityTest : public LibPqTestBase { - void UpdateMiniClusterOptions(ExternalMiniClusterOptions* options) override { - options->extra_tserver_flags.push_back( - "--allowed_preview_flags_csv=ysql_yb_ddl_rollback_enabled"); - options->extra_tserver_flags.push_back("--ysql_yb_ddl_rollback_enabled=true"); - options->extra_tserver_flags.push_back("--report_ysql_ddl_txn_status_to_master=true"); - options->extra_tserver_flags.push_back("--ysql_ddl_transaction_wait_for_ddl_verification=true"); - } - public: void TestMarkColForDeletion(); diff --git a/src/yb/yql/pgwrapper/pg_libpq-test.cc b/src/yb/yql/pgwrapper/pg_libpq-test.cc index aa5eaa2c261d..a60e97aeae15 100644 --- a/src/yb/yql/pgwrapper/pg_libpq-test.cc +++ b/src/yb/yql/pgwrapper/pg_libpq-test.cc @@ -2265,7 +2265,7 @@ TEST_F_EX( TablegroupId next_tg_id = GetPgsqlTablegroupId(database_oid, next_tg_oid); // Force CREATE TABLEGROUP to fail, and delay the cleanup. - ASSERT_OK(cluster_->SetFlagOnMasters("ysql_transaction_bg_task_wait_ms", "3000")); + ASSERT_OK(cluster_->SetFlagOnMasters("TEST_pause_ddl_rollback", "true")); ASSERT_OK(conn.TestFailDdl("CREATE TABLEGROUP tg2")); // Forcing PG to reuse tablegroup OID. @@ -2274,7 +2274,8 @@ TEST_F_EX( // Cleanup hasn't been processed yet, so this fails. ASSERT_NOK_STR_CONTAINS(conn.Execute("CREATE TABLEGROUP tg3"), "Duplicate tablegroup"); - // Wait for cleanup thread to delete a table. + // Wait for cleanup thread to delete the table. + ASSERT_OK(cluster_->SetFlagOnMasters("TEST_pause_ddl_rollback", "false")); // Since delete hasn't started initially, WaitForDeleteTableToFinish will error out. const auto tg_parent_table_id = GetTablegroupParentTableId(next_tg_id); ASSERT_OK(WaitFor( diff --git a/src/yb/yql/pgwrapper/pg_packed_row-test.cc b/src/yb/yql/pgwrapper/pg_packed_row-test.cc index f209dfdf2d4f..b878a77672d6 100644 --- a/src/yb/yql/pgwrapper/pg_packed_row-test.cc +++ b/src/yb/yql/pgwrapper/pg_packed_row-test.cc @@ -736,6 +736,10 @@ TEST_P(PgPackedRowTest, AddDropColumn) { conn = ASSERT_RESULT(Connect()); continue; } + if (status.ToString().find("marked for deletion") != std::string::npos) { + // This is an expected error if writes are performed while columns are being dropped. + continue; + } ASSERT_OK(status); } } @@ -992,7 +996,7 @@ TEST_P(PgPackedRowTest, SstDumpNoMetadata) { ASSERT_STR_EQ_VERBOSE_TRIMMED(util::ApplyEagerLineContinuation( R"#( SubDocKey(DocKey(0x1210, [1], []), [HT{}]) -> PACKED_ROW[0](04000000536F6E65) - SubDocKey(DocKey(0x9eaf, [4], []), [HT{}]) -> PACKED_ROW[2](080000005363686574797265) + SubDocKey(DocKey(0x9eaf, [4], []), [HT{}]) -> PACKED_ROW[3](080000005363686574797265) SubDocKey(DocKey(0xc0c4, [2], []), [HT{}]) -> PACKED_ROW[0](040000005374776F) SubDocKey(DocKey(0xfca0, [3], []), [HT{}]) -> \ PACKED_ROW[1](060000000A00000053746872656553747269)