diff --git a/src/yb/master/ysql_ddl_handler.cc b/src/yb/master/ysql_ddl_handler.cc index bdcb4519d9d2..4120485e9e9a 100644 --- a/src/yb/master/ysql_ddl_handler.cc +++ b/src/yb/master/ysql_ddl_handler.cc @@ -198,16 +198,30 @@ Status CatalogManager::YsqlDdlTxnCompleteCallback(const string& pb_txn_id, } auto state = verifier_state->state; + auto txn_state = is_committed ? TxnState::kCommitted : TxnState::kAborted; if (state == YsqlDdlVerificationState::kDdlPostProcessing) { - // Verification is already in progress. - VLOG(3) << "Transaction " << txn << " is already being verified, ignoring"; - return Status::OK(); + // We used to return Status::OK() here on the grounds that the txn is + // already being verified and we assumed verifier_state->tables represent + // all of the tables involved in txn and they are taken care of by + // calling YsqlDdlTxnCompleteCallbackInternal on each of them below. + // However, verifier_state->tables may not include all of the tables + // involved in txn. It is possible that a table is only added into the + // txn after the txn is already in kDdlPostProcessing state. For example, + // a txn involves three tables t1, t2, t3. After t1 and t2 are added to txn, + // the txn is aborted due to some reason (e.g., conflict). In this case + // YsqlDdlTxnCompleteCallbackInternal is only called on t1 and t2 and + // the state is set to kDdlPostProcessing before t3 gets added. Later when + // t3 gets added, if we return Status::OK() here, then t3 will never be + // processed. Therefore we need to call YsqlDdlTxnCompleteCallbackInternal + // to process t3. It is fine to reprocess t1 and t2, that will result in a + // no-op. + SCHECK_EQ(txn_state, verifier_state->txn_state, IllegalState, + Format("Mismatch in txn_state for transaction $0", txn)); + } else { + verifier_state->txn_state = txn_state; + verifier_state->state = YsqlDdlVerificationState::kDdlPostProcessing; } - tables = verifier_state->tables; - verifier_state->txn_state = - (is_committed) ? TxnState::kCommitted : TxnState::kAborted; - verifier_state->state = YsqlDdlVerificationState::kDdlPostProcessing; } bool ddl_verification_success = true; diff --git a/src/yb/yql/pgwrapper/pg_ddl_atomicity_stress-test.cc b/src/yb/yql/pgwrapper/pg_ddl_atomicity_stress-test.cc index 1e457b0421c4..edabeda9a5fc 100644 --- a/src/yb/yql/pgwrapper/pg_ddl_atomicity_stress-test.cc +++ b/src/yb/yql/pgwrapper/pg_ddl_atomicity_stress-test.cc @@ -366,5 +366,50 @@ TEST_F(PgDdlAtomicityColocatedStressTest, TestFailDdlVerification) { ASSERT_OK(StressTestWithFlag("TEST_ysql_ddl_verification_failure_probability")); } +/* + * Tests on Partitioned Tables. +*/ +class PgDdlAtomicityPartitionedTablesStressTest : public PgDdlAtomicityStressTest { + virtual int NumIterations() override { + // Fewer iterations are sufficient for partitioned table tests because each DDL statement + // internally invokes (num_partitions + 1) DDLs. + return 3; + } + virtual Status SetupTables() override; +}; + +Status PgDdlAtomicityPartitionedTablesStressTest::SetupTables() { + auto conn = VERIFY_RESULT(Connect()); + RETURN_NOT_OK(conn.ExecuteFormat("CREATE TABLE $0 (key INT PRIMARY KEY, value TEXT, num real) " + " PARTITION BY RANGE(key)", kTable)); + RETURN_NOT_OK(conn.ExecuteFormat( + "CREATE TABLE $0_$1 PARTITION OF $0 FOR VALUES FROM ($1) TO ($2)", + kTable, 1, NumIterations())); + // Create a default partition. + RETURN_NOT_OK(conn.ExecuteFormat("CREATE TABLE $0_default PARTITION OF $0 DEFAULT", kTable)); + return Status::OK(); +} + +TEST_F(PgDdlAtomicityPartitionedTablesStressTest, BasicTest) { + ASSERT_OK(StressTestWithFlag("")); +} + +TEST_F(PgDdlAtomicityPartitionedTablesStressTest, TestTxnVerificationFailure) { + ASSERT_OK(StressTestWithFlag("TEST_ysql_ddl_transaction_verification_failure_probability")); +} + +TEST_F(PgDdlAtomicityPartitionedTablesStressTest, TestFailCatalogWrites) { + ASSERT_OK(StressTestWithFlag( + "TEST_ysql_fail_probability_of_catalog_writes_by_ddl_verification")); +} + +TEST_F(PgDdlAtomicityPartitionedTablesStressTest, TestFailDdlRollback) { + ASSERT_OK(StressTestWithFlag("TEST_ysql_ddl_rollback_failure_probability")); +} + +TEST_F(PgDdlAtomicityPartitionedTablesStressTest, TestFailDdlVerification) { + ASSERT_OK(StressTestWithFlag("TEST_ysql_ddl_verification_failure_probability")); +} + } // namespace pgwrapper } // namespace yb