Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c2deada
Fix schemeshard crashing on double vector index replace or on drop af…
vitalif Jul 4, 2025
54f2daa
Fix CreateConsistentMoveIndex sometimes trying to move deleted paths
vitalif Jul 4, 2025
e2ab35f
Add a rename test for vector index
vitalif Jul 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ydb/core/grpc_services/rpc_rename_tables.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
3 changes: 3 additions & 0 deletions ydb/core/tx/schemeshard/schemeshard__operation_move_index.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions ydb/core/tx/schemeshard/schemeshard__operation_move_table.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,7 @@ class TMoveTable: public TSubOperation {

if (dstParent.IsUnderOperation()) {
dstPath = TPath::ResolveWithInactive(OperationId, dstPathStr, context.SS);
dstParent = dstPath.Parent();
}

{
Expand Down
1 change: 1 addition & 0 deletions ydb/core/tx/schemeshard/schemeshard__operation_move_table_index.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
4 changes: 2 additions & 2 deletions ydb/core/tx/schemeshard/ut_helpers/helpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion ydb/core/tx/schemeshard/ut_helpers/helpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions ydb/core/tx/schemeshard/ut_index_build/ut_vector_index_build.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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);
Expand Down
47 changes: 47 additions & 0 deletions ydb/core/tx/schemeshard/ut_move/ut_move.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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>
Expand All @@ -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>();
Expand Down Expand Up @@ -1020,6 +1022,51 @@ 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) });

// Replace again - it previously crashed here when Dec/IncAliveChildren were incorrect
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Для будущих поколений это не несет информации

Suggested change
// Replace again - it previously crashed here when Dec/IncAliveChildren were incorrect
// Replace again

Copy link
Collaborator Author

@vitalif vitalif Jul 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ну... хочется отметить, что в этом месте может быть баг и такой тест (два раза одинаковое переименование) не надо убирать. Ну давай я напишу

// Replace again to check that the renamed index has correct AliveChildren count

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ну или просто может и можно оставить как есть?


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});
}


Y_UNIT_TEST(AsyncIndexWithSyncInFly) {
TTestBasicRuntime runtime;
TTestEnv env(runtime);
Expand Down
Loading