Fix schemeshard crashes on double vector index replace / drop table after index replace (#20501) (#20665)

vitalif · web-flow · commit d332eed355e3 · 2025-07-08T12:59:05.000Z
diff --git a/ydb/core/grpc_services/rpc_rename_tables.cpp b/ydb/core/grpc_services/rpc_rename_tables.cpp
@@ -39,7 +39,7 @@ class TRenameTablesRPC : public TRpcSchemeRequestActor<TRenameTablesRPC, TEvRena
         auto& transaction = *record.MutableTransaction();
 
         if (req->tables().empty()) {
-            Request_->RaiseIssue(NYql::TIssue("Emply move list"));
+            Request_->RaiseIssue(NYql::TIssue("Empty move list"));
             return Reply(StatusIds::BAD_REQUEST, ctx);
         }
 
diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_move_index.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_move_index.cpp
@@ -597,6 +597,9 @@ TVector<ISubOperation::TPtr> CreateConsistentMoveIndex(TOperationId nextId, cons
     for(const auto& implTable : srcIndexPath.Base()->GetChildren()) {
         TString srcImplTableName = implTable.first;
         TPath srcImplTable = srcIndexPath.Child(srcImplTableName);
+        if (srcImplTable.IsDeleted()) {
+            continue;
+        }
 
         Y_ABORT_UNLESS(srcImplTable.Base()->PathId == implTable.second);
 
diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_move_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_move_table.cpp
@@ -716,6 +716,7 @@ class TMoveTable: public TSubOperation {
 
         if (dstParent.IsUnderOperation()) {
             dstPath = TPath::ResolveWithInactive(OperationId, dstPathStr, context.SS);
+            dstParent = dstPath.Parent();
         }
 
         {
diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_move_table_index.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_move_table_index.cpp
@@ -423,6 +423,7 @@ class TMoveTableIndex: public TSubOperation {
 
         if (dstParentPath.IsUnderOperation()) {
             dstPath = TPath::ResolveWithInactive(OperationId, dstPathStr, context.SS);
+            dstParentPath = dstPath.Parent();
         } else {
             Y_ABORT("NONO");
         }
diff --git a/ydb/core/tx/schemeshard/ut_helpers/helpers.cpp b/ydb/core/tx/schemeshard/ut_helpers/helpers.cpp
@@ -1924,11 +1924,11 @@ namespace NSchemeShardUT_Private {
     }
 
     void TestBuildVectorIndex(TTestActorRuntime& runtime, ui64 id, ui64 schemeShard, const TString &dbName,
-                              const TString &src, const TString &name, TString column,
+                              const TString &src, const TString &name, TVector<TString> columns,
                               Ydb::StatusIds::StatusCode expectedStatus)
     {
         TestBuildIndex(runtime, id, schemeShard, dbName, src, TBuildIndexConfig{
-            name, NKikimrSchemeOp::EIndexTypeGlobalVectorKmeansTree, {column}, {}
+            name, NKikimrSchemeOp::EIndexTypeGlobalVectorKmeansTree, columns, {}
         }, expectedStatus);
     }
 
diff --git a/ydb/core/tx/schemeshard/ut_helpers/helpers.h b/ydb/core/tx/schemeshard/ut_helpers/helpers.h
@@ -411,7 +411,7 @@ namespace NSchemeShardUT_Private {
         const TString &src, const TString& columnName, const Ydb::TypedValue& literal, Ydb::StatusIds::StatusCode expectedStatus);
     void TestBuildIndex(TTestActorRuntime& runtime, ui64 id, ui64 schemeShard, const TString &dbName, const TString &src, const TBuildIndexConfig &cfg, Ydb::StatusIds::StatusCode expectedStatus = Ydb::StatusIds::SUCCESS);
     void TestBuildIndex(TTestActorRuntime& runtime, ui64 id, ui64 schemeShard, const TString &dbName, const TString &src, const TString &name, TVector<TString> columns, Ydb::StatusIds::StatusCode expectedStatus = Ydb::StatusIds::SUCCESS);
-    void TestBuildVectorIndex(TTestActorRuntime& runtime, ui64 id, ui64 schemeShard, const TString &dbName, const TString &src, const TString &name, TString column, Ydb::StatusIds::StatusCode expectedStatus = Ydb::StatusIds::SUCCESS);
+    void TestBuildVectorIndex(TTestActorRuntime& runtime, ui64 id, ui64 schemeShard, const TString &dbName, const TString &src, const TString &name, TVector<TString> columns, Ydb::StatusIds::StatusCode expectedStatus = Ydb::StatusIds::SUCCESS);
     TEvIndexBuilder::TEvCancelRequest* CreateCancelBuildIndexRequest(const ui64 id, const TString& dbName, const ui64 buildIndexId);
     NKikimrIndexBuilder::TEvCancelResponse TestCancelBuildIndex(TTestActorRuntime& runtime, const ui64 id, const ui64 schemeShard, const TString &dbName, const ui64 buildIndexId, const TVector<Ydb::StatusIds::StatusCode>& expectedStatuses = {Ydb::StatusIds::SUCCESS});
     TEvIndexBuilder::TEvListRequest* ListBuildIndexRequest(const TString& dbName);
diff --git a/ydb/core/tx/schemeshard/ut_index_build/ut_vector_index_build.cpp b/ydb/core/tx/schemeshard/ut_index_build/ut_vector_index_build.cpp
@@ -92,7 +92,7 @@ Y_UNIT_TEST_SUITE(VectorIndexBuildTest) {
             {NLs::PathExist, NLs::IndexesCount(0), NLs::PathVersionEqual(3)});
 
         ui64 buildIndexTx = ++txId;
-        TestBuildVectorIndex(runtime, buildIndexTx, tenantSchemeShard, "/MyRoot/ServerLessDB", "/MyRoot/ServerLessDB/Table", "index1", "embedding");
+        TestBuildVectorIndex(runtime, buildIndexTx, tenantSchemeShard, "/MyRoot/ServerLessDB", "/MyRoot/ServerLessDB/Table", "index1", {"embedding"});
         env.TestWaitNotification(runtime, buildIndexTx, tenantSchemeShard);
 
         auto buildIndexOperations = TestListBuildIndex(runtime, tenantSchemeShard, "/MyRoot/ServerLessDB");
@@ -160,7 +160,7 @@ Y_UNIT_TEST_SUITE(VectorIndexBuildTest) {
 
         WriteVectorTableRows(runtime, tenantSchemeShard, ++txId, "/MyRoot/ServerLessDB/Table", 0, 0, 200, {1, 5, 3, 4});
 
-        TestBuildVectorIndex(runtime, ++txId, tenantSchemeShard, "/MyRoot/ServerLessDB", "/MyRoot/ServerLessDB/Table", "index2", "embedding");
+        TestBuildVectorIndex(runtime, ++txId, tenantSchemeShard, "/MyRoot/ServerLessDB", "/MyRoot/ServerLessDB/Table", "index2", {"embedding"});
         env.TestWaitNotification(runtime, txId, tenantSchemeShard);
 
         TestDescribeResult(DescribePath(runtime, tenantSchemeShard, "/MyRoot/ServerLessDB/Table"),
@@ -422,7 +422,7 @@ Y_UNIT_TEST_SUITE(VectorIndexBuildTest) {
 
         // Initiate index build:
         ui64 buildIndexTx = ++txId;
-        TestBuildVectorIndex(runtime, buildIndexTx, tenantSchemeShard, "/MyRoot/CommonDB", "/MyRoot/CommonDB/Table", "index1", "embedding");
+        TestBuildVectorIndex(runtime, buildIndexTx, tenantSchemeShard, "/MyRoot/CommonDB", "/MyRoot/CommonDB/Table", "index1", {"embedding"});
         {
             auto buildIndexOperations = TestListBuildIndex(runtime, tenantSchemeShard, "/MyRoot/CommonDB");
             UNIT_ASSERT_VALUES_EQUAL(buildIndexOperations.EntriesSize(), 1);
diff --git a/ydb/core/tx/schemeshard/ut_move/ut_move.cpp b/ydb/core/tx/schemeshard/ut_move/ut_move.cpp
@@ -1,3 +1,4 @@
+#include <ydb/core/base/table_vector_index.h>
 #include <ydb/core/kqp/ut/common/kqp_ut_common.h>
 #include <ydb/core/tx/datashard/change_exchange.h>
 #include <ydb/core/tx/schemeshard/ut_helpers/helpers.h>
@@ -8,6 +9,7 @@
 using namespace NKikimr;
 using namespace NSchemeShard;
 using namespace NSchemeShardUT_Private;
+using namespace NKikimr::NTableIndex::NTableVectorKmeansTreeIndex;
 
 void SetEnableMoveIndex(TTestActorRuntime &runtime, TTestEnv&, ui64 schemeShard, bool value) {
     auto request = MakeHolder<NConsole::TEvConsole::TEvConfigNotificationRequest>();
@@ -1020,6 +1022,59 @@ Y_UNIT_TEST_SUITE(TSchemeShardMoveTest) {
                             NLs::IndexesCount(2)});
     }
 
+    Y_UNIT_TEST(ReplaceVectorIndex) {
+        TTestBasicRuntime runtime;
+        TTestEnv env(runtime);
+        ui64 txId = 100;
+
+        TestCreateTable(runtime, ++txId, "/MyRoot", R"(
+            Name: "Table"
+            Columns { Name: "key"       Type: "Uint32" }
+            Columns { Name: "embedding" Type: "String" }
+            Columns { Name: "prefix"    Type: "Uint32" }
+            Columns { Name: "value"     Type: "String" }
+            KeyColumnNames: ["key"]
+        )");
+        env.TestWaitNotification(runtime, txId);
+
+        TestBuildVectorIndex(runtime, ++txId, TTestTxConfig::SchemeShard, "/MyRoot", "/MyRoot/Table", "index1", {"embedding"});
+        env.TestWaitNotification(runtime, txId);
+
+        TestBuildVectorIndex(runtime, ++txId, TTestTxConfig::SchemeShard, "/MyRoot", "/MyRoot/Table", "index2", {"prefix", "embedding"});
+        env.TestWaitNotification(runtime, txId);
+
+        TestDescribeResult(DescribePath(runtime, "/MyRoot/Table"), {NLs::PathExist, NLs::PathVersionEqual(9), NLs::IndexesCount(2)});
+
+        TestMoveIndex(runtime, ++txId, "/MyRoot/Table", "index2", "index1", true);
+        env.TestWaitNotification(runtime, txId);
+
+        TestDescribeResult(DescribePath(runtime, "/MyRoot/Table/index2"), {NLs::PathNotExist});
+        TestDescribeResult(DescribePath(runtime, "/MyRoot/Table"), {NLs::PathExist, NLs::IndexesCount(1)});
+        TestDescribeResult(DescribePrivatePath(runtime, "/MyRoot/Table/index1/indexImplPrefixTable"),
+            { NLs::PathExist, NLs::CheckColumns(PrefixTable, {"prefix", IdColumn}, {}, {"prefix", IdColumn}, true) });
+        TestDescribeResult(DescribePrivatePath(runtime, "/MyRoot/Table/index1/indexImplLevelTable"),
+            { NLs::PathExist, NLs::CheckColumns(LevelTable, {ParentColumn, IdColumn, CentroidColumn}, {}, {ParentColumn, IdColumn}, true) });
+        TestDescribeResult(DescribePrivatePath(runtime, "/MyRoot/Table/index1/indexImplPostingTable"),
+            { NLs::PathExist, NLs::CheckColumns(PostingTable, {ParentColumn, "key"}, {}, {ParentColumn, "key"}, true) });
+
+        // Replace again - it previously crashed here when Dec/IncAliveChildren were incorrect
+
+        TestBuildVectorIndex(runtime, ++txId, TTestTxConfig::SchemeShard, "/MyRoot", "/MyRoot/Table", "index2", {"embedding"});
+        env.TestWaitNotification(runtime, txId);
+
+        TestMoveIndex(runtime, ++txId, "/MyRoot/Table", "index2", "index1", true);
+        env.TestWaitNotification(runtime, txId);
+
+        TestDescribeResult(DescribePath(runtime, "/MyRoot/Table/index2"), {NLs::PathNotExist});
+        TestDescribeResult(DescribePath(runtime, "/MyRoot/Table"), {NLs::PathExist, NLs::IndexesCount(1)});
+        TestDescribeResult(DescribePrivatePath(runtime, "/MyRoot/Table/index1/indexImplPrefixTable"), {NLs::PathNotExist});
+        TestDescribeResult(DescribePrivatePath(runtime, "/MyRoot/Table/index1/indexImplLevelTable"),
+            { NLs::PathExist, NLs::CheckColumns(LevelTable, {ParentColumn, IdColumn, CentroidColumn}, {}, {ParentColumn, IdColumn}, true) });
+        TestDescribeResult(DescribePrivatePath(runtime, "/MyRoot/Table/index1/indexImplPostingTable"),
+            { NLs::PathExist, NLs::CheckColumns(PostingTable, {ParentColumn, "key"}, {}, {ParentColumn, "key"}, true) });
+    }
+
+
     Y_UNIT_TEST(AsyncIndexWithSyncInFly) {
         TTestBasicRuntime runtime;
         TTestEnv env(runtime);
diff --git a/ydb/tests/stress/common/common.py b/ydb/tests/stress/common/common.py
@@ -34,6 +34,11 @@ def drop_table(self, path_to_table):
         else:
             self.session_pool.retry_operation_sync(lambda session: session.drop_table(path_to_table))
 
+    def replace_index(self, table, src, dst):
+        self.driver.table_client.alter_table(path=table, rename_indexes=[
+            ydb.RenameIndexItem(source_name=src, destination_name=dst, replace_destination=True)
+        ])
+
     def describe(self, path):
         try:
             return self.driver.scheme_client.describe_path(path)
diff --git a/ydb/tests/stress/oltp_workload/workload/type/vector_index.py b/ydb/tests/stress/oltp_workload/workload/type/vector_index.py
@@ -207,6 +207,18 @@ def _check_loop(self, table_path, vector_type, vector_dimension, levels, cluster
             distance=distance,
             similarity=similarity,
         )
+        if random.randint(0, 1) == 0:
+            self._create_index(
+                index_name=index_name+'Rename',
+                table_path=table_path,
+                vector_type=vector_type,
+                vector_dimension=vector_dimension,
+                levels=levels,
+                clusters=clusters,
+                distance=distance,
+                similarity=similarity,
+            )
+            self.client.replace_index(table_path, index_name+'Rename', index_name)
         self._drop_index(index_name, table_path)
         logger.info('check was completed successfully')
 

Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ class TRenameTablesRPC : public TRpcSchemeRequestActor<TRenameTablesRPC, TEvRena`
`39`	`39`	`auto& transaction = *record.MutableTransaction();`
`40`	`40`
`41`	`41`	`if (req->tables().empty()) {`
`42`		`- Request_->RaiseIssue(NYql::TIssue("Emply move list"));`
	`42`	`+ Request_->RaiseIssue(NYql::TIssue("Empty move list"));`
`43`	`43`	`return Reply(StatusIds::BAD_REQUEST, ctx);`
`44`	`44`	`}`
`45`	`45`
Original file line number	Diff line number	Diff line change
`@@ -716,6 +716,7 @@ class TMoveTable: public TSubOperation {`
`716`	`716`
`717`	`717`	`if (dstParent.IsUnderOperation()) {`
`718`	`718`	`dstPath = TPath::ResolveWithInactive(OperationId, dstPathStr, context.SS);`
	`719`	`+ dstParent = dstPath.Parent();`
`719`	`720`	`}`
`720`	`721`
`721`	`722`	`{`
Original file line number	Diff line number	Diff line change
`@@ -423,6 +423,7 @@ class TMoveTableIndex: public TSubOperation {`
`423`	`423`
`424`	`424`	`if (dstParentPath.IsUnderOperation()) {`
`425`	`425`	`dstPath = TPath::ResolveWithInactive(OperationId, dstPathStr, context.SS);`
	`426`	`+ dstParentPath = dstPath.Parent();`
`426`	`427`	`} else {`
`427`	`428`	`Y_ABORT("NONO");`
`428`	`429`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1924,11 +1924,11 @@ namespace NSchemeShardUT_Private {`
`1924`	`1924`	`}`
`1925`	`1925`
`1926`	`1926`	`void TestBuildVectorIndex(TTestActorRuntime& runtime, ui64 id, ui64 schemeShard, const TString &dbName,`
`1927`		`- const TString &src, const TString &name, TString column,`
	`1927`	`+ const TString &src, const TString &name, TVector<TString> columns,`
`1928`	`1928`	`Ydb::StatusIds::StatusCode expectedStatus)`
`1929`	`1929`	`{`
`1930`	`1930`	`TestBuildIndex(runtime, id, schemeShard, dbName, src, TBuildIndexConfig{`
`1931`		`- name, NKikimrSchemeOp::EIndexTypeGlobalVectorKmeansTree, {column}, {}`
	`1931`	`+ name, NKikimrSchemeOp::EIndexTypeGlobalVectorKmeansTree, columns, {}`
`1932`	`1932`	`}, expectedStatus);`
`1933`	`1933`	`}`
`1934`	`1934`