From 739a69646798bfdcdbd8ab42554cd4c89744eb83 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Jun 2022 16:48:06 +0900 Subject: [PATCH 1/6] Ensure reading of existing databases when making backups doesn't create a file --- osu.Game/Database/DatabaseContextFactory.cs | 2 +- osu.Game/Database/RealmAccess.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Database/DatabaseContextFactory.cs b/osu.Game/Database/DatabaseContextFactory.cs index 45557aa5ec..d222aec6b3 100644 --- a/osu.Game/Database/DatabaseContextFactory.cs +++ b/osu.Game/Database/DatabaseContextFactory.cs @@ -151,7 +151,7 @@ namespace osu.Game.Database { Logger.Log($"Creating full EF database backup at {backupFilename}", LoggingTarget.Database); - using (var source = storage.GetStream(DATABASE_NAME)) + using (var source = storage.GetStream(DATABASE_NAME, mode: FileMode.Open)) using (var destination = storage.GetStream(backupFilename, FileAccess.Write, FileMode.CreateNew)) source.CopyTo(destination); } diff --git a/osu.Game/Database/RealmAccess.cs b/osu.Game/Database/RealmAccess.cs index de7e3020c6..bd47920f82 100644 --- a/osu.Game/Database/RealmAccess.cs +++ b/osu.Game/Database/RealmAccess.cs @@ -762,7 +762,7 @@ namespace osu.Game.Database { try { - using (var source = storage.GetStream(Filename)) + using (var source = storage.GetStream(Filename, mode: FileMode.Open)) using (var destination = storage.GetStream(backupFilename, FileAccess.Write, FileMode.CreateNew)) source.CopyTo(destination); return; From 17dbb599d145248a013cd639dd4efdf37fcee162 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Jun 2022 16:48:52 +0900 Subject: [PATCH 2/6] Don't backup collection database This is no longer required due to recent changes which mean the collection database will retain beatmap references even if they aren't loaded locally (see https://github.com/ppy/osu/pull/18619). --- osu.Game/OsuGameBase.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 5dbdf6f602..3f896ff52e 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -244,15 +244,6 @@ namespace osu.Game EFContextFactory.CreateBackup(Path.Combine(backup_folder, $"client.{migration}.db")); realm.CreateBackup(Path.Combine(backup_folder, $"client.{migration}.realm")); - - using (var source = Storage.GetStream("collection.db")) - { - if (source != null) - { - using (var destination = Storage.CreateFileSafely(Path.Combine(backup_folder, $"collection.{migration}.db"))) - source.CopyTo(destination); - } - } } dependencies.CacheAs(Storage); From 4526f8c07d79d380a39cda62204b30e15e4c75f1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Jun 2022 16:50:00 +0900 Subject: [PATCH 3/6] Move database backup creation to async thread where possible --- osu.Game/Database/EFToRealmMigrator.cs | 13 ++++++++++++- osu.Game/Database/RealmAccess.cs | 4 ++-- osu.Game/OsuGameBase.cs | 13 ------------- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/osu.Game/Database/EFToRealmMigrator.cs b/osu.Game/Database/EFToRealmMigrator.cs index 4e98b7d3d2..d2f52fa635 100644 --- a/osu.Game/Database/EFToRealmMigrator.cs +++ b/osu.Game/Database/EFToRealmMigrator.cs @@ -123,8 +123,18 @@ namespace osu.Game.Database private void beginMigration() { + const string backup_folder = "backups"; + + string backupSuffix = $"before_final_migration_{DateTimeOffset.UtcNow.ToUnixTimeSeconds()}"; + + // required for initial backup. + var realmBlockOperations = realm.BlockAllOperations(); + Task.Factory.StartNew(() => { + realm.CreateBackup(Path.Combine(backup_folder, $"client.{backupSuffix}.realm"), realmBlockOperations); + efContextFactory.CreateBackup(Path.Combine(backup_folder, $"client.{backupSuffix}.db")); + using (var ef = efContextFactory.Get()) { realm.Write(r => @@ -182,7 +192,6 @@ namespace osu.Game.Database true); const string attachment_filename = "attach_me.zip"; - const string backup_folder = "backups"; var backupStorage = storage.GetStorageForDirectory(backup_folder); @@ -209,6 +218,8 @@ namespace osu.Game.Database // If we were to not do this, the migration would run another time the next time the user starts the game. deletePreRealmData(); + realmBlockOperations.Dispose(); + migrationCompleted.SetResult(true); efContextFactory.SetMigrationCompletion(); }); diff --git a/osu.Game/Database/RealmAccess.cs b/osu.Game/Database/RealmAccess.cs index bd47920f82..b144b70574 100644 --- a/osu.Game/Database/RealmAccess.cs +++ b/osu.Game/Database/RealmAccess.cs @@ -750,9 +750,9 @@ namespace osu.Game.Database private string? getRulesetShortNameFromLegacyID(long rulesetId) => efContextFactory?.Get().RulesetInfo.FirstOrDefault(r => r.ID == rulesetId)?.ShortName; - public void CreateBackup(string backupFilename) + public void CreateBackup(string backupFilename, IDisposable? blockAllOperations = null) { - using (BlockAllOperations()) + using (blockAllOperations ?? BlockAllOperations()) { Logger.Log($"Creating full realm database backup at {backupFilename}", LoggingTarget.Database); diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 3f896ff52e..a4e6486c46 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -233,19 +233,6 @@ namespace osu.Game Decoder.RegisterDependencies(RulesetStore); - // Backup is taken here rather than in EFToRealmMigrator to avoid recycling realm contexts - // after initial usages below. It can be moved once a direction is established for handling re-subscription. - // See https://github.com/ppy/osu/pull/16547 for more discussion. - if (EFContextFactory != null) - { - const string backup_folder = "backups"; - - string migration = $"before_final_migration_{DateTimeOffset.UtcNow.ToUnixTimeSeconds()}"; - - EFContextFactory.CreateBackup(Path.Combine(backup_folder, $"client.{migration}.db")); - realm.CreateBackup(Path.Combine(backup_folder, $"client.{migration}.realm")); - } - dependencies.CacheAs(Storage); var largeStore = new LargeTextureStore(Host.CreateTextureLoaderStore(new NamespacedResourceStore(Resources, @"Textures"))); From 7b0fad6461342948385a75166d68df24b175d18c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Jun 2022 23:29:33 +0900 Subject: [PATCH 4/6] Null disposal token after use --- osu.Game/Database/EFToRealmMigrator.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Database/EFToRealmMigrator.cs b/osu.Game/Database/EFToRealmMigrator.cs index d2f52fa635..aa25c76943 100644 --- a/osu.Game/Database/EFToRealmMigrator.cs +++ b/osu.Game/Database/EFToRealmMigrator.cs @@ -133,6 +133,8 @@ namespace osu.Game.Database Task.Factory.StartNew(() => { realm.CreateBackup(Path.Combine(backup_folder, $"client.{backupSuffix}.realm"), realmBlockOperations); + realmBlockOperations = null; + efContextFactory.CreateBackup(Path.Combine(backup_folder, $"client.{backupSuffix}.db")); using (var ef = efContextFactory.Get()) @@ -218,7 +220,7 @@ namespace osu.Game.Database // If we were to not do this, the migration would run another time the next time the user starts the game. deletePreRealmData(); - realmBlockOperations.Dispose(); + realmBlockOperations?.Dispose(); migrationCompleted.SetResult(true); efContextFactory.SetMigrationCompletion(); From 7809566f166e847b80723bf2bab3bc9f6a190f40 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Jun 2022 23:31:38 +0900 Subject: [PATCH 5/6] Add explanatory comments --- osu.Game/Database/EFToRealmMigrator.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Database/EFToRealmMigrator.cs b/osu.Game/Database/EFToRealmMigrator.cs index aa25c76943..74a3edc3ee 100644 --- a/osu.Game/Database/EFToRealmMigrator.cs +++ b/osu.Game/Database/EFToRealmMigrator.cs @@ -133,6 +133,9 @@ namespace osu.Game.Database Task.Factory.StartNew(() => { realm.CreateBackup(Path.Combine(backup_folder, $"client.{backupSuffix}.realm"), realmBlockOperations); + + // Above call will dispose of the blocking token when done. + // Clean up here so we don't accidentally dispose twice. realmBlockOperations = null; efContextFactory.CreateBackup(Path.Combine(backup_folder, $"client.{backupSuffix}.db")); @@ -220,6 +223,7 @@ namespace osu.Game.Database // If we were to not do this, the migration would run another time the next time the user starts the game. deletePreRealmData(); + // If something went wrong and the disposal token wasn't invoked above, ensure it is here. realmBlockOperations?.Dispose(); migrationCompleted.SetResult(true); From bf6c6682bcf05ae241bb41d4f60de4960d29acb4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Jun 2022 23:37:24 +0900 Subject: [PATCH 6/6] Move null to finally --- osu.Game/Database/EFToRealmMigrator.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/osu.Game/Database/EFToRealmMigrator.cs b/osu.Game/Database/EFToRealmMigrator.cs index 74a3edc3ee..2486071c01 100644 --- a/osu.Game/Database/EFToRealmMigrator.cs +++ b/osu.Game/Database/EFToRealmMigrator.cs @@ -132,11 +132,16 @@ namespace osu.Game.Database Task.Factory.StartNew(() => { - realm.CreateBackup(Path.Combine(backup_folder, $"client.{backupSuffix}.realm"), realmBlockOperations); - - // Above call will dispose of the blocking token when done. - // Clean up here so we don't accidentally dispose twice. - realmBlockOperations = null; + try + { + realm.CreateBackup(Path.Combine(backup_folder, $"client.{backupSuffix}.realm"), realmBlockOperations); + } + finally + { + // Above call will dispose of the blocking token when done. + // Clean up here so we don't accidentally dispose twice. + realmBlockOperations = null; + } efContextFactory.CreateBackup(Path.Combine(backup_folder, $"client.{backupSuffix}.db"));