Skip to content

Commit 05c3e3f

Browse files
GrigoriyPAGrigoriyPA
committed
YQ-4348 fixed verify fail during concurrent alter (#20269)
1 parent 0cffab3 commit 05c3e3f

File tree

8 files changed

+308
-60
lines changed

8 files changed

+308
-60
lines changed

‎ydb/core/kqp/executer_actor/kqp_scheme_executer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ class TKqpSchemeExecuter : public TActorBootstrapped<TKqpSchemeExecuter> {
545545
} else {
546546
ev->Result.SetStatus(status.GetStatus());
547547
if (TString message = status.GetErrorMessage()) {
548-
ev->Result.AddIssue(NYql::TIssue{message});
548+
ev->Result.AddIssue(NYql::TIssue(message).SetCode(status.GetStatus(), NYql::TSeverityIds::S_ERROR));
549549
}
550550
}
551551
actorSystem->Send(selfId, ev.Release());

‎ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp

Lines changed: 104 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -6683,14 +6683,14 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
66836683
.EnableExternalDataSourcesOnServerless(false)
66846684
.Create();
66856685

6686-
auto checkDisabled = [](const auto& result, NYdb::EStatus status) {
6687-
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), status, result.GetIssues().ToString());
6688-
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToString(), "External data sources are disabled for serverless domains. Please contact your system administrator to enable it");
6686+
auto checkDisabled = [](const auto& result) {
6687+
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::PRECONDITION_FAILED, result.GetIssues().ToString());
6688+
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "External data sources are disabled for serverless domains. Please contact your system administrator to enable it", result.GetIssues().ToString());
66896689
};
66906690

6691-
auto checkNotFound = [](const auto& result, NYdb::EStatus status) {
6692-
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), status, result.GetIssues().ToString());
6693-
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToString(), "Path does not exist");
6691+
auto checkNotFound = [](const auto& result) {
6692+
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, result.GetIssues().ToString());
6693+
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "Path does not exist", result.GetIssues().ToString());
66946694
};
66956695

66966696
const auto& createSourceSql = R"(
@@ -6731,40 +6731,63 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
67316731

67326732
// Serverless, disabled
67336733
settings.Database(ydb->GetSettings().GetServerlessTenantName()).NodeIndex(2);
6734-
checkDisabled(ydb->ExecuteQuery(createSourceSql, settings), NYdb::EStatus::GENERIC_ERROR);
6735-
checkDisabled(ydb->ExecuteQuery(createTableSql, settings), NYdb::EStatus::PRECONDITION_FAILED);
6736-
checkNotFound(ydb->ExecuteQuery(dropTableSql, settings), NYdb::EStatus::SCHEME_ERROR);
6737-
checkNotFound(ydb->ExecuteQuery(dropSourceSql, settings), NYdb::EStatus::GENERIC_ERROR);
6734+
checkDisabled(ydb->ExecuteQuery(createSourceSql, settings));
6735+
checkDisabled(ydb->ExecuteQuery(createTableSql, settings));
6736+
checkNotFound(ydb->ExecuteQuery(dropTableSql, settings));
6737+
checkNotFound(ydb->ExecuteQuery(dropSourceSql, settings));
67386738
}
67396739

67406740
Y_UNIT_TEST(CreateExternalDataSource) {
67416741
NKikimrConfig::TAppConfig appCfg;
6742-
appCfg.MutableQueryServiceConfig()->AddHostnamePatterns("my-bucket");
6742+
appCfg.MutableQueryServiceConfig()->AddHostnamePatterns("my-bucket|other-bucket");
6743+
appCfg.MutableFeatureFlags()->SetEnableReplaceIfExistsForExternalEntities(true);
67436744

67446745
TKikimrRunner kikimr(appCfg);
67456746
kikimr.GetTestServer().GetRuntime()->GetAppData(0).FeatureFlags.SetEnableExternalDataSources(true);
67466747
auto db = kikimr.GetTableClient();
67476748
auto session = db.CreateSession().GetValueSync().GetSession();
67486749
TString externalDataSourceName = "/Root/ExternalDataSource";
6749-
auto query = TStringBuilder() << R"(
6750-
CREATE EXTERNAL DATA SOURCE `)" << externalDataSourceName << R"(` WITH (
6751-
SOURCE_TYPE="ObjectStorage",
6752-
LOCATION="my-bucket",
6753-
AUTH_METHOD="NONE"
6754-
);)";
6755-
auto result = session.ExecuteSchemeQuery(query).GetValueSync();
6756-
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
6750+
{
6751+
auto query = TStringBuilder() << R"(
6752+
CREATE EXTERNAL DATA SOURCE `)" << externalDataSourceName << R"(` WITH (
6753+
SOURCE_TYPE="ObjectStorage",
6754+
LOCATION="my-bucket",
6755+
AUTH_METHOD="NONE"
6756+
);)";
6757+
auto result = session.ExecuteSchemeQuery(query).GetValueSync();
6758+
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
6759+
}
67576760

67586761
auto& runtime = *kikimr.GetTestServer().GetRuntime();
6759-
auto externalDataSourceDesc = Navigate(runtime, runtime.AllocateEdgeActor(), externalDataSourceName, NSchemeCache::TSchemeCacheNavigate::EOp::OpUnknown);
6760-
const auto& externalDataSource = externalDataSourceDesc->ResultSet.at(0);
6761-
UNIT_ASSERT_EQUAL(externalDataSource.Kind, NSchemeCache::TSchemeCacheNavigate::EKind::KindExternalDataSource);
6762-
UNIT_ASSERT(externalDataSource.ExternalDataSourceInfo);
6763-
UNIT_ASSERT_VALUES_EQUAL(externalDataSource.ExternalDataSourceInfo->Description.GetSourceType(), "ObjectStorage");
6764-
UNIT_ASSERT_VALUES_EQUAL(externalDataSource.ExternalDataSourceInfo->Description.GetInstallation(), "");
6765-
UNIT_ASSERT_VALUES_EQUAL(externalDataSource.ExternalDataSourceInfo->Description.GetLocation(), "my-bucket");
6766-
UNIT_ASSERT_VALUES_EQUAL(externalDataSource.ExternalDataSourceInfo->Description.GetName(), SplitPath(externalDataSourceName).back());
6767-
UNIT_ASSERT(externalDataSource.ExternalDataSourceInfo->Description.GetAuth().HasNone());
6762+
{
6763+
auto externalDataSourceDesc = Navigate(runtime, runtime.AllocateEdgeActor(), externalDataSourceName, NSchemeCache::TSchemeCacheNavigate::EOp::OpUnknown);
6764+
const auto& externalDataSource = externalDataSourceDesc->ResultSet.at(0);
6765+
UNIT_ASSERT_EQUAL(externalDataSource.Kind, NSchemeCache::TSchemeCacheNavigate::EKind::KindExternalDataSource);
6766+
UNIT_ASSERT(externalDataSource.ExternalDataSourceInfo);
6767+
UNIT_ASSERT_VALUES_EQUAL(externalDataSource.ExternalDataSourceInfo->Description.GetSourceType(), "ObjectStorage");
6768+
UNIT_ASSERT_VALUES_EQUAL(externalDataSource.ExternalDataSourceInfo->Description.GetInstallation(), "");
6769+
UNIT_ASSERT_VALUES_EQUAL(externalDataSource.ExternalDataSourceInfo->Description.GetLocation(), "my-bucket");
6770+
UNIT_ASSERT_VALUES_EQUAL(externalDataSource.ExternalDataSourceInfo->Description.GetName(), SplitPath(externalDataSourceName).back());
6771+
UNIT_ASSERT(externalDataSource.ExternalDataSourceInfo->Description.GetAuth().HasNone());
6772+
}
6773+
6774+
auto queryClient = kikimr.GetQueryClient();
6775+
{
6776+
auto query = TStringBuilder() << R"(
6777+
CREATE OR REPLACE EXTERNAL DATA SOURCE `)" << externalDataSourceName << R"(` WITH (
6778+
SOURCE_TYPE="ObjectStorage",
6779+
LOCATION="other-bucket",
6780+
AUTH_METHOD="NONE"
6781+
);)";
6782+
auto result = queryClient.ExecuteQuery(query, NYdb::NQuery::TTxControl::NoTx()).GetValueSync();
6783+
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
6784+
}
6785+
6786+
{
6787+
auto externalDataSourceDesc = Navigate(runtime, runtime.AllocateEdgeActor(), externalDataSourceName, NSchemeCache::TSchemeCacheNavigate::EOp::OpUnknown);
6788+
const auto& externalDataSource = externalDataSourceDesc->ResultSet.at(0);
6789+
UNIT_ASSERT_VALUES_EQUAL(externalDataSource.ExternalDataSourceInfo->Description.GetLocation(), "other-bucket");
6790+
}
67686791
}
67696792

67706793
Y_UNIT_TEST(CreateExternalDataSourceWithSa) {
@@ -6865,7 +6888,7 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
68656888
AUTH_METHOD="NONE"
68666889
);)";
68676890
auto result = session.ExecuteQuery(query, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
6868-
UNIT_ASSERT_VALUES_EQUAL(result.GetStatus(), EStatus::GENERIC_ERROR);
6891+
UNIT_ASSERT_VALUES_EQUAL(result.GetStatus(), EStatus::SCHEME_ERROR);
68696892
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "External source with type ObjectStorage is disabled. Please contact your system administrator to enable it", result.GetIssues().ToString());
68706893

68716894
auto query2 = TStringBuilder() << R"(
@@ -7014,38 +7037,63 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
70147037
NKikimrConfig::TAppConfig config;
70157038
config.MutableQueryServiceConfig()->AddAvailableExternalDataSources("ObjectStorage");
70167039
config.MutableQueryServiceConfig()->MutableS3()->SetGeneratorPathsLimit(50000);
7040+
config.MutableFeatureFlags()->SetEnableReplaceIfExistsForExternalEntities(true);
70177041
TKikimrRunner kikimr(NKqp::TKikimrSettings().SetAppConfig(config));
70187042

70197043
kikimr.GetTestServer().GetRuntime()->GetAppData(0).FeatureFlags.SetEnableExternalDataSources(true);
70207044
auto db = kikimr.GetTableClient();
70217045
auto session = db.CreateSession().GetValueSync().GetSession();
70227046
TString externalDataSourceName = "/Root/ExternalDataSource";
70237047
TString externalTableName = "/Root/ExternalTable";
7024-
auto query = TStringBuilder() << R"(
7025-
CREATE EXTERNAL DATA SOURCE `)" << externalDataSourceName << R"(` WITH (
7026-
SOURCE_TYPE="ObjectStorage",
7027-
LOCATION="my-bucket",
7028-
AUTH_METHOD="NONE"
7029-
);
7030-
CREATE EXTERNAL TABLE `)" << externalTableName << R"(` (
7031-
Key Uint64,
7032-
Value String
7033-
) WITH (
7034-
DATA_SOURCE=")" << externalDataSourceName << R"(",
7035-
LOCATION="/"
7036-
);)";
7037-
auto result = session.ExecuteSchemeQuery(query).GetValueSync();
7038-
UNIT_ASSERT_C(result.GetStatus() == EStatus::SUCCESS, result.GetIssues().ToString());
7048+
{
7049+
auto query = TStringBuilder() << R"(
7050+
CREATE EXTERNAL DATA SOURCE `)" << externalDataSourceName << R"(` WITH (
7051+
SOURCE_TYPE="ObjectStorage",
7052+
LOCATION="my-bucket",
7053+
AUTH_METHOD="NONE"
7054+
);
7055+
CREATE EXTERNAL TABLE `)" << externalTableName << R"(` (
7056+
Key Uint64,
7057+
Value String
7058+
) WITH (
7059+
DATA_SOURCE=")" << externalDataSourceName << R"(",
7060+
LOCATION="/"
7061+
);)";
7062+
auto result = session.ExecuteSchemeQuery(query).GetValueSync();
7063+
UNIT_ASSERT_C(result.GetStatus() == EStatus::SUCCESS, result.GetIssues().ToString());
7064+
}
70397065

70407066
auto& runtime = *kikimr.GetTestServer().GetRuntime();
7041-
auto externalTableDesc = Navigate(runtime, runtime.AllocateEdgeActor(), externalTableName, NKikimr::NSchemeCache::TSchemeCacheNavigate::EOp::OpUnknown);
7042-
const auto& externalTable = externalTableDesc->ResultSet.at(0);
7043-
UNIT_ASSERT_EQUAL(externalTable.Kind, NKikimr::NSchemeCache::TSchemeCacheNavigate::EKind::KindExternalTable);
7044-
UNIT_ASSERT(externalTable.ExternalTableInfo);
7045-
UNIT_ASSERT_VALUES_EQUAL(externalTable.ExternalTableInfo->Description.ColumnsSize(), 2);
7046-
UNIT_ASSERT_VALUES_EQUAL(externalTable.ExternalTableInfo->Description.GetDataSourcePath(), externalDataSourceName);
7047-
UNIT_ASSERT_VALUES_EQUAL(externalTable.ExternalTableInfo->Description.GetLocation(), "/");
7048-
UNIT_ASSERT_VALUES_EQUAL(externalTable.ExternalTableInfo->Description.GetSourceType(), "ObjectStorage");
7067+
{
7068+
auto externalTableDesc = Navigate(runtime, runtime.AllocateEdgeActor(), externalTableName, NKikimr::NSchemeCache::TSchemeCacheNavigate::EOp::OpUnknown);
7069+
const auto& externalTable = externalTableDesc->ResultSet.at(0);
7070+
UNIT_ASSERT_EQUAL(externalTable.Kind, NKikimr::NSchemeCache::TSchemeCacheNavigate::EKind::KindExternalTable);
7071+
UNIT_ASSERT(externalTable.ExternalTableInfo);
7072+
UNIT_ASSERT_VALUES_EQUAL(externalTable.ExternalTableInfo->Description.ColumnsSize(), 2);
7073+
UNIT_ASSERT_VALUES_EQUAL(externalTable.ExternalTableInfo->Description.GetDataSourcePath(), externalDataSourceName);
7074+
UNIT_ASSERT_VALUES_EQUAL(externalTable.ExternalTableInfo->Description.GetLocation(), "/");
7075+
UNIT_ASSERT_VALUES_EQUAL(externalTable.ExternalTableInfo->Description.GetSourceType(), "ObjectStorage");
7076+
}
7077+
7078+
auto queryClient = kikimr.GetQueryClient();
7079+
{
7080+
auto query = TStringBuilder() << R"(
7081+
CREATE OR REPLACE EXTERNAL TABLE `)" << externalTableName << R"(` (
7082+
Key Uint64,
7083+
Value String
7084+
) WITH (
7085+
DATA_SOURCE=")" << externalDataSourceName << R"(",
7086+
LOCATION="/other/location/"
7087+
);)";
7088+
auto result = queryClient.ExecuteQuery(query, NYdb::NQuery::TTxControl::NoTx()).GetValueSync();
7089+
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString());
7090+
}
7091+
7092+
{
7093+
auto externalTableDesc = Navigate(runtime, runtime.AllocateEdgeActor(), externalTableName, NKikimr::NSchemeCache::TSchemeCacheNavigate::EOp::OpUnknown);
7094+
const auto& externalTable = externalTableDesc->ResultSet.at(0);
7095+
UNIT_ASSERT_VALUES_EQUAL(externalTable.ExternalTableInfo->Description.GetLocation(), "/other/location/");
7096+
}
70497097
}
70507098

70517099
Y_UNIT_TEST(DisableCreateExternalTable) {
@@ -9717,13 +9765,13 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
97179765
.Create();
97189766

97199767
auto checkDisabled = [](const auto& result) {
9720-
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), NYdb::EStatus::GENERIC_ERROR, result.GetIssues().ToString());
9721-
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToString(), "Resource pools are disabled for serverless domains. Please contact your system administrator to enable it");
9768+
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::PRECONDITION_FAILED, result.GetIssues().ToString());
9769+
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "Resource pools are disabled for serverless domains. Please contact your system administrator to enable it", result.GetIssues().ToString());
97229770
};
97239771

97249772
auto checkNotFound = [](const auto& result) {
9725-
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), NYdb::EStatus::GENERIC_ERROR, result.GetIssues().ToString());
9726-
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToString(), "Path does not exist");
9773+
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, result.GetIssues().ToString());
9774+
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "Path does not exist", result.GetIssues().ToString());
97279775
};
97289776

97299777
const auto& createSql = R"(

