Skip to content

25-1-1: Ingore basic scheme limits for internal operations #16001

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.

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions ydb/core/protos/index_builder.proto
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ message TEvCreateRequest {
optional uint64 TxId = 2;
optional string DatabaseName = 3;
optional TIndexBuildSettings Settings = 4;
// Internal flag is true for system-generated operations and is false for those initiated directly by the user.
optional bool Internal = 5 [default = false];
}

message TEvCreateResponse {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ TVector<ISubOperation::TPtr> CreateBuildColumn(TOperationId opId, const TTxTrans
{
auto outTx = TransactionTemplate(table.Parent().PathString(), NKikimrSchemeOp::EOperationType::ESchemeOpInitiateBuildIndexMainTable);
*outTx.MutableLockGuard() = tx.GetLockGuard();
outTx.SetInternal(tx.GetInternal());

auto& snapshot = *outTx.MutableInitiateBuildIndexMainTable();
snapshot.SetTableName(table.LeafName());
Expand Down Expand Up @@ -62,8 +63,12 @@ TVector<ISubOperation::TPtr> CreateBuildIndex(TOperationId opId, const TTxTransa
checks
.IsValidLeafName()
.PathsLimit(2) // index and impl-table
.DirChildrenLimit()
.ShardsLimit(1); // impl-table
.DirChildrenLimit();

if (!tx.GetInternal()) {
checks
.ShardsLimit(1); // impl-table
}

if (!checks) {
return {CreateReject(opId, checks.GetStatus(), checks.GetError())};
Expand Down Expand Up @@ -95,6 +100,7 @@ TVector<ISubOperation::TPtr> CreateBuildIndex(TOperationId opId, const TTxTransa
*outTx.MutableLockGuard() = tx.GetLockGuard();
outTx.MutableCreateTableIndex()->CopyFrom(indexDesc);
outTx.MutableCreateTableIndex()->SetState(NKikimrSchemeOp::EIndexStateWriteOnly);
outTx.SetInternal(tx.GetInternal());

if (!indexDesc.HasType()) {
outTx.MutableCreateTableIndex()->SetType(NKikimrSchemeOp::EIndexTypeGlobal);
Expand All @@ -106,6 +112,7 @@ TVector<ISubOperation::TPtr> CreateBuildIndex(TOperationId opId, const TTxTransa
{
auto outTx = TransactionTemplate(table.Parent().PathString(), NKikimrSchemeOp::EOperationType::ESchemeOpInitiateBuildIndexMainTable);
*outTx.MutableLockGuard() = tx.GetLockGuard();
outTx.SetInternal(tx.GetInternal());

auto& snapshot = *outTx.MutableInitiateBuildIndexMainTable();
snapshot.SetTableName(table.LeafName());
Expand All @@ -118,6 +125,7 @@ TVector<ISubOperation::TPtr> CreateBuildIndex(TOperationId opId, const TTxTransa

auto outTx = TransactionTemplate(index.PathString(), NKikimrSchemeOp::EOperationType::ESchemeOpInitiateBuildIndexImplTable);
*outTx.MutableCreateTable() = std::move(implTableDesc);
outTx.SetInternal(tx.GetInternal());

return CreateInitializeBuildIndexImplTable(NextPartId(opId, result), outTx);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,13 @@ TVector<ISubOperation::TPtr> CreateIndexedTable(TOperationId nextId, const TTxTr
{
auto checks = baseTablePath.Check();
checks
.PathShardsLimit(baseShards)
.PathsLimit(pathToCreate)
.ShardsLimit(shardsToCreate);
.PathsLimit(pathToCreate);

if (!tx.GetInternal()) {
checks
.PathShardsLimit(baseShards)
.ShardsLimit(shardsToCreate);
}

if (!checks) {
return {CreateReject(nextId, NKikimrScheme::EStatus::StatusResourceExhausted, checks.GetError())};
Expand Down Expand Up @@ -224,6 +228,7 @@ TVector<ISubOperation::TPtr> CreateIndexedTable(TOperationId nextId, const TTxTr
auto scheme = TransactionTemplate(tx.GetWorkingDir(), NKikimrSchemeOp::EOperationType::ESchemeOpCreateTable);
scheme.SetFailOnExist(tx.GetFailOnExist());
scheme.SetAllowCreateInTempDir(tx.GetAllowCreateInTempDir());
scheme.SetInternal(tx.GetInternal());

scheme.MutableCreateTable()->CopyFrom(baseTableDescription);
if (tx.HasAlterUserAttributes()) {
Expand Down Expand Up @@ -270,6 +275,7 @@ TVector<ISubOperation::TPtr> CreateIndexedTable(TOperationId nextId, const TTxTr
NKikimrSchemeOp::EOperationType::ESchemeOpCreateTable);
scheme.SetFailOnExist(tx.GetFailOnExist());
scheme.SetAllowCreateInTempDir(tx.GetAllowCreateInTempDir());
scheme.SetInternal(tx.GetInternal());

*scheme.MutableCreateTable() = std::move(implTableDesc);

Expand Down Expand Up @@ -311,6 +317,7 @@ TVector<ISubOperation::TPtr> CreateIndexedTable(TOperationId nextId, const TTxTr
NKikimrSchemeOp::EOperationType::ESchemeOpCreateSequence);
scheme.SetFailOnExist(tx.GetFailOnExist());
scheme.SetAllowCreateInTempDir(tx.GetAllowCreateInTempDir());
scheme.SetInternal(tx.GetInternal());

*scheme.MutableSequence() = sequenceDescription;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,9 +503,13 @@ class TCreateTable: public TSubOperation {
.IsValidLeafName()
.PathsLimit()
.DirChildrenLimit()
.ShardsLimit(shardsToCreate)
.PathShardsLimit(shardsToCreate)
.IsValidACL(acl);

if (!Transaction.GetInternal()) {
checks
.ShardsLimit(shardsToCreate)
.PathShardsLimit(shardsToCreate);
}
}

if (!checks) {
Expand Down
8 changes: 6 additions & 2 deletions ydb/core/tx/schemeshard/schemeshard_build_index__create.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,12 @@ class TSchemeShard::TIndexBuilder::TTxCreate: public TSchemeShard::TIndexBuilder
checks
.IsValidLeafName()
.PathsLimit(2) // index and impl-table
.DirChildrenLimit()
.ShardsLimit(1); // impl-table
.DirChildrenLimit();

if (!request.GetInternal()) {
checks
.ShardsLimit(1); // impl-table
}

if (!checks) {
return Reply(checks.GetStatus(), checks.GetError());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,9 @@ THolder<TEvIndexBuilder::TEvCreateRequest> BuildIndexPropose(

const TPath domainPath = TPath::Init(importInfo->DomainPathId, ss);
auto propose = MakeHolder<TEvIndexBuilder::TEvCreateRequest>(ui64(txId), domainPath.PathString(), std::move(settings));
(*propose->Record.MutableOperationParams()->mutable_labels())["uid"] = uid;
auto& request = propose->Record;
(*request.MutableOperationParams()->mutable_labels())["uid"] = uid;
request.SetInternal(true);

return propose;
}
Expand Down Expand Up @@ -262,11 +264,11 @@ THolder<TEvSchemeShard::TEvModifySchemeTransaction> CreateChangefeedPropose(
if (!FillChangefeedDescription(cdcStreamDescription, changefeed, status, error)) {
return nullptr;
}

if (topic.has_retention_period()) {
cdcStream.SetRetentionPeriodSeconds(topic.retention_period().seconds());
}

if (topic.has_partitioning_settings()) {
i64 minActivePartitions =
topic.partitioning_settings().min_active_partitions();
Expand Down
118 changes: 114 additions & 4 deletions ydb/core/tx/schemeshard/ut_restore/ut_restore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5310,6 +5310,116 @@ Y_UNIT_TEST_SUITE(TImportTests) {
Y_UNIT_TEST(ChangefeedsWithTablePermissions) {
TestImportChangefeeds(3, AddedSchemeWithPermissions);
}

Y_UNIT_TEST(IgnoreBasicSchemeLimits) {
TTestBasicRuntime runtime;
TTestEnv env(runtime);
ui64 txId = 100;

TestCreateExtSubDomain(runtime, ++txId, "/MyRoot", R"(
Name: "Alice"
)");
TestAlterExtSubDomain(runtime, ++txId, "/MyRoot", R"(
Name: "Alice"
ExternalSchemeShard: true
PlanResolution: 50
Coordinators: 1
Mediators: 1
TimeCastBucketsPerMediator: 2
StoragePools {
Name: "Alice:hdd"
Kind: "hdd"
}
)");
env.TestWaitNotification(runtime, txId);

ui64 tenantSchemeShard = 0;
TestDescribeResult(DescribePath(runtime, "/MyRoot/Alice"), {
NLs::ExtractTenantSchemeshard(&tenantSchemeShard)
});
UNIT_ASSERT_UNEQUAL(tenantSchemeShard, 0);

TSchemeLimits basicLimits;
basicLimits.MaxShards = 4;
basicLimits.MaxShardsInPath = 2;
SetSchemeshardSchemaLimits(runtime, basicLimits, tenantSchemeShard);

TestDescribeResult(DescribePath(runtime, tenantSchemeShard, "/MyRoot/Alice"), {
NLs::DomainLimitsIs(basicLimits.MaxPaths, basicLimits.MaxShards),
NLs::PathsInsideDomain(0),
NLs::ShardsInsideDomain(3)
});

TestCreateTable(runtime, tenantSchemeShard, ++txId, "/MyRoot/Alice", R"(
Name: "table1"
Columns { Name: "Key" Type: "Uint64" }
Columns { Name: "Value" Type: "Utf8" }
KeyColumnNames: ["Key"]
)");
env.TestWaitNotification(runtime, txId, tenantSchemeShard);

TestCreateTable(runtime, tenantSchemeShard, ++txId, "/MyRoot/Alice", R"(
Name: "table2"
Columns { Name: "Key" Type: "Uint64" }
Columns { Name: "Value" Type: "Utf8" }
KeyColumnNames: ["Key"]
)",
{ NKikimrScheme::StatusResourceExhausted }
);

const auto data = GenerateTestData(R"(
columns {
name: "key"
type { optional_type { item { type_id: UTF8 } } }
}
columns {
name: "value"
type { optional_type { item { type_id: UTF8 } } }
}
primary_key: "key"
partition_at_keys {
split_points {
type { tuple_type { elements { optional_type { item { type_id: UTF8 } } } } }
value { items { text_value: "b" } }
}
}
indexes {
name: "ByValue"
index_columns: "value"
global_index {}
}
)",
{{"a", 1}, {"b", 1}}
);

TPortManager portManager;
const ui16 port = portManager.GetPort();

TS3Mock s3Mock(ConvertTestData(data), TS3Mock::TSettings(port));
UNIT_ASSERT(s3Mock.Start());

const auto importId = ++txId;
TestImport(runtime, tenantSchemeShard, importId, "/MyRoot/Alice", Sprintf(R"(
ImportFromS3Settings {
endpoint: "localhost:%d"
scheme: HTTP
items {
source_prefix: ""
destination_path: "/MyRoot/Alice/ImportDir/Table"
}
}
)",
port
));
env.TestWaitNotification(runtime, importId, tenantSchemeShard);
TestGetImport(runtime, tenantSchemeShard, importId, "/MyRoot/Alice");

TestDescribeResult(DescribePath(runtime, tenantSchemeShard, "/MyRoot/Alice"), {
NLs::DomainLimitsIs(basicLimits.MaxPaths, basicLimits.MaxShards),
NLs::PathsInsideDomain(5),
NLs::ShardsInsideDomain(7)
});
}
}

Y_UNIT_TEST_SUITE(TImportWithRebootsTests) {
Expand Down Expand Up @@ -5701,7 +5811,7 @@ Y_UNIT_TEST_SUITE(TImportWithRebootsTests) {
const auto changefeedName = "update_changefeed";

schemes.emplace("", R"(
columns {
columns {
name: "key"
type { optional_type { item { type_id: UTF8 } } }
}
Expand Down Expand Up @@ -5768,11 +5878,11 @@ Y_UNIT_TEST_SUITE(TImportWithRebootsTests) {
}
}
)";

NAttr::TAttributes attr;
attr.emplace(NAttr::EKeys::TOPIC_DESCRIPTION, topicDesc);

schemes.emplace("/update_feed",
schemes.emplace("/update_feed",
TTypedScheme {
EPathTypeCdcStream,
changefeedDesc,
Expand All @@ -5789,7 +5899,7 @@ Y_UNIT_TEST_SUITE(TImportWithRebootsTests) {
Y_UNIT_TEST(CancelShouldSucceedOnSingleChangefeed) {
CancelShouldSucceed(GetSchemeWithChangefeed());
}

Y_UNIT_TEST(CancelShouldSucceedOnDependentView) {
CancelShouldSucceed(
{
Expand Down
Loading