Skip to content

Commit a85488c

Browse files
jepett0jepett0
authored
Bugfix in ydb tools restore --replace, do not throw an error if the object is missing from the database (#18560)
1 parent 149321f commit a85488c

File tree

5 files changed

+156
-32
lines changed

5 files changed

+156
-32
lines changed

‎ydb/public/lib/ydb_cli/commands/ydb_tools.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ int TCommandRestore::Run(TConfig& config) {
279279

280280
if (VerifyExistence && !Replace) {
281281
throw TMisuseException()
282-
<< "the --verify-existence option must be used together with the --replace option";
282+
<< "The --verify-existence option must be used together with the --replace option.";
283283
}
284284

285285
auto log = std::make_shared<TLog>(CreateLogBackend("cerr", TConfig::VerbosityLevelToELogPriority(config.VerbosityLevel)));

‎ydb/public/lib/ydb_cli/dump/restore_impl.cpp

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,9 +1115,12 @@ TRestoreResult TRestoreClient::Restore(NScheme::ESchemeEntryType type, const TFs
11151115

11161116
}
11171117

1118-
TRestoreResult TRestoreClient::DropAndRestoreExternals(const TVector<TFsBackupEntry>& backupEntries, const TVector<size_t>& externalDataSources, const THashMap<TString, size_t>& externalTables, const TRestoreSettings& settings) {
1118+
TRestoreResult TRestoreClient::DropAndRestoreExternals(const TVector<TFsBackupEntry>& backupEntries, const TVector<size_t>& externalDataSources, const THashMap<TString, size_t>& externalTables, const TRestoreSettings& settings, const THashMap<TString, ESchemeEntryType>& existingEntries) {
11191119
for (size_t i : externalDataSources) {
11201120
const auto& [fsPath, dbPath, type] = backupEntries[i];
1121+
if (!existingEntries.contains(dbPath)) {
1122+
continue;
1123+
}
11211124
TVector<TString> references;
11221125
if (auto status = GetExternalTablesReferencingSource(TableClient, dbPath, references); !status.IsSuccess()) {
11231126
return Result<TRestoreResult>(fsPath, std::move(status));
@@ -1132,15 +1135,19 @@ TRestoreResult TRestoreClient::DropAndRestoreExternals(const TVector<TFsBackupEn
11321135
}
11331136

11341137
for (const auto& [dbPath, i] : externalTables) {
1135-
auto result = Drop(ESchemeEntryType::ExternalTable, dbPath, settings);
1136-
if (!result.IsSuccess()) {
1137-
return result;
1138+
if (existingEntries.contains(dbPath)) {
1139+
auto result = Drop(ESchemeEntryType::ExternalTable, dbPath, settings);
1140+
if (!result.IsSuccess()) {
1141+
return result;
1142+
}
11381143
}
11391144
}
11401145
for (size_t i : externalDataSources) {
11411146
const auto& [fsPath, dbPath, type] = backupEntries[i];
1142-
if (auto result = Drop(type, dbPath, settings); !result.IsSuccess()) {
1143-
return result;
1147+
if (existingEntries.contains(dbPath)) {
1148+
if (auto result = Drop(type, dbPath, settings); !result.IsSuccess()) {
1149+
return result;
1150+
}
11441151
}
11451152
if (auto result = RestoreExternalDataSource(fsPath, dbPath, settings, false); !result.IsSuccess()) {
11461153
return result;
@@ -1178,22 +1185,28 @@ TRestoreResult TRestoreClient::DropAndRestoreTablesAndDependents(const TVector<T
11781185

11791186
for (size_t i : views) {
11801187
const auto& [fsPath, dbPath, type] = backupEntries[i];
1181-
if (auto result = Drop(type, dbPath, settings); !result.IsSuccess()) {
1182-
return result;
1188+
if (existingEntries.contains(dbPath)) {
1189+
if (auto result = Drop(type, dbPath, settings); !result.IsSuccess()) {
1190+
return result;
1191+
}
11831192
}
11841193
}
11851194

11861195
for (const auto& [dbPath, i] : replications) {
1187-
if (auto result = Drop(ESchemeEntryType::Replication, dbPath, settings); !result.IsSuccess()) {
1188-
return result;
1196+
if (existingEntries.contains(dbPath)) {
1197+
if (auto result = Drop(ESchemeEntryType::Replication, dbPath, settings); !result.IsSuccess()) {
1198+
return result;
1199+
}
11891200
}
11901201
}
11911202

11921203
// the main loop: tables are restored here
11931204
for (const auto& [_, i] : tables) {
11941205
const auto& [fsPath, dbPath, type] = backupEntries[i];
1195-
if (auto result = Drop(type, dbPath, settings); !result.IsSuccess()) {
1196-
return result;
1206+
if (existingEntries.contains(dbPath)) {
1207+
if (auto result = Drop(type, dbPath, settings); !result.IsSuccess()) {
1208+
return result;
1209+
}
11971210
}
11981211
if (auto result = RestoreTable(fsPath, dbPath, settings, false); !result.IsSuccess()) {
11991212
return result;
@@ -1289,7 +1302,7 @@ TRestoreResult TRestoreClient::DropAndRestore(const TFsPath& fsBackupRoot, const
12891302
}
12901303
}
12911304

1292-
if (auto result = DropAndRestoreExternals(backupEntries, externalDataSources, externalTables, settings); !result.IsSuccess()) {
1305+
if (auto result = DropAndRestoreExternals(backupEntries, externalDataSources, externalTables, settings, existingEntries); !result.IsSuccess()) {
12931306
return result;
12941307
}
12951308
if (auto result = DropAndRestoreTablesAndDependents(backupEntries, tables, views, replications, dbRestoreRoot, settings, existingEntries); !result.IsSuccess()) {
@@ -1298,8 +1311,10 @@ TRestoreResult TRestoreClient::DropAndRestore(const TFsPath& fsBackupRoot, const
12981311

12991312
for (size_t i : regular) {
13001313
const auto& [fsPath, dbPath, type] = backupEntries[i];
1301-
if (auto result = Drop(type, dbPath, settings); !result.IsSuccess()) {
1302-
return result;
1314+
if (existingEntries.contains(dbPath)) {
1315+
if (auto result = Drop(type, dbPath, settings); !result.IsSuccess()) {
1316+
return result;
1317+
}
13031318
}
13041319
Y_ENSURE(dbPath.StartsWith(dbRestoreRoot), "dbPath must be built by appending a relative path to dbRestoreRoot");
13051320
if (auto result = Restore(type, fsPath, dbRestoreRoot, dbPath.substr(dbRestoreRoot.size()), settings, false, false); !result.IsSuccess()) {

‎ydb/public/lib/ydb_cli/dump/restore_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ class TRestoreClient {
237237
TRestoreResult Drop(NScheme::ESchemeEntryType type, const TString& path, const TRestoreSettings& settings);
238238
TRestoreResult Restore(NScheme::ESchemeEntryType type, const TFsPath& fsPath, const TString& dbRestoreRoot, const TString& dbPathRelativeToRestoreRoot, const TRestoreSettings& settings, bool isAlreadyExisting, bool delay);
239239
TRestoreResult DropAndRestore(const TFsPath& fsPath, const TString& dbRestoreRoot, const TRestoreSettings& settings, const THashMap<TString, NScheme::ESchemeEntryType>& existingEntries);
240-
TRestoreResult DropAndRestoreExternals(const TVector<NPrivate::TFsBackupEntry>& backupEntries, const TVector<size_t>& externalDataSources, const THashMap<TString, size_t>& externalTables, const TRestoreSettings& settings);
240+
TRestoreResult DropAndRestoreExternals(const TVector<NPrivate::TFsBackupEntry>& backupEntries, const TVector<size_t>& externalDataSources, const THashMap<TString, size_t>& externalTables, const TRestoreSettings& settings, const THashMap<TString, NScheme::ESchemeEntryType>& existingEntries);
241241
TRestoreResult DropAndRestoreTablesAndDependents(const TVector<NPrivate::TFsBackupEntry>& backupEntries, const THashMap<TString, size_t>& tables, const TVector<size_t>& views, const THashMap<TString, size_t>& replications, const TString& dbRestoreRoot, const TRestoreSettings& settings, const THashMap<TString, NScheme::ESchemeEntryType>& existingEntries);
242242

243243
public:

‎ydb/services/ydb/backup_ut/ydb_backup_ut.cpp

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2316,6 +2316,83 @@ Y_UNIT_TEST_SUITE(BackupRestore) {
23162316
);
23172317
}
23182318

2319+
Y_UNIT_TEST(TestReplaceRestoreOptionOnNonExistingSchemeObjects) {
2320+
NKikimrConfig::TAppConfig config;
2321+
config.MutableQueryServiceConfig()->AddAvailableExternalDataSources("ObjectStorage");
2322+
TKikimrWithGrpcAndRootSchema server(config);
2323+
2324+
server.GetRuntime()->GetAppData().FeatureFlags.SetEnableExternalDataSources(true);
2325+
2326+
const auto endpoint = Sprintf("localhost:%u", server.GetPort());
2327+
auto driver = TDriver(TDriverConfig().SetEndpoint(endpoint).SetAuthToken("root@builtin"));
2328+
2329+
TTableClient tableClient(driver);
2330+
auto tableSession = tableClient.GetSession().ExtractValueSync().GetSession();
2331+
NQuery::TQueryClient queryClient(driver);
2332+
auto querySession = queryClient.GetSession().ExtractValueSync().GetSession();
2333+
NTopic::TTopicClient topicClient(driver);
2334+
TReplicationClient replicationClient(driver);
2335+
NCoordination::TClient nodeClient(driver);
2336+
2337+
TTempDir tempDir;
2338+
const auto& pathToBackup = tempDir.Path();
2339+
2340+
auto cleanup = [&pathToBackup, &driver] {
2341+
TVector<TFsPath> children;
2342+
pathToBackup.List(children);
2343+
for (auto& i : children) {
2344+
i.ForceDelete();
2345+
}
2346+
2347+
const auto settings = NConsoleClient::TRemoveDirectoryRecursiveSettings().RemoveSelf(false);
2348+
RemoveDirectoryRecursive(driver, "/Root", settings);
2349+
};
2350+
2351+
constexpr const char* table = "/Root/table";
2352+
constexpr const char* topic = "/Root/topic";
2353+
constexpr const char* view = "/Root/view";
2354+
constexpr const char* externalTable = "/Root/externalTable";
2355+
constexpr const char* externalDataSource = "/Root/externalDataSource";
2356+
const std::string kesus = "/Root/kesus";
2357+
2358+
const auto restorationSettings = NDump::TRestoreSettings().Replace(true);
2359+
2360+
cleanup();
2361+
TestTableContentIsPreserved(table, tableSession,
2362+
CreateBackupLambda(driver, pathToBackup), CreateRestoreLambda(driver, pathToBackup, "/Root", restorationSettings)
2363+
);
2364+
2365+
cleanup();
2366+
TestTopicSettingsArePreserved(topic, querySession, topicClient,
2367+
CreateBackupLambda(driver, pathToBackup), CreateRestoreLambda(driver, pathToBackup, "/Root", restorationSettings)
2368+
);
2369+
2370+
cleanup();
2371+
TestViewOutputIsPreserved(view, querySession,
2372+
CreateBackupLambda(driver, pathToBackup), CreateRestoreLambda(driver, pathToBackup, "/Root", restorationSettings)
2373+
);
2374+
2375+
cleanup();
2376+
TestExternalDataSourceSettingsArePreserved(externalDataSource, tableSession, querySession,
2377+
CreateBackupLambda(driver, pathToBackup), CreateRestoreLambda(driver, pathToBackup, "/Root", restorationSettings)
2378+
);
2379+
2380+
cleanup();
2381+
TestExternalTableSettingsArePreserved(externalTable, externalDataSource, tableSession, querySession,
2382+
CreateBackupLambda(driver, pathToBackup), CreateRestoreLambda(driver, pathToBackup, "/Root", restorationSettings)
2383+
);
2384+
2385+
cleanup();
2386+
TestCoordinationNodeSettingsArePreserved(kesus, nodeClient,
2387+
CreateBackupLambda(driver, pathToBackup), CreateRestoreLambda(driver, pathToBackup, "/Root", restorationSettings)
2388+
);
2389+
2390+
cleanup();
2391+
TestReplicationSettingsArePreserved(endpoint, querySession, replicationClient,
2392+
CreateBackupLambda(driver, pathToBackup), CreateRestoreLambda(driver, pathToBackup, "/Root", restorationSettings), true
2393+
);
2394+
}
2395+
23192396
Y_UNIT_TEST(PrefixedVectorIndex) {
23202397
TestTableWithIndexBackupRestore(NKikimrSchemeOp::EIndexTypeGlobalVectorKmeansTree, true);
23212398
}

‎ydb/tests/functional/ydb_cli/test_ydb_backup.py

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1625,16 +1625,19 @@ def test_database_backup_restore(self):
16251625

16261626
class TestRestoreReplaceOption(BaseTestBackupInFiles):
16271627
def ydb_cli(self, args):
1628+
yatest.common.execute(
1629+
[
1630+
backup_bin(),
1631+
"-vvv",
1632+
"--endpoint", "grpc://localhost:%d" % self.cluster.nodes[1].grpc_port,
1633+
"--database", "/Root",
1634+
]
1635+
+ args
1636+
)
1637+
1638+
def try_ydb_cli(self, args):
16281639
try:
1629-
result = yatest.common.execute(
1630-
[
1631-
backup_bin(),
1632-
"-vvv",
1633-
"--endpoint", "grpc://localhost:%d" % self.cluster.nodes[1].grpc_port,
1634-
"--database", "/Root",
1635-
]
1636-
+ args
1637-
)
1640+
result = self.ydb_cli(args)
16381641
return True, result.std_out.decode("utf-8")
16391642

16401643
except yatest.common.process.ExecutionError as exception:
@@ -1659,7 +1662,12 @@ def test_table_replacement(self):
16591662
assert_that(os.listdir(backup_files_dir), is_(["table"]))
16601663

16611664
# Restore the table
1662-
self.ydb_cli(["tools", "restore", "--path", "./restoration/point", "--input", backup_files_dir, "--dry-run"])
1665+
assert_that(
1666+
self.try_ydb_cli(
1667+
["tools", "restore", "--path", "./restoration/point", "--input", backup_files_dir, "--dry-run"]
1668+
)[0],
1669+
is_(False),
1670+
)
16631671
assert_that(
16641672
[child.name for child in self.driver.scheme_client.list_directory("/Root").children],
16651673
is_not(has_items("restoration")),
@@ -1688,9 +1696,9 @@ def test_table_replacement(self):
16881696
# Drop the folder
16891697
self.ydb_cli(["scheme", "rmdir", "--force", "--recursive", "./restoration/point"])
16901698

1691-
# Try to replace the table with the --verify-existence option turned on
1699+
# Replace a non-existent table with the --verify-existence option turned on
16921700
assert_that(
1693-
self.ydb_cli(
1701+
self.try_ydb_cli(
16941702
[
16951703
"tools",
16961704
"restore",
@@ -1701,10 +1709,10 @@ def test_table_replacement(self):
17011709
"--dry-run",
17021710
]
17031711
)[0],
1704-
equal_to(False),
1712+
is_(False),
17051713
)
17061714
assert_that(
1707-
self.ydb_cli(
1715+
self.try_ydb_cli(
17081716
[
17091717
"tools",
17101718
"restore",
@@ -1714,7 +1722,7 @@ def test_table_replacement(self):
17141722
"--verify-existence",
17151723
]
17161724
)[0],
1717-
equal_to(False),
1725+
is_(False),
17181726
)
17191727

17201728
assert_that(
@@ -1725,3 +1733,27 @@ def test_table_replacement(self):
17251733
[child.name for child in self.driver.scheme_client.list_directory("/Root/restoration").children],
17261734
empty()
17271735
)
1736+
1737+
# Replace a non-existent table
1738+
assert_that(
1739+
self.try_ydb_cli(
1740+
[
1741+
"tools",
1742+
"restore",
1743+
"--path", "./restoration/point",
1744+
"--input", backup_files_dir,
1745+
"--replace",
1746+
"--dry-run",
1747+
]
1748+
)[0],
1749+
is_(False),
1750+
)
1751+
assert_that(
1752+
[child.name for child in self.driver.scheme_client.list_directory("/Root/restoration").children],
1753+
empty()
1754+
)
1755+
self.ydb_cli(["tools", "restore", "--path", "./restoration/point", "--input", backup_files_dir, "--replace"])
1756+
assert_that(
1757+
is_tables_the_same(session, self.driver.scheme_client, "/Root/table", "/Root/restoration/point/table"),
1758+
is_(True),
1759+
)

0 commit comments

Comments
 (0)