h1or4dme8/osu-lazer
1
0
Fork 0
mirror of https://github.com/ppy/osu.git synced 2025-01-12 22:33:05 +08:00
Code Issues Releases Wiki Activity

Fix migration failing on single file copy failure

Browse Source 
No longer throw if copying of single files fails during data migration.

Aiming to fix https://github.com/ppy/osu/runs/7601653833?check_suite_focus=true, which could also affect end users. I've left a limit before an exception is still thrown, to handle cases like the user running out of disk space (where we probably *do* want to bail, so they can continue to use their still-intact original storage location).

If this isn't seen as a good direction, an alternative will be to make the migration code aware of the structure of the temporary files created by `Storage` (but doesn't guarantee this will cover all cases of such temporary files – there could for isntance be metadata files created by the host operating system).

Another option would be to mark those temporary files as hidden and skip any hidden files during iteration.
This commit is contained in:
Dean Herbert 2022-08-01 14:58:09 +09:00
parent 5335d60748
commit fc8835d43a
1 changed files with 21 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
osu.Game/IO/MigratableStorage.cs
View File

@ -7,6 +7,7 @@ using System;
using System.IO; using System.IO;
using System.Linq; using System.Linq;
using System.Threading; using System.Threading;
using osu.Framework.Logging;
using osu.Framework.Platform; using osu.Framework.Platform;
namespace osu.Game.IO namespace osu.Game.IO
@ -16,6 +17,12 @@ namespace osu.Game.IO
/// </summary> /// </summary>
public abstract class MigratableStorage : WrappedStorage public abstract class MigratableStorage : WrappedStorage
{ {
/// <summary>
/// The number of file copy failures before actually bailing on migration.
/// This allows some lenience to cover things like temporary files which could not be copied but are also not too important.
/// </summary>
private const int allowed_failures = 10;
/// <summary> /// <summary>
/// A relative list of directory paths which should not be migrated. /// A relative list of directory paths which should not be migrated.
/// </summary> /// </summary>
@ -73,7 +80,7 @@ namespace osu.Game.IO
if (topLevelExcludes && IgnoreFiles.Contains(fi.Name)) if (topLevelExcludes && IgnoreFiles.Contains(fi.Name))
continue; continue;
allFilesDeleted &= AttemptOperation(() => fi.Delete(), throwOnFailure: false); allFilesDeleted &= AttemptOperation(() => fi.Delete());
} }
foreach (DirectoryInfo dir in target.GetDirectories()) foreach (DirectoryInfo dir in target.GetDirectories())
@ -81,16 +88,16 @@ namespace osu.Game.IO
if (topLevelExcludes && IgnoreDirectories.Contains(dir.Name)) if (topLevelExcludes && IgnoreDirectories.Contains(dir.Name))
continue; continue;
allFilesDeleted &= AttemptOperation(() => dir.Delete(true), throwOnFailure: false); allFilesDeleted &= AttemptOperation(() => dir.Delete(true));
} }
if (target.GetFiles().Length == 0 && target.GetDirectories().Length == 0) if (target.GetFiles().Length == 0 && target.GetDirectories().Length == 0)
allFilesDeleted &= AttemptOperation(target.Delete, throwOnFailure: false); allFilesDeleted &= AttemptOperation(target.Delete);
return allFilesDeleted; return allFilesDeleted;
} }
protected void CopyRecursive(DirectoryInfo source, DirectoryInfo destination, bool topLevelExcludes = true) protected void CopyRecursive(DirectoryInfo source, DirectoryInfo destination, bool topLevelExcludes = true, int totalFailedOperations = 0)
{ {
// based off example code https://docs.microsoft.com/en-us/dotnet/api/system.io.directoryinfo // based off example code https://docs.microsoft.com/en-us/dotnet/api/system.io.directoryinfo
if (!destination.Exists) if (!destination.Exists)
@ -101,7 +108,14 @@ namespace osu.Game.IO
if (topLevelExcludes && IgnoreFiles.Contains(fi.Name)) if (topLevelExcludes && IgnoreFiles.Contains(fi.Name))
continue; continue;
AttemptOperation(() => fi.CopyTo(Path.Combine(destination.FullName, fi.Name), true)); if (!AttemptOperation(() => fi.CopyTo(Path.Combine(destination.FullName, fi.Name), false)))
{
Logger.Log($"Failed to copy file {fi.Name} during folder migration");
totalFailedOperations++;
if (totalFailedOperations > allowed_failures)
throw new Exception("Aborting due to too many file copy failures during data migration");
}
} }
foreach (DirectoryInfo dir in source.GetDirectories()) foreach (DirectoryInfo dir in source.GetDirectories())
@ -109,7 +123,7 @@ namespace osu.Game.IO
if (topLevelExcludes && IgnoreDirectories.Contains(dir.Name)) if (topLevelExcludes && IgnoreDirectories.Contains(dir.Name))
continue; continue;
CopyRecursive(dir, destination.CreateSubdirectory(dir.Name), false); CopyRecursive(dir, destination.CreateSubdirectory(dir.Name), false, totalFailedOperations);
} }
} }
@ -118,8 +132,7 @@ namespace osu.Game.IO
/// </summary> /// </summary>
/// <param name="action">The action to perform.</param> /// <param name="action">The action to perform.</param>
/// <param name="attempts">The number of attempts (250ms wait between each).</param> /// <param name="attempts">The number of attempts (250ms wait between each).</param>
/// <param name="throwOnFailure">Whether to throw an exception on failure. If <c>false</c>, will silently fail.</param> protected static bool AttemptOperation(Action action, int attempts = 10)
protected static bool AttemptOperation(Action action, int attempts = 10, bool throwOnFailure = true)
{ {
while (true) while (true)
{ {
@ -131,13 +144,8 @@ namespace osu.Game.IO
catch (Exception) catch (Exception)
{ {
if (attempts-- == 0) if (attempts-- == 0)
{
if (throwOnFailure)
throw;
return false; return false;
} }
}
Thread.Sleep(250); Thread.Sleep(250);
} }