Skip to content

Commit 0197f82

Browse files
committed
Fix index discovery for nested tables and tighten path matching
Addresses two critical PR review comments: 1. Fix nested table path discovery (CRITICAL BUG): - Previous: Only looked 1 level deep in __ydb_backup_meta/indexes/ - Impact: Tables in subdirectories (e.g. /Root/Dir/Foo) had indexes silently skipped - Fix: Implemented recursive DiscoverIndexesRecursive() that: * Traverses the full directory tree under indexes/ * Accumulates relative path as it descends (Root -> Root/Dir -> Root/Dir/Foo) * Matches against target tables at each level * Creates index operations when match found Example: Backup structure: __ydb_backup_meta/indexes/Root/Dir/Foo/index1 Old behavior: Only saw "Root", failed to match, skipped all indexes New behavior: Descends to "Root/Dir/Foo", matches table, restores indexes 2. Fix path matching to prevent false positives: - Previous: itemPath.EndsWith(relativeTablePath) - Problem: /Root/FooBar matched "Bar", /Root/users_table matched "table" - Fix: Only accept exact match OR suffix preceded by "/" * itemPath == relativeTablePath * itemPath.EndsWith("/" + relativeTablePath) Changes: - Added DiscoverIndexesRecursive() helper for recursive directory traversal - Updated FindTargetTablePath() to use exact matching logic - Updated DiscoverAndCreateIndexRestoreOperations() to call recursive helper Files modified: - schemeshard_impl.h: Added DiscoverIndexesRecursive() declaration - schemeshard_incremental_restore_scan.cpp: Implemented recursive discovery Both issues validated and confirmed as correct feedback.
1 parent d32e711 commit 0197f82

File tree

2 files changed

+70
-29
lines changed

2 files changed

+70
-29
lines changed

ydb/core/tx/schemeshard/schemeshard_impl.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,6 +1202,15 @@ class TSchemeShard
12021202
const TBackupCollectionInfo::TPtr& backupCollectionInfo,
12031203
const TActorContext& ctx);
12041204

1205+
void DiscoverIndexesRecursive(
1206+
ui64 operationId,
1207+
const TString& backupName,
1208+
const TPath& bcPath,
1209+
const TBackupCollectionInfo::TPtr& backupCollectionInfo,
1210+
const TPath& currentPath,
1211+
const TString& accumulatedRelativePath,
1212+
const TActorContext& ctx);
1213+
12051214
void CreateSingleIndexRestoreOperation(
12061215
ui64 operationId,
12071216
const TString& backupName,

ydb/core/tx/schemeshard/schemeshard_incremental_restore_scan.cpp

Lines changed: 61 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -622,16 +622,64 @@ TString TSchemeShard::FindTargetTablePath(
622622
// Item path is like /Root/db/table1, we need to extract the relative part
623623
TString itemPath = item.GetPath();
624624

625-
// Find the last path component(s) to match with relativeTablePath
626-
// relativeTablePath might be "table1" or "folder/table1"
627-
if (itemPath.EndsWith(relativeTablePath) || itemPath.EndsWith("/" + relativeTablePath)) {
625+
// Only accept exact matches or suffixes preceded by path separator
626+
// to avoid false matches (e.g. "/Root/FooBar" should not match "Bar")
627+
if (itemPath == relativeTablePath || itemPath.EndsWith("/" + relativeTablePath)) {
628628
return itemPath;
629629
}
630630
}
631631

632632
return {};
633633
}
634634

635+
void TSchemeShard::DiscoverIndexesRecursive(
636+
ui64 operationId,
637+
const TString& backupName,
638+
const TPath& bcPath,
639+
const TBackupCollectionInfo::TPtr& backupCollectionInfo,
640+
const TPath& currentPath,
641+
const TString& accumulatedRelativePath,
642+
const TActorContext& ctx) {
643+
644+
// Try to find target table for current accumulated path
645+
TString targetTablePath = FindTargetTablePath(backupCollectionInfo, accumulatedRelativePath);
646+
647+
if (!targetTablePath.empty()) {
648+
// Found target table, children are indexes
649+
LOG_I("Found table mapping: " << accumulatedRelativePath << " -> " << targetTablePath);
650+
651+
for (const auto& [indexName, indexDirPathId] : currentPath.Base()->GetChildren()) {
652+
CreateSingleIndexRestoreOperation(
653+
operationId,
654+
backupName,
655+
bcPath,
656+
accumulatedRelativePath,
657+
indexName,
658+
targetTablePath,
659+
ctx
660+
);
661+
}
662+
} else {
663+
// Not a table yet, descend into children to build up the path
664+
for (const auto& [childName, childPathId] : currentPath.Base()->GetChildren()) {
665+
auto childPath = TPath::Init(childPathId, this);
666+
TString newRelativePath = accumulatedRelativePath.empty()
667+
? childName
668+
: accumulatedRelativePath + "/" + childName;
669+
670+
DiscoverIndexesRecursive(
671+
operationId,
672+
backupName,
673+
bcPath,
674+
backupCollectionInfo,
675+
childPath,
676+
newRelativePath,
677+
ctx
678+
);
679+
}
680+
}
681+
}
682+
635683
void TSchemeShard::DiscoverAndCreateIndexRestoreOperations(
636684
const TPathId& backupCollectionPathId,
637685
ui64 operationId,
@@ -663,32 +711,16 @@ void TSchemeShard::DiscoverAndCreateIndexRestoreOperations(
663711

664712
LOG_I("Discovering indexes for restore at: " << indexMetaBasePath);
665713

666-
// Iterate through table directories under indexes/
667-
for (const auto& [relativeTablePath, tableMetaDirPathId] : indexMetaPath.Base()->GetChildren()) {
668-
auto tableMetaDirPath = TPath::Init(tableMetaDirPathId, this);
669-
670-
// Find corresponding target table
671-
TString targetTablePath = FindTargetTablePath(backupCollectionInfo, relativeTablePath);
672-
if (targetTablePath.empty()) {
673-
LOG_W("Could not find target table for relative path: " << relativeTablePath);
674-
continue;
675-
}
676-
677-
LOG_I("Found table mapping: " << relativeTablePath << " -> " << targetTablePath);
678-
679-
// For each index under this table
680-
for (const auto& [indexName, indexDirPathId] : tableMetaDirPath.Base()->GetChildren()) {
681-
CreateSingleIndexRestoreOperation(
682-
operationId,
683-
backupName,
684-
bcPath,
685-
relativeTablePath,
686-
indexName,
687-
targetTablePath,
688-
ctx
689-
);
690-
}
691-
}
714+
// Start recursive discovery from the indexes root with empty accumulated path
715+
DiscoverIndexesRecursive(
716+
operationId,
717+
backupName,
718+
bcPath,
719+
backupCollectionInfo,
720+
indexMetaPath,
721+
"", // Start with empty accumulated path
722+
ctx
723+
);
692724
}
693725

694726
void TSchemeShard::CreateSingleIndexRestoreOperation(

0 commit comments

Comments
 (0)