From af8f3ae17695504c965a1b211bf5f1785bad3687 Mon Sep 17 00:00:00 2001 From: Andrei Martsinchyk Date: Mon, 11 Nov 2024 10:47:43 -0800 Subject: [PATCH] [#24313] YSQL: test tidy up Summary: Follow up on 68f7de82addd7eaed4b30db0b1c5d8c1dc590dc0 / D39430 to address outstanding review comments. Jira: DB-13202 Test Plan: ./yb_build.sh --cxx-test pgwrapper_pg_index_backfill-test --gtest_filter PgIndexBackfillTest.PhantomIdxEntry Reviewers: jason Reviewed By: jason Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D39810 --- .../yql/pgwrapper/pg_index_backfill-test.cc | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/yb/yql/pgwrapper/pg_index_backfill-test.cc b/src/yb/yql/pgwrapper/pg_index_backfill-test.cc index 328e10d5256..813c9758b86 100644 --- a/src/yb/yql/pgwrapper/pg_index_backfill-test.cc +++ b/src/yb/yql/pgwrapper/pg_index_backfill-test.cc @@ -2300,8 +2300,23 @@ class PgIndexBackfillReadCommitted : public PgIndexBackfillTest { // Test for https://github.com/yugabyte/yugabyte-db/issues/24313 // Verify that concurrent updates do not leave phantom entries in the index +// Phantom entries were created because inplace index update function did not check if the index +// was ready. The index build procedure wait until "old" transactions (those who don't know about +// the index) are completed before proceeding to "ready" state because update by old transaction may +// prevent proper index update. This happens as follows ("new" transaction thinks the index is +// ready, "old" transaction thinks the index does not exist): +// - "new" transaction changes value in indexed column of record R from 'a' to 'b'. +// Index key is changed from 'a' to 'b'. Index is being built, the 'a' key may not exist yet, in +// this case new key is created. +// - "old" transaction changes value in indexed column of record R from 'b' to 'c'. +// Index key is not updated, it remains 'b'. +// - "new" transaction changes value in indexed column of record R from 'c' to 'd'. +// The transaction attempts to change the index key from 'c' to 'd'. The key 'c' does not exist +// in the index, so new key is created. Now index has two keys, 'b' and 'd' pointing to the same +// record R. +// If index readiness is not checked, any transaction aware of the index acts as "new". TEST_F_EX(PgIndexBackfillTest, PhantomIdxEntry, PgIndexBackfillReadCommitted) { - constexpr int64_t kNumRows = 10; + constexpr int kNumRows = 10; const IndexStateFlags index_live_flags{IndexStateFlag::kIndIsLive}; ASSERT_OK(conn_->ExecuteFormat("CREATE TABLE $0 (i int, t text)", kTableName)); ASSERT_OK(conn_->ExecuteFormat("INSERT INTO $0 VALUES (generate_series(1, $1), 'a')", @@ -2315,42 +2330,30 @@ TEST_F_EX(PgIndexBackfillTest, PhantomIdxEntry, PgIndexBackfillReadCommitted) { }); // There's no reliable indicator that index build has stopped before 'indislive' phase, just give // the index creation thread some extra time. - SleepFor(MonoDelta::FromMilliseconds(RegularBuildVsSanitizers(5000, 60000))); + SleepFor(RegularBuildVsSanitizers(5s, 60s)); auto other_conn = ASSERT_RESULT(ConnectToDB(kDatabaseName)); - // The index shouldn't be visible ASSERT_FALSE(ASSERT_RESULT(IsAtTargetIndexStateFlags(kIndexName, IndexStateFlags{}))); - // Start transaction and make sure it is in progress LOG(INFO) << "Begin older txn"; ASSERT_OK(other_conn.Execute("BEGIN")); const std::string query = Format("SELECT t FROM $0 WHERE i = $1", kTableName, 1); auto rows = ASSERT_RESULT((other_conn.FetchRows(query))); ASSERT_EQ(rows, (decltype(rows){{"a"}})); - // Allow index build to proceed to the next phase ASSERT_OK(cluster_->SetFlagOnTServers("ysql_yb_test_block_index_phase", "indisready")); ASSERT_OK(WaitForIndexStateFlags(index_live_flags, kIndexName)); - // New transaction can see the index and add 'b' into it LOG(INFO) << "Update record by newer txn"; ASSERT_OK(conn_->ExecuteFormat("UPDATE $0 SET t = 'b' WHERE i = $1", kTableName, 2)); - // If old transaction use cached metadata, it does not see index, so don't replace 'b' with 'c' - // index record b becomes phantom LOG(INFO) << "Update record by older txn"; ASSERT_OK(other_conn.ExecuteFormat("UPDATE $0 SET t = 'c' WHERE i = $1", kTableName, 2)); ASSERT_OK(other_conn.Execute("COMMIT")); - // New transaction attempts to replace 'c' with 'd' but if there's the phantom record, it only - // inserts 'd' LOG(INFO) << "Update record by newer txn again"; ASSERT_OK(conn_->ExecuteFormat("UPDATE $0 SET t = 'd' WHERE i = $1", kTableName, 2)); ASSERT_TRUE(ASSERT_RESULT(IsAtTargetIndexStateFlags(kIndexName, index_live_flags))); - // Make sure catalog versions differ - // Complete the index build ASSERT_OK(cluster_->SetFlagOnTServers("ysql_yb_test_block_index_phase", "none")); thread_holder_.Stop(); - // Check rowcount const std::string count_query = Format("SELECT count(t) FROM $0 WHERE t = 'b'", kTableName); ASSERT_OK(WaitForIndexScan(count_query)); LOG(INFO) << "Check for phantom record"; auto idx_count = ASSERT_RESULT((conn_->FetchRow(count_query))); - // Phantom entry makes count 1 ASSERT_EQ(idx_count, 0); }