Skip to content

Commit 390cce1

Browse files
committed
[yugabyte#29315] YSQL: Fix TestRollbackToSavepointWithDropCreateTableSameName in TSAN/ASAN
Summary: The test TestRollbackToSavepointWithDropCreateTableSameName has been quite flaky especially in slower builds such as TSAN and ASAN. This revision fixes the test. The issue happens when a table gets deleted as part of rollback to savepoint operation. When we have to drop a table in such scenarios, we don't wait for the table deletion to finish. Instead, we assume it'll be successful once the table has been marked DELETING. See the call to `RemoveDdlRollbackToSubTxnState` in the YsqlDdlTxnDropTableHelper function. This is safe to do because the deleted table can only be seen by the current transaction since it must have been created in the same txn block. In the test, we trigger the rollback to savepoint operation and immediately check for the deletion of the ysql_ddl_txn_verifier_state from the table. The verifier state is only deleted at the later stages of the table deletion i.e. after the tablet deletion. As a result, on slower builds, we end up making the assertion sooner than the deletion completion. Fixed the test by introducing a wait before we assert for the cleanup of verifier state. Test Plan: Jenkins: test regex: .*PgDdlSavepointMiniClusterTest.* ./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_transaction-test --gtest_filter bool/PgDdlSavepointMiniClusterTest.TestRollbackToSavepointWithDropCreateTableSameName/0 -n 10 ./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_transaction-test --gtest_filter bool/PgDdlSavepointMiniClusterTest.TestRollbackToSavepointWithDropCreateTableSameName/1 -n 10 Reviewers: pjain, myang Reviewed By: myang Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D48195
1 parent d9cfc51 commit 390cce1

File tree

1 file changed

+16
-0
lines changed

1 file changed

+16
-0
lines changed

src/yb/yql/pgwrapper/pg_ddl_transaction-test.cc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "yb/tablet/tablet.h"
2121
#include "yb/tablet/tablet_peer.h"
2222
#include "yb/util/async_util.h"
23+
#include "yb/util/backoff_waiter.h"
2324
#include "yb/yql/pgwrapper/libpq_test_base.h"
2425
#include "yb/yql/pgwrapper/libpq_utils.h"
2526
#include "yb/yql/pgwrapper/pg_mini_test_base.h"
@@ -309,6 +310,20 @@ class PgDdlSavepointMiniClusterTest : public PgMiniTestBase,
309310
}
310311
}
311312
}
313+
314+
Status WaitForTableDeletionToFinish(client::YBClient* client, const std::string& table_id) {
315+
return LoggedWaitFor([&]() -> Result<bool> {
316+
bool table_found = false;
317+
const auto tables = VERIFY_RESULT(client->ListTables());
318+
for (const auto& t : tables) {
319+
if (t.namespace_name() == "yugabyte" && t.table_id() == table_id) {
320+
table_found = true;
321+
break;
322+
}
323+
}
324+
return !table_found;
325+
}, MonoDelta::FromSeconds(60), "Wait for Table deletion to finish for table " + table_id);
326+
}
312327
};
313328

314329
INSTANTIATE_TEST_CASE_P(bool, PgDdlSavepointMiniClusterTest,
@@ -621,6 +636,7 @@ TEST_P(PgDdlSavepointMiniClusterTest, TestRollbackToSavepointWithDropCreateTable
621636
{.name = "e", .data_type = DataType::STRING},
622637
{.name = "f", .data_type = DataType::STRING}});
623638
ASSERT_FALSE(table->LockForRead()->has_ysql_ddl_txn_verifier_state());
639+
ASSERT_OK(WaitForTableDeletionToFinish(client.get(), table_id_after_drop));
624640
ASSERT_FALSE(table_after_drop->LockForRead()->has_ysql_ddl_txn_verifier_state());
625641

626642
if (GetParam()) {

0 commit comments

Comments
 (0)