Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to import more than 9 beatmaps at the same time. #31667

Closed
1plam opened this issue Jan 24, 2025 · 3 comments · Fixed by #31698
Closed

Unable to import more than 9 beatmaps at the same time. #31667

1plam opened this issue Jan 24, 2025 · 3 comments · Fixed by #31698
Assignees
Labels
area:import dealing with importing of data from stable or file formats

Comments

@1plam
Copy link

1plam commented Jan 24, 2025

Type

Game behaviour

Bug description

Attempted to import 10 & more .osz files simultaneously.

Screenshots or videos

No response

Version

2025.118.3-lazer

Logs

compressed-logs.zip

@smoogipoo smoogipoo added the area:import dealing with importing of data from stable or file formats label Jan 27, 2025
@bdach
Copy link
Collaborator

bdach commented Jan 27, 2025

I tried importing 20 beatmaps and it works fine here, so cannot reproduce in general. The logs attached have a bunch of this inside:

2025-01-24 20:39:11 [verbose]: [f1324] Beginning import from 1208 ORANGE RANGE - Ikenai Taiyou.osz...
2025-01-24 20:39:11 [verbose]: [f1324] No content found in beatmap file orange range - ikenai taiyou (echo49) [easy].osu.
2025-01-24 20:39:11 [verbose]: [f1324] No content found in beatmap file orange range - ikenai taiyou (echo49) [hard].osu.
2025-01-24 20:39:11 [verbose]: [f1324] No content found in beatmap file orange range - ikenai taiyou (echo49) [normal].osu.
2025-01-24 20:39:11 [verbose]: [f1324] No content found in beatmap file orange range - ikenai taiyou (echo49) [beginner].osu.
2025-01-24 20:39:11 [error]: [f1324] Database import or population failed and has been rolled back.
2025-01-24 20:39:11 [error]: System.ArgumentException: No valid beatmap files found in the beatmap archive.
2025-01-24 20:39:11 [error]: at osu.Game.Beatmaps.BeatmapImporter.createBeatmapDifficulties(BeatmapSetInfo beatmapSet, Realm realm)
2025-01-24 20:39:11 [error]: at osu.Game.Beatmaps.BeatmapImporter.Populate(BeatmapSetInfo beatmapSet, ArchiveReader archive, Realm realm, CancellationToken cancellationToken)
2025-01-24 20:39:11 [error]: at osu.Game.Database.RealmArchiveModelImporter`1.<>c__DisplayClass28_0.<ImportModel>b__0(Realm realm)
2025-01-24 20:39:11 [error]: Could not import (1208 ORANGE RANGE - Ikenai Taiyou.osz)

Please attach the beatmaps you were trying to import at the time. I suspect they are corrupted.

@bdach bdach added the missing details Can't move forward without more details from the reporter label Jan 27, 2025
@1plam
Copy link
Author

1plam commented Jan 27, 2025

Indeed, the beatmaps appear to be corrupted, despite being extracted from my system for backup during migration (Windows reinstall/macOS Lazer transition). However, it's unclear why importing fewer than 10 maps at once works successfully.

Please find the link to an archive (Google Drive) containing 10 beatmaps as an example.

I couldn't have attached an archive directly due to size limit.

@bdach bdach self-assigned this Jan 27, 2025
@bdach
Copy link
Collaborator

bdach commented Jan 27, 2025

Thanks for the archive, that's helpful.

This is actually a bug in sharpcompress, which is tracked upstream as adamhathcock/sharpcompress#88. The reason why this bug only manifests when there are 10+ imports is that this bug is a "heisenbug", in that it only appears if a given ZIP entry is attempted to be read more than once.

A minimal reproducer follows:

using SharpCompress.Archives.Zip;

const string filePath =
    "/home/dachb/Downloads/ba69d88b-85bb-445b-82f2-185c18f74264 (2)/50462 Lon - Yuru Fuwa Jukai Girl.osz";

using var archive = ZipArchive.Open(filePath);

Console.WriteLine($"First entry size is {archive.Entries.First().Size}");
archive.Entries.First().OpenEntryStream().ReadExactly(new byte[archive.Entries.First().Size].AsSpan());
Console.WriteLine($"First entry size is {archive.Entries.First().Size}");

which prints

First entry size is 34713
First entry size is 0

When the batch of imports has 10+ items, the following block of code comes alive:

if (parameters.Batch && archive != null)
{
// this is a fast bail condition to improve large import performance.
item.Hash = computeHashFast(archive);
existing = CheckForExisting(item, realm);
if (existing != null)
{
// bare minimum comparisons
//
// note that this should really be checking filesizes on disk (of existing files) for some degree of sanity.
// or alternatively doing a faster hash check. either of these require database changes and reprocessing of existing files.
if (CanSkipImport(existing, item) &&
getFilenames(existing.Files).SequenceEqual(getShortenedFilenames(archive).Select(p => p.shortened).Order()) &&
checkAllFilesExist(existing))
{
LogForModel(item, @$"Found existing (optimised) {HumanisedModelName} for {item} (ID {existing.ID}) – skipping import.");
using (var transaction = realm.BeginWrite())
{
UndeleteForReuse(existing);
transaction.Commit();
}
return existing.ToLive(Realm);
}
LogForModel(item, @"Found existing (optimised) but failed pre-check.");
}
}

This includes computeHashFast(), which incurs the extra read of the archive entries that triggers the bug:

foreach (string? file in reader.Filenames.Where(f => HashableFileTypes.Any(ext => f.EndsWith(ext, StringComparison.OrdinalIgnoreCase))).Order())
{
using (Stream s = reader.GetStream(file))
s.CopyTo(hashable);
}

sharpcompress appears to initially read data from the "directory entry header" of the ZIP, which has the correct sizes, but then after the first read of a given file, gains access to the "local entry header" of the given file, which will have the incorrect zero size.

The ZIP produced in adamhathcock/sharpcompress#88 (comment) exhibits the exact same symptom as the ones you attached.

I'll probably apply a local workaround to remedy this - that said, I'd probably consider switching away from whatever tool it was that produced these ZIPs, because it must be not doing something like everyone else is as it's been years without anyone hitting this.

@bdach bdach removed the missing details Can't move forward without more details from the reporter label Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:import dealing with importing of data from stable or file formats
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants