Skip to content

Commit 9ba8604

Browse files
committed
[#26633] DocDB: Drop invalid index DDL halts the replication in xCluster
Summary: ### Problem Dropping an INVALID index halts xCluster DDL replication. When CREATE UNIQUE INDEX fails due to a duplicate key violation, the index is left in an INVALID state (`indisvalid=false`) on the source but is never created on the target. Dropping this index causes replication to fail with: - `index "<invalid_index_name>" does not exist` on the target - `DDL replication is paused due to repeated failures` ### Root Cause There is a state mismatch between the source and target catalogs. The CREATE INDEX that failed was never replicated to the target (since ddl_command_end doesn't execute for failed DDLs). However, the subsequent DROP INDEX was replicated, which causes the target to try dropping an index that doesn't exist. This causes the state mismatch between the source and target catalogs: the source has an invalid object and the target has no object. The replication stream sends DDLs assuming the object exists on the target, violating the "exactly one" semantics required for the target. ### Solution On the target side, when `yb_xcluster_automatic_mode_target_ddl` is set (running in xCluster automatic mode), we implement a "silent skip" for DROP INDEX, RENAME INDEX and attribute/tablespace changes like SET STATISTICS, SET TABLESPACE. By treating these "phantom" objects (failed creation on the source and never existed on the target) as no-ops, the DDL applier can progress through the replication stream without halting. Valid objects are still dropped normally. Test Plan: ### Automated Tests ``` ./yb_build.sh release --cxx-test xcluster_ddl_replication-test --gtest_filter='XClusterDDLReplicationTest.DropInvalidIndexDoesNotHaltReplication' ./yb_build.sh release --cxx-test xcluster_ddl_replication-test --gtest_filter='XClusterDDLReplicationTest.DropPartitionedIndexOnOnlyReplicates' ./yb_build.sh release --cxx-test xcluster_ddl_replication-test --gtest_filter='XClusterDDLReplicationTest.AlterPhantomIndexLifecycleDoesNotHaltReplication' ``` Test 1: DropInvalidIndexDoesNotHaltReplication - CREATE UNIQUE INDEX fails due to duplicate key, meaning the index is INVALID and not replicated - DROP INDEX is replicated to target, which fails with "does not exist", and we validate that the error is ignored - Then verify that replication continues normally Test 2: DropPartitionedIndexOnOnlyReplicates - CREATE INDEX ON ONLY creates an INVALID index that IS replicated, meaning the CREATE succeeded - DROP INDEX is replicated to target, which succeeds since the index exists - Then verify that we don't incorrectly skip DROP for valid cases Test 3: AlterPhantomIndexLifecycleDoesNotHaltReplication - CREATE UNIQUE INDEX (on an expression) fails on source and is not created on target - ALTER INDEX ... SET STATISTICS and SET TABLESPACE are replicated for the phantom index - ALTER INDEX ... RENAME TO is replicated, changing the phantom's name on the source - ALTER TABLE ... SET SCHEMA is replicated, triggering an implicit schema move for the phantom index - DROP INDEX using the new name is replicated - Then verify that the target handles the entire lifecycle of attribute, name, and namespace changes on the missing index gracefully, and replication continues (Note: SET TABLESPACE here goes through the same code path as SET STATISTICS: `T_AlterTableStmt`) The fixed code passes all four tests. The old code fails Test 1 (because the "does not exist" error halts replication) and Test, but passes Test 2 (because the index was successfully created and replicated, so the DROP succeeds normally). This is because Test 2 validates the fix doesn't break successful CREATEs of INVALID indexes (meaning the index was replicated). Did not test DROP on multiple indices since this isn't supported yet in YB. See [issue #880](#880). Reviewers: jhe, xCluster, hsunder Reviewed By: jhe Subscribers: ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D50184
1 parent 1152182 commit 9ba8604

File tree

4 files changed

+204
-0
lines changed

4 files changed

+204
-0
lines changed

src/postgres/yb-extensions/yb_xcluster_ddl_replication/yb_xcluster_ddl_replication.c

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,11 +480,52 @@ HandleSourceDDLStart(EventTriggerData *trig_data)
480480
ClearRewrittenTableOidList();
481481
}
482482

