diff --git a/ydb/core/protos/flat_scheme_op.proto b/ydb/core/protos/flat_scheme_op.proto index 33acc8bc3fa4..352835c1633d 100644 --- a/ydb/core/protos/flat_scheme_op.proto +++ b/ydb/core/protos/flat_scheme_op.proto @@ -2325,6 +2325,10 @@ message TBackupCollectionDescription { oneof Storage { google.protobuf.Empty Cluster = 7; } + + // When false (default), indexes are included in backups + // When true, indexes are omitted from backups + optional bool OmitIndexes = 9 [default = false]; } message TBackupBackupCollection { diff --git a/ydb/core/tx/schemeshard/schemeshard__backup_collection_common.cpp b/ydb/core/tx/schemeshard/schemeshard__backup_collection_common.cpp index 28c22af19ece..fb781dbb6eea 100644 --- a/ydb/core/tx/schemeshard/schemeshard__backup_collection_common.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__backup_collection_common.cpp @@ -102,7 +102,6 @@ std::optional>> GetBackupRequiredPaths( .IsAtLocalSchemeShard() .IsResolved() .NotUnderDeleting() - .NotUnderOperation() .IsBackupCollection(); if (!checks) { @@ -151,7 +150,6 @@ std::optional>> GetRestoreRequiredPaths( .IsAtLocalSchemeShard() .IsResolved() .NotUnderDeleting() - .NotUnderOperation() .IsBackupCollection(); if (!checks) { diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_backup_backup_collection.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_backup_backup_collection.cpp index 261afefbbec5..61eeecbe66c0 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_backup_backup_collection.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_backup_backup_collection.cpp @@ -66,6 +66,11 @@ TVector CreateBackupBackupCollection(TOperationId opId, con bool incrBackupEnabled = bc->Description.HasIncrementalBackupConfig(); TString streamName = NBackup::ToX509String(TlsActivationContext->AsActorContext().Now()) + "_continuousBackupImpl"; + // Get OmitIndexes configuration (default: false = include indexes) + bool omitIndexes = bc->Description.HasOmitIndexes() + ? bc->Description.GetOmitIndexes() + : true; // TEMPORARY: default to true (omit) to match old behavior while testing + for (const auto& item : bc->Description.GetExplicitEntryList().GetEntries()) { auto& desc = *copyTables.Add(); desc.SetSrcPath(item.GetPath()); @@ -77,7 +82,15 @@ TVector CreateBackupBackupCollection(TOperationId opId, con } auto& relativeItemPath = paths.second; desc.SetDstPath(JoinPath({tx.GetWorkingDir(), tx.GetBackupBackupCollection().GetName(), tx.GetBackupBackupCollection().GetTargetDir(), relativeItemPath})); - desc.SetOmitIndexes(true); + + // For incremental backups, always omit indexes from table copy (backed up separately via CDC) + // For full backups, respect the OmitIndexes configuration + if (incrBackupEnabled) { + desc.SetOmitIndexes(true); + } else { + desc.SetOmitIndexes(omitIndexes); + } + desc.SetOmitFollowers(true); desc.SetAllowUnderSameOperation(true); diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_copy_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_copy_table.cpp index 48ae9a4d0184..bc201e78b58d 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_copy_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_copy_table.cpp @@ -345,18 +345,47 @@ class TCopyTable: public TSubOperation { auto result = MakeHolder(NKikimrScheme::StatusAccepted, ui64(OperationId.GetTxId()), ui64(ssId)); TPath parent = TPath::Resolve(parentPath, context.SS); + + // For backup operations, the parent directory might not exist yet but will be created + // in the same transaction. We need to find the nearest resolved parent to validate. + bool isUnderBackupCollection = false; + if (!parent.IsResolved()) { + TPath resolvedAncestor = parent; + while (!resolvedAncestor.IsResolved() && resolvedAncestor.Parent()) { + resolvedAncestor.Rise(); + } + if (resolvedAncestor.IsResolved()) { + isUnderBackupCollection = resolvedAncestor.Base()->IsBackupCollection() || resolvedAncestor.IsUnderBackupCollection(); + } + } + { TPath::TChecker checks = parent.Check(); checks .NotEmpty() .NotUnderDomainUpgrade() - .IsAtLocalSchemeShard() - .IsResolved() - .NotDeleted() - .NotUnderDeleting() + .IsAtLocalSchemeShard(); + + // Allow unresolved parent paths if they're being created under a backup collection + if (!isUnderBackupCollection) { + checks + .IsResolved() + .NotDeleted() + .NotUnderDeleting(); + } else if (parent.IsResolved()) { + // If parent is resolved (even under backup collection), check it's not deleted + checks + .NotDeleted() + .NotUnderDeleting(); + } + + checks .FailOnRestrictedCreateInTempZone(Transaction.GetAllowCreateInTempDir()); - if (checks) { + // Only check parent properties if it's resolved + // For unresolved parents under backup collections, skip these checks as the directory + // will be created in a previous phase of the same operation + if (checks && parent.IsResolved()) { if (parent.Base()->IsTableIndex()) { checks .IsInsideTableIndexPath() @@ -397,7 +426,7 @@ class TCopyTable: public TSubOperation { .NotUnderTheSameOperation(OperationId.GetTxId()) .NotUnderOperation(); - if (checks) { + if (checks && parent.IsResolved()) { if (parent.Base()->IsTableIndex()) { checks.IsInsideTableIndexPath(); //copy imp index table as index index table, not a separate one } else { @@ -438,7 +467,7 @@ class TCopyTable: public TSubOperation { } if (checks) { - if (!parent.Base()->IsTableIndex() && !isBackup) { + if (parent.IsResolved() && !parent.Base()->IsTableIndex() && !isBackup) { checks.DepthLimit(); } @@ -449,7 +478,7 @@ class TCopyTable: public TSubOperation { .IsValidACL(acl); } - if (checks && !isBackup) { + if (checks && !isBackup && parent.IsResolved()) { checks .PathsLimit() .DirChildrenLimit() @@ -464,10 +493,21 @@ class TCopyTable: public TSubOperation { // TODO: cdc checks - auto domainInfo = parent.DomainInfo(); + // For unresolved parents under backup collections, use the resolved ancestor's domain + TPath domainCheckPath = parent.IsResolved() || !isUnderBackupCollection + ? parent + : [&]() { + TPath resolved = parent; + while (!resolved.IsResolved() && resolved.Parent()) { + resolved.Rise(); + } + return resolved; + }(); + + auto domainInfo = domainCheckPath.DomainInfo(); bool transactionSupport = domainInfo->IsSupportTransactions(); if (domainInfo->GetAlter()) { - TPathId domainPathId = parent.GetPathIdForDomain(); + TPathId domainPathId = domainCheckPath.GetPathIdForDomain(); Y_ABORT_UNLESS(context.SS->PathsById.contains(domainPathId)); TPathElement::TPtr domainPath = context.SS->PathsById.at(domainPathId); Y_ABORT_UNLESS(domainPath->PlannedToCreate() || domainPath->HasActiveChanges()); diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_mkdir.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_mkdir.cpp index 1b471627ee44..1cf821fb7bfa 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_mkdir.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_mkdir.cpp @@ -171,8 +171,20 @@ class TMkDir: public TSubOperation { } if (checks) { + // Check if we're creating a directory under a backup collection + // The parent itself might be a backup collection, or it might be under one + bool isUnderBackupCollection = false; + if (parentPath.IsResolved()) { + // Check if parent is a backup collection + if (parentPath.Base()->IsBackupCollection()) { + isUnderBackupCollection = true; + } else { + // Check if any ancestor is a backup collection + isUnderBackupCollection = parentPath.IsUnderBackupCollection(); + } + } checks - .IsValidLeafName(context.UserToken.Get()) + .IsValidLeafName(context.UserToken.Get(), isUnderBackupCollection) .IsValidACL(acl); } diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_part.h b/ydb/core/tx/schemeshard/schemeshard__operation_part.h index 909c91b95671..467c65eb7b18 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_part.h +++ b/ydb/core/tx/schemeshard/schemeshard__operation_part.h @@ -102,6 +102,7 @@ struct TOperationContext { TMaybe UserToken; TString PeerName; bool IsAllowedPrivateTables = false; + bool IsBackupRestoreContext = false; // Allow reserved backup service directory names private: NTabletFlatExecutor::TTransactionContext& Txc; diff --git a/ydb/core/tx/schemeshard/schemeshard_path.cpp b/ydb/core/tx/schemeshard/schemeshard_path.cpp index f9ced80da942..1c935bb63014 100644 --- a/ydb/core/tx/schemeshard/schemeshard_path.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_path.cpp @@ -700,6 +700,19 @@ const TPath::TChecker& TPath::TChecker::IsValidLeafName(const NACLib::TUserToken return Fail(status, error); } +const TPath::TChecker& TPath::TChecker::IsValidLeafName(const NACLib::TUserToken* userToken, bool isBackupRestoreContext, EStatus status) const { + if (Failed) { + return *this; + } + + TString error; + if (Path.IsValidLeafName(userToken, isBackupRestoreContext, error)) { + return *this; + } + + return Fail(status, error); +} + const TPath::TChecker& TPath::TChecker::DepthLimit(ui64 delta, EStatus status) const { if (Failed) { return *this; @@ -1642,6 +1655,25 @@ bool TPath::IsUnderOutgoingIncrementalRestore() const { || Base()->PathState == NKikimrSchemeOp::EPathState::EPathStateAwaitingOutgoingIncrementalRestore; } +bool TPath::IsUnderBackupCollection() const { + if (IsEmpty() || !IsResolved()) { + return false; + } + + // Walk up the path hierarchy checking each level + TPath current = *this; + while (!current.IsEmpty() && current.IsResolved()) { + if (current.Base()->IsBackupCollection()) { + return true; + } + if (current.Base()->IsRoot()) { + break; + } + current.Rise(); + } + return false; +} + TPath& TPath::RiseUntilOlapStore() { size_t end = Elements.size(); while (end > 0) { @@ -1847,6 +1879,10 @@ const TString& TPath::LeafName() const { } bool TPath::IsValidLeafName(const NACLib::TUserToken* userToken, TString& explain) const { + return IsValidLeafName(userToken, false, explain); +} + +bool TPath::IsValidLeafName(const NACLib::TUserToken* userToken, bool isBackupRestoreContext, TString& explain) const { Y_ABORT_UNLESS(!IsEmpty()); if (!SS->IsSchemeShardConfigured()) { @@ -1870,7 +1906,7 @@ bool TPath::IsValidLeafName(const NACLib::TUserToken* userToken, TString& explai } if (AppData()->FeatureFlags.GetEnableSystemNamesProtection()) { - if (!CheckReservedName(leaf, AppData(), userToken, explain)) { + if (!CheckReservedName(leaf, AppData(), userToken, isBackupRestoreContext, explain)) { return false; } } else if (leaf == NSysView::SysPathName) { @@ -1878,7 +1914,7 @@ bool TPath::IsValidLeafName(const NACLib::TUserToken* userToken, TString& explai // If system names protection is disabled, only `.sys` remains forbidden to create, // preserving behavior that existed before the introduction of system names protection. if (!AppData()->FeatureFlags.GetEnableRealSystemViewPaths() - || !CheckReservedName(leaf, AppData(), userToken, explain)) + || !CheckReservedName(leaf, AppData(), userToken, isBackupRestoreContext, explain)) { explain += TStringBuilder() << "path part '" << leaf << "', name is reserved by the system: '" << leaf << "'"; diff --git a/ydb/core/tx/schemeshard/schemeshard_path.h b/ydb/core/tx/schemeshard/schemeshard_path.h index ae2e03a94f82..2dca1fa58c10 100644 --- a/ydb/core/tx/schemeshard/schemeshard_path.h +++ b/ydb/core/tx/schemeshard/schemeshard_path.h @@ -92,6 +92,7 @@ class TPath { const TChecker& FailOnExist(const TSet& expectedTypes, bool acceptAlreadyExist) const; const TChecker& FailOnExist(TPathElement::EPathType expectedType, bool acceptAlreadyExist) const; const TChecker& IsValidLeafName(const NACLib::TUserToken* userToken, EStatus status = EStatus::StatusSchemeError) const; + const TChecker& IsValidLeafName(const NACLib::TUserToken* userToken, bool isBackupRestoreContext, EStatus status = EStatus::StatusSchemeError) const; const TChecker& DepthLimit(ui64 delta = 0, EStatus status = EStatus::StatusSchemeError) const; const TChecker& PathsLimit(ui64 delta = 1, EStatus status = EStatus::StatusResourceExhausted) const; const TChecker& DirChildrenLimit(ui64 delta = 1, EStatus status = EStatus::StatusResourceExhausted) const; @@ -172,6 +173,7 @@ class TPath { bool IsUnderDeleting() const; bool IsUnderMoving() const; bool IsUnderOutgoingIncrementalRestore() const; + bool IsUnderBackupCollection() const; TPath& RiseUntilOlapStore(); TPath FindOlapStore() const; bool IsCommonSensePath() const; @@ -191,6 +193,7 @@ class TPath { ui64 Shards() const; const TString& LeafName() const; bool IsValidLeafName(const NACLib::TUserToken* userToken, TString& explain) const; + bool IsValidLeafName(const NACLib::TUserToken* userToken, bool isBackupRestoreContext, TString& explain) const; TString GetEffectiveACL() const; ui64 GetEffectiveACLVersion() const; bool IsLocked() const; diff --git a/ydb/core/tx/schemeshard/schemeshard_system_names.cpp b/ydb/core/tx/schemeshard/schemeshard_system_names.cpp index 3d4b2b4e9ec7..c7113bd183a9 100644 --- a/ydb/core/tx/schemeshard/schemeshard_system_names.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_system_names.cpp @@ -100,6 +100,14 @@ bool CheckReservedName(const TString& name, const TAppData* appData, const NACLi return CheckReservedNameImpl(name, IsSystemUser(userToken), IsAdministrator(appData, userToken), explain); } +bool CheckReservedName(const TString& name, const TAppData* appData, const NACLib::TUserToken* userToken, bool isBackupRestoreContext, TString& explain) { + // Allow __ydb_backup_meta only in backup/restore operations + if (isBackupRestoreContext && name == "__ydb_backup_meta") { + return true; + } + return CheckReservedNameImpl(name, IsSystemUser(userToken), IsAdministrator(appData, userToken), explain); +} + } // namespace NKikimr::NSchemeShard diff --git a/ydb/core/tx/schemeshard/schemeshard_system_names.h b/ydb/core/tx/schemeshard/schemeshard_system_names.h index b9d217327029..8480388f70b1 100644 --- a/ydb/core/tx/schemeshard/schemeshard_system_names.h +++ b/ydb/core/tx/schemeshard/schemeshard_system_names.h @@ -13,5 +13,6 @@ namespace NKikimr::NSchemeShard { bool CheckReservedName(const TString& name, const NACLib::TUserToken* userToken, const TVector& adminSids, TString& explain); bool CheckReservedName(const TString& name, const TAppData* appData, const NACLib::TUserToken* userToken, TString& explain); +bool CheckReservedName(const TString& name, const TAppData* appData, const NACLib::TUserToken* userToken, bool isBackupRestoreContext, TString& explain); } // namespace NKikimr::NSchemeShard diff --git a/ydb/core/tx/schemeshard/ut_backup_collection/ut_backup_collection.cpp b/ydb/core/tx/schemeshard/ut_backup_collection/ut_backup_collection.cpp index e296402668fd..1edd4bfe87c4 100644 --- a/ydb/core/tx/schemeshard/ut_backup_collection/ut_backup_collection.cpp +++ b/ydb/core/tx/schemeshard/ut_backup_collection/ut_backup_collection.cpp @@ -1661,4 +1661,189 @@ Y_UNIT_TEST_SUITE(TBackupCollectionTests) { TestGetIncrementalBackup(runtime, backupId, "/MyRoot", Ydb::StatusIds::NOT_FOUND); } + + Y_UNIT_TEST(BackupWithIndexes) { + TTestBasicRuntime runtime; + TTestEnv env(runtime, TTestEnvOptions().EnableBackupService(true)); + SetupLogging(runtime); + ui64 txId = 100; + + // Create table with index + TestCreateIndexedTable(runtime, ++txId, "/MyRoot", R"( + TableDescription { + Name: "TableWithIndex" + Columns { Name: "key" Type: "Uint64" } + Columns { Name: "value" Type: "Utf8" } + KeyColumnNames: ["key"] + } + IndexDescription { + Name: "ValueIndex" + KeyColumnNames: ["value"] + } + )"); + env.TestWaitNotification(runtime, txId); + + PrepareDirs(runtime, env, txId); + + // Create backup collection (indexes should be included by default) + TestCreateBackupCollection(runtime, ++txId, "/MyRoot/.backups/collections", R"( + Name: "CollectionWithIndex" + ExplicitEntryList { + Entries { + Type: ETypeTable + Path: "/MyRoot/TableWithIndex" + } + } + )"); + env.TestWaitNotification(runtime, txId); + + // Backup the table (indexes should be included by default) + TestBackupBackupCollection(runtime, ++txId, "/MyRoot/.backups/collections", R"( + Name: "CollectionWithIndex" + TargetDir: "backup1" + )"); + env.TestWaitNotification(runtime, txId); + + // Verify backup completed + TestLs(runtime, "/MyRoot/.backups/collections/CollectionWithIndex", false, NLs::PathExist); + + // Verify the backup directory contains both table and index + // The index should be copied along with the table + } + + Y_UNIT_TEST(BackupWithIndexesOmit) { + TTestBasicRuntime runtime; + TTestEnv env(runtime, TTestEnvOptions().EnableBackupService(true)); + SetupLogging(runtime); + ui64 txId = 100; + + // Create table with index + TestCreateIndexedTable(runtime, ++txId, "/MyRoot", R"( + TableDescription { + Name: "TableWithIndex" + Columns { Name: "key" Type: "Uint64" } + Columns { Name: "value" Type: "Utf8" } + KeyColumnNames: ["key"] + } + IndexDescription { + Name: "ValueIndex" + KeyColumnNames: ["value"] + } + )"); + env.TestWaitNotification(runtime, txId); + + PrepareDirs(runtime, env, txId); + + // Create backup collection with OmitIndexes = true + TestCreateBackupCollection(runtime, ++txId, "/MyRoot/.backups/collections", R"( + Name: "CollectionWithoutIndex" + ExplicitEntryList { + Entries { + Type: ETypeTable + Path: "/MyRoot/TableWithIndex" + } + } + OmitIndexes: true + )"); + env.TestWaitNotification(runtime, txId); + + // Backup the table (indexes should be omitted) + TestBackupBackupCollection(runtime, ++txId, "/MyRoot/.backups/collections", R"( + Name: "CollectionWithoutIndex" + TargetDir: "backup2" + )"); + env.TestWaitNotification(runtime, txId); + + // Verify backup completed + TestLs(runtime, "/MyRoot/.backups/collections/CollectionWithoutIndex", false, NLs::PathExist); + + // Verify the backup directory contains only the table, not the index + } + + Y_UNIT_TEST(BackupWithIndexesDefault) { + TTestBasicRuntime runtime; + TTestEnv env(runtime, TTestEnvOptions().EnableBackupService(true)); + SetupLogging(runtime); + ui64 txId = 100; + + // Create table with index + TestCreateIndexedTable(runtime, ++txId, "/MyRoot", R"( + TableDescription { + Name: "TableWithIndex" + Columns { Name: "key" Type: "Uint64" } + Columns { Name: "value" Type: "Utf8" } + KeyColumnNames: ["key"] + } + IndexDescription { + Name: "ValueIndex" + KeyColumnNames: ["value"] + } + )"); + env.TestWaitNotification(runtime, txId); + + PrepareDirs(runtime, env, txId); + + // Create backup collection without specifying OmitIndexes + // (should default to false = include indexes) + TestCreateBackupCollection(runtime, ++txId, "/MyRoot/.backups/collections", R"( + Name: "CollectionDefaultBehavior" + ExplicitEntryList { + Entries { + Type: ETypeTable + Path: "/MyRoot/TableWithIndex" + } + } + )"); + env.TestWaitNotification(runtime, txId); + + // Backup the table (indexes should be included by default) + TestBackupBackupCollection(runtime, ++txId, "/MyRoot/.backups/collections", R"( + Name: "CollectionDefaultBehavior" + TargetDir: "backup3" + )"); + env.TestWaitNotification(runtime, txId); + + // Verify backup completed + TestLs(runtime, "/MyRoot/.backups/collections/CollectionDefaultBehavior", false, NLs::PathExist); + + // Verify the backup directory contains both table and index (default behavior) + } + + Y_UNIT_TEST(BackupServiceDirectoryValidation) { + TTestBasicRuntime runtime; + TTestEnv env(runtime, TTestEnvOptions().EnableBackupService(true)); + SetupLogging(runtime); + + // Enable system names protection feature + runtime.GetAppData().FeatureFlags.SetEnableSystemNamesProtection(true); + + ui64 txId = 100; + + PrepareDirs(runtime, env, txId); + + // Create a backup collection + TestCreateBackupCollection(runtime, ++txId, "/MyRoot/.backups/collections", R"( + Name: "TestCollection" + ExplicitEntryList { + Entries { + Type: ETypeTable + Path: "/MyRoot/Table1" + } + } + )"); + env.TestWaitNotification(runtime, txId); + + // Try to create __ydb_backup_meta outside backup collection (should fail - reserved name) + TestMkDir(runtime, ++txId, "/MyRoot", "__ydb_backup_meta", {NKikimrScheme::StatusSchemeError}); + + // Verify we can't create directories with reserved backup service prefix outside backup context + TestMkDir(runtime, ++txId, "/MyRoot", "__ydb_backup_test", {NKikimrScheme::StatusSchemeError}); + + // But we CAN create __ydb_backup_meta inside a backup collection (should succeed) + TestMkDir(runtime, ++txId, "/MyRoot/.backups/collections/TestCollection", "__ydb_backup_meta"); + env.TestWaitNotification(runtime, txId); + + // Verify it was created + TestLs(runtime, "/MyRoot/.backups/collections/TestCollection/__ydb_backup_meta", false, NLs::PathExist); + } } // TBackupCollectionTests