From 10fe0fb5eaf5fab7ee233243814d58665dddf7ad Mon Sep 17 00:00:00 2001 From: Grigoriy Pisarenko Date: Tue, 3 Sep 2024 11:27:55 +0000 Subject: [PATCH 1/4] Added database checking for metadata objects --- ydb/core/protos/feature_flags.proto | 1 + ydb/services/metadata/manager/alter_impl.h | 26 ++++ ydb/services/metadata/manager/common.h | 1 + .../metadata/manager/fetch_database.cpp | 112 ++++++++++++++++++ .../metadata/manager/fetch_database.h | 24 ++++ ydb/services/metadata/manager/ya.make | 2 + 6 files changed, 166 insertions(+) create mode 100644 ydb/services/metadata/manager/fetch_database.cpp create mode 100644 ydb/services/metadata/manager/fetch_database.h diff --git a/ydb/core/protos/feature_flags.proto b/ydb/core/protos/feature_flags.proto index fc7346a8c3cb..aa7c2296efaf 100644 --- a/ydb/core/protos/feature_flags.proto +++ b/ydb/core/protos/feature_flags.proto @@ -157,4 +157,5 @@ message TFeatureFlags { optional bool EnableAlterShardingInColumnShard = 138 [default = false]; optional bool EnablePgSyntax = 139 [default = true]; optional bool EnableTieringInColumnShard = 140 [default = false]; + optional bool EnableMetadataObjectsOnServerless = 141 [default = true]; } diff --git a/ydb/services/metadata/manager/alter_impl.h b/ydb/services/metadata/manager/alter_impl.h index 25e7cca1bca9..a216f100b9a2 100644 --- a/ydb/services/metadata/manager/alter_impl.h +++ b/ydb/services/metadata/manager/alter_impl.h @@ -1,5 +1,6 @@ #pragma once #include "abstract.h" +#include "fetch_database.h" #include "modification_controller.h" #include "preparation_controller.h" #include "restore.h" @@ -111,6 +112,7 @@ class TModificationActorImpl: public NActors::TActorBootstrappedPassAway(); } + if (!AppData()->FeatureFlags.GetEnableMetadataObjectsOnServerless() && Context.GetActivityType() != IOperationsManager::EActivityType::Drop) { + TBase::Register(CreateDatabaseFetcherActor(Context.GetExternalData().GetDatabase())); + } else { + CreateSession(); + } + } + + void Handle(TEvFetchDatabaseResponse::TPtr& ev) { + TString errorMessage; + if (const auto& errorString = ev->Get()->GetErrorString()) { + errorMessage = TStringBuilder() << "cannot fetch database '" << Context.GetExternalData().GetDatabase() << "': " << errorString; + } else if (ev->Get()->GetServerless()) { + errorMessage = TStringBuilder() << "objects " << TObject::GetTypeId() << " are disabled for serverless domains. Please contact your system administrator to enable it"; + } + + if (errorMessage) { + auto g = TBase::PassAwayGuard(); + ExternalController->OnAlteringProblem(TStringBuilder() << "Objects " << TObject::GetTypeId() << " are disabled for serverless domains. Please contact your system administrator to enable it"); + } else { + CreateSession(); + } + } + + void CreateSession() const { TBase::Register(new NRequest::TYDBCallbackRequest( NRequest::TDialogCreateSession::TRequest(), UserToken, TBase::SelfId())); } diff --git a/ydb/services/metadata/manager/common.h b/ydb/services/metadata/manager/common.h index b10bccb00ae1..d6fbf7510236 100644 --- a/ydb/services/metadata/manager/common.h +++ b/ydb/services/metadata/manager/common.h @@ -65,6 +65,7 @@ enum EEvents { EvAlterProblem, EvAlterPreparationFinished, EvAlterPreparationProblem, + EvFetchDatabaseResponse, EvEnd }; static_assert(EEvents::EvEnd < EventSpaceEnd(TKikimrEvents::ES_METADATA_MANAGER), "expect EvEnd < EventSpaceEnd(TKikimrEvents::ES_METADATA_MANAGER)"); diff --git a/ydb/services/metadata/manager/fetch_database.cpp b/ydb/services/metadata/manager/fetch_database.cpp new file mode 100644 index 000000000000..03d8568034a9 --- /dev/null +++ b/ydb/services/metadata/manager/fetch_database.cpp @@ -0,0 +1,112 @@ +#include "fetch_database.h" + +#include + +#include + +#include + + +namespace NKikimr::NMetadata::NModifications { + +namespace { + +class TDatabaseFetcherActor : public TActorBootstrapped { + using TBase = TActorBootstrapped; + using TRetryPolicy = IRetryPolicy<>; + +public: + explicit TDatabaseFetcherActor(const TString& database) + : Database(database) + {} + + void Registered(TActorSystem* sys, const TActorId& owner) override { + TBase::Registered(sys, owner); + Owner = owner; + } + + void Bootstrap() { + StartRequest(); + Become(&TDatabaseFetcherActor::StateFunc); + } + + void Handle(TEvents::TEvUndelivered::TPtr& ev) { + if (ev->Get()->Reason == NActors::TEvents::TEvUndelivered::ReasonActorUnknown && ScheduleRetry()) { + return; + } + + Reply("Scheme cache is unavailable"); + } + + void Handle(TEvTxProxySchemeCache::TEvNavigateKeySetResult::TPtr& ev) { + const auto& results = ev->Get()->Request->ResultSet; + Y_ABORT_UNLESS(results.size() == 1); + + const auto& result = results[0]; + if (result.DomainInfo) { + Serverless = result.DomainInfo->IsServerless(); + Reply(); + return; + } + + if (result.Status == NSchemeCache::TSchemeCacheNavigate::EStatus::LookupError && ScheduleRetry()) { + return; + } + + Reply(TStringBuilder() << "Failed to fetch database info: " << result.Status); + } + + STRICT_STFUNC(StateFunc, + sFunc(TEvents::TEvWakeup, StartRequest); + hFunc(TEvents::TEvUndelivered, Handle); + hFunc(TEvTxProxySchemeCache::TEvNavigateKeySetResult, Handle); + ) + +private: + void StartRequest() { + auto event = NTableCreator::BuildSchemeCacheNavigateRequest( + {{}}, + Database ? Database : AppData()->TenantName, + MakeIntrusive(BUILTIN_ACL_METADATA, TVector{}) + ); + event->ResultSet[0].Operation = NSchemeCache::TSchemeCacheNavigate::OpPath; + Send(MakeSchemeCacheID(), new TEvTxProxySchemeCache::TEvNavigateKeySet(event.Release()), IEventHandle::FlagTrackDelivery); + } + + bool ScheduleRetry() { + if (!RetryState) { + RetryState = TRetryPolicy::GetFixedIntervalPolicy( + [](){return ERetryErrorClass::ShortRetry;} + , TDuration::MilliSeconds(100) + , TDuration::MilliSeconds(500) + , 100 + )->CreateRetryState();; + } + + if (const auto delay = RetryState->GetNextRetryDelay()) { + this->Schedule(*delay, new TEvents::TEvWakeup()); + return true; + } + + return false; + } + + void Reply(const TString& errorMessage = "") { + Send(Owner, new TEvFetchDatabaseResponse(Serverless, errorMessage)); + PassAway(); + } + +private: + const TString Database; + TActorId Owner; + TRetryPolicy::IRetryState::TPtr RetryState; + bool Serverless = false; +}; + +} // anonymous namespace + +IActor* CreateDatabaseFetcherActor(const TString& database) { + return new TDatabaseFetcherActor(database); +} + +} // NKikimr::NMetadata::NModifications diff --git a/ydb/services/metadata/manager/fetch_database.h b/ydb/services/metadata/manager/fetch_database.h new file mode 100644 index 000000000000..4f33d09bc0bc --- /dev/null +++ b/ydb/services/metadata/manager/fetch_database.h @@ -0,0 +1,24 @@ +#pragma once + +#include "common.h" + +#include + + +namespace NKikimr::NMetadata::NModifications { + +class TEvFetchDatabaseResponse : public TEventLocal { +private: + YDB_READONLY_DEF(bool, Serverless); + YDB_READONLY_DEF(TString, ErrorString); + +public: + explicit TEvFetchDatabaseResponse(bool serverless, const TString& errorString) + : Serverless(serverless) + , ErrorString(errorString) + {} +}; + +IActor* CreateDatabaseFetcherActor(const TString& database); + +} // NKikimr::NMetadata::NModifications diff --git a/ydb/services/metadata/manager/ya.make b/ydb/services/metadata/manager/ya.make index 2c29f61e9601..79beeb811c81 100644 --- a/ydb/services/metadata/manager/ya.make +++ b/ydb/services/metadata/manager/ya.make @@ -14,11 +14,13 @@ SRCS( ydb_value_operator.cpp modification_controller.cpp object.cpp + fetch_database.cpp ) PEERDIR( ydb/library/accessor ydb/library/actors/core + ydb/library/table_creator ydb/public/api/protos ydb/core/protos ydb/services/bg_tasks/abstract From 4a4866565696a05549757980253f629a4785cd88 Mon Sep 17 00:00:00 2001 From: Grigoriy Pisarenko Date: Thu, 5 Sep 2024 13:30:12 +0000 Subject: [PATCH 2/4] Added unit test for disable objects on serverless --- ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp | 40 +++++++++++++++++++ .../common/kqp_workload_service_ut_common.cpp | 1 + .../common/kqp_workload_service_ut_common.h | 1 + ydb/core/testlib/basics/feature_flags.h | 1 + ydb/services/metadata/manager/alter_impl.h | 6 +-- 5 files changed, 46 insertions(+), 3 deletions(-) diff --git a/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp b/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp index 735a563f7180..fc41e87f9721 100644 --- a/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp +++ b/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp @@ -7212,6 +7212,46 @@ Y_UNIT_TEST_SUITE(KqpScheme) { UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::GENERIC_ERROR, result.GetIssues().ToString()); UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToString(), "Classifier with name MyResourcePoolClassifier not found in database /Root"); } + + Y_UNIT_TEST(DisableMetadataObjectsOnServerless) { + auto ydb = NWorkload::TYdbSetupSettings() + .CreateSampleTenants(true) + .EnableMetadataObjectsOnServerless(false) + .Create(); + + auto checkDisabled = [](const auto& result) { + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), NYdb::EStatus::GENERIC_ERROR, result.GetIssues().ToString()); + UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToString(), "Objects SECRET are disabled for serverless domains. Please contact your system administrator to enable it"); + }; + + const auto& createSql = "CREATE OBJECT MySecretObject (TYPE SECRET) WITH (value=\"qwerty\");"; + const auto& alterSql = "ALTER OBJECT MySecretObject (TYPE SECRET) SET value = \"abcde\";"; + const auto& upsertSql = "UPSERT OBJECT MySecretObject (TYPE SECRET) WITH value = \"edcba\";"; + const auto& dropSql = "DROP OBJECT MySecretObject (TYPE SECRET);"; + + auto settings = NWorkload::TQueryRunnerSettings().PoolId(""); + + // Dedicated, enabled + settings.Database(ydb->GetSettings().GetDedicatedTenantName()).NodeIndex(1); + NWorkload::TSampleQueries::CheckSuccess(ydb->ExecuteQuery(createSql, settings)); + NWorkload::TSampleQueries::CheckSuccess(ydb->ExecuteQuery(alterSql, settings)); + NWorkload::TSampleQueries::CheckSuccess(ydb->ExecuteQuery(upsertSql, settings)); + NWorkload::TSampleQueries::CheckSuccess(ydb->ExecuteQuery(dropSql, settings)); + + // Shared, enabled + settings.Database(ydb->GetSettings().GetSharedTenantName()).NodeIndex(2); + NWorkload::TSampleQueries::CheckSuccess(ydb->ExecuteQuery(createSql, settings)); + NWorkload::TSampleQueries::CheckSuccess(ydb->ExecuteQuery(alterSql, settings)); + NWorkload::TSampleQueries::CheckSuccess(ydb->ExecuteQuery(upsertSql, settings)); + NWorkload::TSampleQueries::CheckSuccess(ydb->ExecuteQuery(dropSql, settings)); + + // Serverless, disabled + settings.Database(ydb->GetSettings().GetServerlessTenantName()).NodeIndex(2); + checkDisabled(ydb->ExecuteQuery(createSql, settings)); + checkDisabled(ydb->ExecuteQuery(alterSql, settings)); + checkDisabled(ydb->ExecuteQuery(upsertSql, settings)); + NWorkload::TSampleQueries::CheckSuccess(ydb->ExecuteQuery(dropSql, settings)); + } } Y_UNIT_TEST_SUITE(KqpOlapScheme) { diff --git a/ydb/core/kqp/workload_service/ut/common/kqp_workload_service_ut_common.cpp b/ydb/core/kqp/workload_service/ut/common/kqp_workload_service_ut_common.cpp index 26e49b7fb0c9..412116c51b7d 100644 --- a/ydb/core/kqp/workload_service/ut/common/kqp_workload_service_ut_common.cpp +++ b/ydb/core/kqp/workload_service/ut/common/kqp_workload_service_ut_common.cpp @@ -231,6 +231,7 @@ class TWorkloadServiceYdbSetup : public IYdbSetup { TAppConfig appConfig; appConfig.MutableFeatureFlags()->SetEnableResourcePools(Settings_.EnableResourcePools_); appConfig.MutableFeatureFlags()->SetEnableResourcePoolsOnServerless(Settings_.EnableResourcePoolsOnServerless_); + appConfig.MutableFeatureFlags()->SetEnableMetadataObjectsOnServerless(Settings_.EnableMetadataObjectsOnServerless_); appConfig.MutableFeatureFlags()->SetEnableResourcePoolsCounters(true); return appConfig; diff --git a/ydb/core/kqp/workload_service/ut/common/kqp_workload_service_ut_common.h b/ydb/core/kqp/workload_service/ut/common/kqp_workload_service_ut_common.h index 9c3b9e6ba773..037cd3e5373e 100644 --- a/ydb/core/kqp/workload_service/ut/common/kqp_workload_service_ut_common.h +++ b/ydb/core/kqp/workload_service/ut/common/kqp_workload_service_ut_common.h @@ -70,6 +70,7 @@ struct TYdbSetupSettings { FLUENT_SETTING_DEFAULT(bool, CreateSampleTenants, false); FLUENT_SETTING_DEFAULT(bool, EnableResourcePools, true); FLUENT_SETTING_DEFAULT(bool, EnableResourcePoolsOnServerless, false); + FLUENT_SETTING_DEFAULT(bool, EnableMetadataObjectsOnServerless, true); // Default pool settings FLUENT_SETTING_DEFAULT(TString, PoolId, "sample_pool_id"); diff --git a/ydb/core/testlib/basics/feature_flags.h b/ydb/core/testlib/basics/feature_flags.h index 1dbdab825a3c..5a310526f4bd 100644 --- a/ydb/core/testlib/basics/feature_flags.h +++ b/ydb/core/testlib/basics/feature_flags.h @@ -66,6 +66,7 @@ class TTestFeatureFlagsHolder { FEATURE_FLAG_SETTER(EnableGranularTimecast) FEATURE_FLAG_SETTER(EnablePgSyntax) FEATURE_FLAG_SETTER(EnableTieringInColumnShard) + FEATURE_FLAG_SETTER(EnableMetadataObjectsOnServerless) #undef FEATURE_FLAG_SETTER }; diff --git a/ydb/services/metadata/manager/alter_impl.h b/ydb/services/metadata/manager/alter_impl.h index a216f100b9a2..7a78ff393411 100644 --- a/ydb/services/metadata/manager/alter_impl.h +++ b/ydb/services/metadata/manager/alter_impl.h @@ -138,14 +138,14 @@ class TModificationActorImpl: public NActors::TActorBootstrappedGet()->GetErrorString()) { - errorMessage = TStringBuilder() << "cannot fetch database '" << Context.GetExternalData().GetDatabase() << "': " << errorString; + errorMessage = TStringBuilder() << "Cannot fetch database '" << Context.GetExternalData().GetDatabase() << "': " << errorString; } else if (ev->Get()->GetServerless()) { - errorMessage = TStringBuilder() << "objects " << TObject::GetTypeId() << " are disabled for serverless domains. Please contact your system administrator to enable it"; + errorMessage = TStringBuilder() << "Objects " << TObject::GetTypeId() << " are disabled for serverless domains. Please contact your system administrator to enable it"; } if (errorMessage) { auto g = TBase::PassAwayGuard(); - ExternalController->OnAlteringProblem(TStringBuilder() << "Objects " << TObject::GetTypeId() << " are disabled for serverless domains. Please contact your system administrator to enable it"); + ExternalController->OnAlteringProblem(errorMessage); } else { CreateSession(); } From 14fd6216ed9c7f96cb6555b7a615d4b525bead8a Mon Sep 17 00:00:00 2001 From: Grigoriy Pisarenko Date: Thu, 5 Sep 2024 13:49:18 +0000 Subject: [PATCH 3/4] Removed extra explicit --- ydb/services/metadata/manager/fetch_database.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/services/metadata/manager/fetch_database.h b/ydb/services/metadata/manager/fetch_database.h index 4f33d09bc0bc..4af15964f3a8 100644 --- a/ydb/services/metadata/manager/fetch_database.h +++ b/ydb/services/metadata/manager/fetch_database.h @@ -13,7 +13,7 @@ class TEvFetchDatabaseResponse : public TEventLocal Date: Fri, 6 Sep 2024 12:51:35 +0000 Subject: [PATCH 4/4] Fixed issues --- ydb/services/metadata/manager/alter_impl.h | 2 +- ydb/services/metadata/manager/fetch_database.cpp | 2 +- ydb/services/metadata/manager/fetch_database.h | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ydb/services/metadata/manager/alter_impl.h b/ydb/services/metadata/manager/alter_impl.h index 7a78ff393411..127c6d530954 100644 --- a/ydb/services/metadata/manager/alter_impl.h +++ b/ydb/services/metadata/manager/alter_impl.h @@ -138,7 +138,7 @@ class TModificationActorImpl: public NActors::TActorBootstrappedGet()->GetErrorString()) { - errorMessage = TStringBuilder() << "Cannot fetch database '" << Context.GetExternalData().GetDatabase() << "': " << errorString; + errorMessage = TStringBuilder() << "Cannot fetch database '" << Context.GetExternalData().GetDatabase() << "': " << *errorString; } else if (ev->Get()->GetServerless()) { errorMessage = TStringBuilder() << "Objects " << TObject::GetTypeId() << " are disabled for serverless domains. Please contact your system administrator to enable it"; } diff --git a/ydb/services/metadata/manager/fetch_database.cpp b/ydb/services/metadata/manager/fetch_database.cpp index 03d8568034a9..cb7ebfcc52f5 100644 --- a/ydb/services/metadata/manager/fetch_database.cpp +++ b/ydb/services/metadata/manager/fetch_database.cpp @@ -91,7 +91,7 @@ class TDatabaseFetcherActor : public TActorBootstrapped { return false; } - void Reply(const TString& errorMessage = "") { + void Reply(const std::optional& errorMessage = std::nullopt) { Send(Owner, new TEvFetchDatabaseResponse(Serverless, errorMessage)); PassAway(); } diff --git a/ydb/services/metadata/manager/fetch_database.h b/ydb/services/metadata/manager/fetch_database.h index 4af15964f3a8..819fd5d4503f 100644 --- a/ydb/services/metadata/manager/fetch_database.h +++ b/ydb/services/metadata/manager/fetch_database.h @@ -10,10 +10,10 @@ namespace NKikimr::NMetadata::NModifications { class TEvFetchDatabaseResponse : public TEventLocal { private: YDB_READONLY_DEF(bool, Serverless); - YDB_READONLY_DEF(TString, ErrorString); + YDB_READONLY_DEF(std::optional, ErrorString); public: - TEvFetchDatabaseResponse(bool serverless, const TString& errorString) + TEvFetchDatabaseResponse(bool serverless, const std::optional& errorString) : Serverless(serverless) , ErrorString(errorString) {}