483+
/*
484+
* xCluster: Prevent replication from halting on phantom indexes (invalid
485+
* indexes that exist on the source but not the target). DDLs referencing them
486+
* are silently skipped rather than forcing replication to halt.
487+
*/
488+
static void
489+
yb_xcluster_handle_phantom_indexes(Node *parsetree)
490+
{
491+
if (yb_xcluster_automatic_mode_target_ddl)
492+
{
493+
switch (nodeTag(parsetree))
494+
{
495+
case T_DropStmt:
496+
{
497+
DropStmt *stmt = (DropStmt *) parsetree;
498+
if (stmt->removeType == OBJECT_INDEX)
499+
stmt->missing_ok = true;
500+
break;
501+
}
502+
case T_RenameStmt:
503+
{
504+
RenameStmt *stmt = (RenameStmt *) parsetree;
505+
if (stmt->renameType == OBJECT_INDEX)
506+
stmt->missing_ok = true;
507+
break;
508+
}
509+
case T_AlterTableStmt:
510+
{
511+
AlterTableStmt *stmt = (AlterTableStmt *) parsetree;
512+
if (stmt->objtype == OBJECT_INDEX)
513+
stmt->missing_ok = true;
514+
break;
515+
}
516+
default:
517+
break;
518+
}
519+
}
520+
}
521+
483522
void
484523
HandleTargetDDLStart(EventTriggerData *trig_data)
485524
{
486525
yb_xcluster_target_ddl_bypass = false;
487526

527+
yb_xcluster_handle_phantom_indexes(parsetree);
528+
488529
/* Bypass DDLs executed in manual mode, or from the target poller. */
489530
if (enable_manual_ddl_replication || yb_xcluster_automatic_mode_target_ddl)
490531
{

src/yb/integration-tests/xcluster/xcluster_ddl_replication-test.cc

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -926,6 +926,161 @@ TEST_F(XClusterDDLReplicationTest, NonconcurrentBackfillsWithPartitions) {
926926
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
927927
}
928928

929+
TEST_F(XClusterDDLReplicationTest, DropInvalidIndexDoesNotHaltReplication) {
930+
ASSERT_OK(SetUpClustersAndReplication());
931+
932+
const std::string kBaseTableName = "test_invalid_idx";
933+
const std::string kIndexName = "idx_invalid";
934+
const std::string kColumn2Name = "value";
935+
936+
ASSERT_OK(producer_conn_->ExecuteFormat(
937+
"CREATE TABLE $0($1 int PRIMARY KEY, $2 int);", kBaseTableName, kKeyColumnName,
938+
kColumn2Name));
939+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
940+
941+
// Insert rows with duplicate values to cause unique index creation to fail.
942+
ASSERT_OK(producer_conn_->ExecuteFormat(
943+
"INSERT INTO $0 VALUES (1, 10), (2, 10), (3, 20);", kBaseTableName));
944+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
945+
ASSERT_OK(VerifyWrittenRecords({kBaseTableName}));
946+
947+
// Create unique index fails due to duplicate key violation.
948+
auto create_result = producer_conn_->ExecuteFormat(
949+
"CREATE UNIQUE INDEX $0 ON $1($2)", kIndexName, kBaseTableName, kColumn2Name);
950+
ASSERT_NOK(create_result);
951+
ASSERT_STR_CONTAINS(create_result.ToString(), "duplicate key");
952+
953+
// Verify index is invalid on producer.
954+
ASSERT_FALSE(ASSERT_RESULT(IsIndexValid(*producer_conn_, kIndexName)))
955+
<< "Index should be INVALID";
956+
957+
// Verify invalid index was not replicated to consumer using consumer_conn_ directly.
958+
auto consumer_index_exists = ASSERT_RESULT(consumer_conn_->FetchRow<bool>(Format(
959+
"SELECT EXISTS(SELECT 1 FROM pg_class WHERE relname = '$0')", kIndexName)));
960+
ASSERT_FALSE(consumer_index_exists) << "INVALID index should not exist on consumer";
961+
962+
// Drop the invalid index.
963+
ASSERT_OK(producer_conn_->ExecuteFormat("DROP INDEX $0", kIndexName));
964+
965+
// Verify replication continues by writing more rows to the original table.
966+
ASSERT_OK(producer_conn_->ExecuteFormat(
967+
"INSERT INTO $0 VALUES (4, 30), (5, 40);", kBaseTableName));
968+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
969+
ASSERT_OK(VerifyWrittenRecords({kBaseTableName}));
970+
}
971+
972+
973+
TEST_F(XClusterDDLReplicationTest, DropPartitionedIndexOnOnlyReplicates) {
974+
ASSERT_OK(SetUpClustersAndReplication());
975+
976+
const std::string kParentTableName = "test_partitioned";
977+
const std::string kPartitionName = "test_partitioned_p1";
978+
const std::string kIndexName = "idx_partitioned_only";
979+
const std::string kColumn2Name = "value";
980+
981+
ASSERT_OK(producer_conn_->ExecuteFormat(
982+
"CREATE TABLE $0($1 int PRIMARY KEY, $2 int) PARTITION BY RANGE ($1);",
983+
kParentTableName, kKeyColumnName, kColumn2Name));
984+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
985+
986+
ASSERT_OK(producer_conn_->ExecuteFormat(
987+
"CREATE TABLE $0 PARTITION OF $1 FOR VALUES FROM (1) TO (100);",
988+
kPartitionName, kParentTableName));
989+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
990+
991+
// ON ONLY creates an index marked invalid until attached to a partition index.
992+
ASSERT_OK(producer_conn_->ExecuteFormat(
993+
"CREATE INDEX $0 ON ONLY $1($2)", kIndexName, kParentTableName, kColumn2Name));
994+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
995+
996+
// Verify index is invalid on producer.
997+
ASSERT_FALSE(ASSERT_RESULT(IsIndexValid(*producer_conn_, kIndexName)))
998+
<< "Index should be INVALID on producer";
999+
1000+
// Verify the invalid index was replicated to consumer.
1001+
auto consumer_index_exists = ASSERT_RESULT(consumer_conn_->FetchRow<bool>(Format(
1002+
"SELECT EXISTS(SELECT 1 FROM pg_class WHERE relname = '$0')", kIndexName)));
1003+
ASSERT_TRUE(consumer_index_exists) << "Partitioned index should exist on consumer";
1004+
1005+
// Create a child index and attach it to verify the ATTACH PARTITION path.
1006+
const std::string kPartIdxName = "idx_partition_child";
1007+
ASSERT_OK(producer_conn_->ExecuteFormat(
1008+
"CREATE INDEX $0 ON $1($2)", kPartIdxName, kPartitionName, kColumn2Name));
1009+
ASSERT_OK(producer_conn_->ExecuteFormat(
1010+
"ALTER INDEX $0 ATTACH PARTITION $1", kIndexName, kPartIdxName));
1011+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
1012+
1013+
// Verify parent index is now valid on both clusters.
1014+
ASSERT_TRUE(ASSERT_RESULT(IsIndexValid(*producer_conn_, kIndexName)));
1015+
ASSERT_TRUE(ASSERT_RESULT(IsIndexValid(*consumer_conn_, kIndexName)))
1016+
<< "Attached index did not become valid on consumer";
1017+
1018+
// Cleanup: drop the parent index.
1019+
ASSERT_OK(producer_conn_->ExecuteFormat("DROP INDEX $0", kIndexName));
1020+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
1021+
1022+
// Verify the drop was replicated.
1023+
consumer_index_exists = ASSERT_RESULT(consumer_conn_->FetchRow<bool>(Format(
1024+
"SELECT EXISTS(SELECT 1 FROM pg_class WHERE relname = '$0')", kIndexName)));
1025+
ASSERT_FALSE(consumer_index_exists) << "Dropped index should not exist on consumer";
1026+
1027+
// Verify replication continues.
1028+
const std::string kSecondTableName = "verify_replication_works2";
1029+
ASSERT_OK(producer_conn_->ExecuteFormat(
1030+
"CREATE TABLE $0($1 int PRIMARY KEY);", kSecondTableName, kKeyColumnName));
1031+
ASSERT_OK(producer_conn_->ExecuteFormat(
1032+
"INSERT INTO $0 VALUES (1), (2), (3);", kSecondTableName));
1033+
1034+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
1035+
ASSERT_OK(VerifyWrittenRecords({kSecondTableName}));
1036+
}
1037+
1038+
TEST_F(XClusterDDLReplicationTest, AlterPhantomIndexLifecycleDoesNotHaltReplication) {
1039+
ASSERT_OK(SetUpClustersAndReplication());
1040+
1041+
const std::string kTableName = "schema_test_table";
1042+
const std::string kIdxName = "idx_phantom_schema";
1043+
const std::string kIdxNewName = "idx_phantom_renamed";
1044+
const std::string kSchemaName = "new_index_schema";
1045+
1046+
// Setup table with duplicate data and a new schema.
1047+
ASSERT_OK(producer_conn_->ExecuteFormat(
1048+
"CREATE TABLE $0(key int PRIMARY KEY, val int);", kTableName));
1049+
ASSERT_OK(producer_conn_->ExecuteFormat("INSERT INTO $0 VALUES (1, 10), (2, 10);", kTableName));
1050+
ASSERT_OK(producer_conn_->ExecuteFormat("CREATE SCHEMA $0;", kSchemaName));
1051+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
1052+
1053+
// Create unique index fails on producer and is not created on consumer.
1054+
ASSERT_NOK_STR_CONTAINS(
1055+
producer_conn_->ExecuteFormat(
1056+
"CREATE UNIQUE INDEX $0 ON $1((val + 0));", kIdxName, kTableName),
1057+
"duplicate key");
1058+
1059+
// ALTER INDEX attribute change on the phantom index.
1060+
ASSERT_OK(producer_conn_->ExecuteFormat(
1061+
"ALTER INDEX $0 ALTER COLUMN 1 SET STATISTICS 100;", kIdxName));
1062+
1063+
// ALTER INDEX tablespace change on the phantom index.
1064+
ASSERT_OK(producer_conn_->Execute("CREATE TABLESPACE dummy_ts LOCATION '/tmp/dummy_path';"));
1065+
ASSERT_OK(producer_conn_->ExecuteFormat("ALTER INDEX $0 SET TABLESPACE dummy_ts;", kIdxName));
1066+
1067+
// Rename the phantom index.
1068+
ASSERT_OK(producer_conn_->ExecuteFormat("ALTER INDEX $0 RENAME TO $1;", kIdxName, kIdxNewName));
1069+
1070+
// Move the parent table to a new schema (indexes follow implicitly).
1071+
ASSERT_OK(producer_conn_->ExecuteFormat(
1072+
"ALTER TABLE $0 SET SCHEMA $1;", kTableName, kSchemaName));
1073+
1074+
// Drop the phantom index (renamed, now in the new schema).
1075+
ASSERT_OK(producer_conn_->ExecuteFormat("DROP INDEX $0.$1;", kSchemaName, kIdxNewName));
1076+
1077+
// Verify replication continues by writing more rows to the original table.
1078+
ASSERT_OK(producer_conn_->ExecuteFormat(
1079+
"INSERT INTO $0.$1 VALUES (3, 30), (4, 40);", kSchemaName, kTableName));
1080+
ASSERT_OK(WaitForSafeTimeToAdvanceToNow());
1081+
ASSERT_OK(VerifyWrittenRecords({kTableName}, /*database_name=*/"", kSchemaName));
1082+
}
1083+
9291084
TEST_F(XClusterDDLReplicationTest, ExactlyOnceReplication) {
9301085
// Test that DDLs are only replicated exactly once.
9311086
const int kNumTablets = 3;

src/yb/integration-tests/xcluster/xcluster_ysql_test_base.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,12 @@ Result<pgwrapper::PGResultPtr> XClusterYsqlTestBase::ScanToStrings(
507507
return result;
508508
}
509509

510+
Result<bool> XClusterYsqlTestBase::IsIndexValid(
511+
pgwrapper::PGConn& conn, const std::string& index_name) {
512+
return conn.FetchRow<bool>(Format(
513+
"SELECT indisvalid FROM pg_index WHERE indexrelid = '$0'::regclass", index_name));
514+
}
515+
510516
Result<int> XClusterYsqlTestBase::GetRowCount(
511517
const YBTableName& table_name, Cluster* cluster, bool read_latest) {
512518
auto conn = VERIFY_RESULT(

src/yb/integration-tests/xcluster/xcluster_ysql_test_base.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ class XClusterYsqlTestBase : public XClusterTestBase {
109109
static Result<pgwrapper::PGResultPtr> ScanToStrings(
110110
const client::YBTableName& table_name, Cluster* cluster);
111111

112+
static Result<bool> IsIndexValid(pgwrapper::PGConn& conn, const std::string& index_name);
113+
112114
static Result<int> GetRowCount(
113115
const client::YBTableName& table_name, Cluster* cluster, bool read_latest = false);
114116

0 commit comments

Comments
 (0)