‎ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ class TAlterExternalDataSource : public TSubOperation {
9797
checks.IsAtLocalSchemeShard()
9898
.IsResolved()
9999
.NotUnderDeleting()
100+
.NotUnderOperation()
100101
.FailOnWrongType(TPathElement::EPathType::EPathTypeExternalDataSource)
101102
.IsValidLeafName()
102103
.DepthLimit()

‎ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ class TAlterExternalTable: public TSubOperation {
134134
checks.IsAtLocalSchemeShard()
135135
.IsResolved()
136136
.NotUnderDeleting()
137+
.NotUnderOperation()
137138
.FailOnWrongType(TPathElement::EPathType::EPathTypeExternalTable)
138139
.IsValidLeafName()
139140
.DepthLimit()

‎ydb/core/tx/schemeshard/schemeshard__operation_alter_resource_pool.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ class TAlterResourcePool : public TSubOperation {
8787
checks.IsAtLocalSchemeShard()
8888
.IsResolved()
8989
.NotUnderDeleting()
90+
.NotUnderOperation()
9091
.FailOnWrongType(TPathElement::EPathType::EPathTypeResourcePool)
9192
.IsValidLeafName()
9293
.DepthLimit()

‎ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,65 @@ Y_UNIT_TEST_SUITE(TExternalDataSourceTest) {
463463
}
464464
}
465465

466+
Y_UNIT_TEST(ParallelReplaceExternalDataSourceIfNotExists) {
467+
TTestBasicRuntime runtime;
468+
TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExistsForExternalEntities(true).RunFakeConfigDispatcher(true));
469+
ui64 txId = 100;
470+
471+
TestCreateExternalDataSource(runtime, ++txId, "/MyRoot",R"(
472+
Name: "MyExternalDataSource"
473+
SourceType: "ObjectStorage"
474+
Location: "https://s3.cloud.net/my_bucket"
475+
Auth {
476+
None {
477+
}
478+
}
479+
ReplaceIfExists: true
480+
)", {NKikimrScheme::StatusAccepted}
481+
);
482+
483+
env.TestWaitNotification(runtime, txId);
484+
485+
constexpr ui32 TEST_RUNS = 30;
486+
TSet<ui64> txIds;
487+
for (ui32 i = 0; i < TEST_RUNS; ++i) {
488+
AsyncCreateExternalDataSource(runtime, ++txId, "/MyRoot",R"(
489+
Name: "MyExternalDataSource"
490+
SourceType: "ObjectStorage"
491+
Location: "https://s3.cloud.net/other_bucket"
492+
Auth {
493+
None {
494+
}
495+
}
496+
ReplaceIfExists: true
497+
)"
498+
);
499+
500+
txIds.insert(txId);
501+
}
502+
503+
ui32 acceptedCount = 0;
504+
for (auto testTx : txIds) {
505+
const auto result = TestModificationResults(runtime, testTx, {NKikimrScheme::StatusAccepted, NKikimrScheme::StatusMultipleModifications});
506+
acceptedCount += result == NKikimrScheme::StatusAccepted;
507+
}
508+
UNIT_ASSERT_GE(acceptedCount, 1);
509+
510+
env.TestWaitNotification(runtime, txIds);
511+
512+
{
513+
auto describeResult = DescribePath(runtime, "/MyRoot/MyExternalDataSource");
514+
TestDescribeResult(describeResult, {NLs::PathExist});
515+
UNIT_ASSERT(describeResult.GetPathDescription().HasExternalDataSourceDescription());
516+
const auto& externalDataSourceDescription = describeResult.GetPathDescription().GetExternalDataSourceDescription();
517+
UNIT_ASSERT_VALUES_EQUAL(externalDataSourceDescription.GetName(), "MyExternalDataSource");
518+
UNIT_ASSERT_VALUES_EQUAL(externalDataSourceDescription.GetVersion(), acceptedCount + 1);
519+
UNIT_ASSERT_VALUES_EQUAL(externalDataSourceDescription.GetSourceType(), "ObjectStorage");
520+
UNIT_ASSERT_VALUES_EQUAL(externalDataSourceDescription.GetLocation(), "https://s3.cloud.net/other_bucket");
521+
UNIT_ASSERT_EQUAL(externalDataSourceDescription.GetAuth().identity_case(), NKikimrSchemeOp::TAuth::kNone);
522+
}
523+
}
524+
466525
Y_UNIT_TEST(CreateExternalDataSourceShouldFailIfSuchEntityAlreadyExists) {
467526
TTestBasicRuntime runtime;
468527
TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExistsForExternalEntities(true).RunFakeConfigDispatcher(true));

0 commit comments

Comments
 (0)