fix(491): unique index on LibraryFolder(LibraryPathId, PathHash) + tolerate concurrent insert
GetOrAddFolder was a check-then-insert with no unique constraint behind it, so two callers racing the same folder could both miss the lookup and both insert. Enforce identity in the schema and make the loser adopt the winner. - LibraryFolder gains a SHA-256 PathHash (the MediaFile.Path/PathHash precedent): Path is MySQL longtext, which cannot be indexed without a prefix length and collates case-insensitively, so the unique index is on (LibraryPathId, PathHash) instead. - GetOrAddFolder and SetEtag catch a classified unique violation via the existing TvContext.IsUniqueConstraintViolation seam (#308) and re-read. - Dual-provider migration audits and collapses pre-existing duplicates (repointing MediaFile, ParentId and ImageFolderDuration) before creating the index; legacy rows keep a null hash and heal on the next scan. - Tests: deterministic cross-connection race, 8x10 barrier stress with an insert-attempt vacuity guard, classifier-inversion negative control, and a real-migration dedupe test. Refs #488 #308 fix #491
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
using ErsatzTV.Core.Domain;
|
||||
using ErsatzTV.Core.Domain;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.EntityFrameworkCore.Metadata.Builders;
|
||||
|
||||
@@ -10,6 +10,19 @@ public class LibraryFolderConfiguration : IEntityTypeConfiguration<LibraryFolder
|
||||
{
|
||||
builder.ToTable("LibraryFolder");
|
||||
|
||||
// ersatztv#491: GetOrAddFolder is a check-then-insert, so two callers racing the same folder both
|
||||
// miss the lookup and both insert. The unique index makes the duplicate impossible at the storage
|
||||
// layer (the loser gets a constraint violation the repository catches and re-reads).
|
||||
// Indexed on PathHash rather than Path because Path is unbounded (MySQL longtext, which cannot be
|
||||
// indexed without a prefix length, and whose default collation is case-INsensitive — a prefix
|
||||
// index would also false-collide sibling folders differing only in case on a case-sensitive
|
||||
// filesystem). This mirrors the existing MediaFile.Path/PathHash pair.
|
||||
builder.Property(f => f.PathHash)
|
||||
.HasMaxLength(64);
|
||||
|
||||
builder.HasIndex(f => new { f.LibraryPathId, f.PathHash })
|
||||
.IsUnique();
|
||||
|
||||
builder.HasOne(f => f.Parent)
|
||||
.WithMany(p => p.Children)
|
||||
.HasForeignKey(f => f.ParentId)
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
using System.IO.Abstractions;
|
||||
using Dapper;
|
||||
using ErsatzTV.Core;
|
||||
using ErsatzTV.Core.Domain;
|
||||
using ErsatzTV.Core.Interfaces.Repositories;
|
||||
using ErsatzTV.Infrastructure.Extensions;
|
||||
@@ -110,15 +111,35 @@ public class LibraryRepository(IFileSystem fileSystem, IDbContextFactory<TvConte
|
||||
|
||||
if (knownFolder.IsNone)
|
||||
{
|
||||
await dbContext.LibraryFolders.AddAsync(
|
||||
new LibraryFolder
|
||||
{
|
||||
Path = path,
|
||||
Etag = etag,
|
||||
LibraryPathId = libraryPath.Id
|
||||
});
|
||||
var newFolder = new LibraryFolder
|
||||
{
|
||||
Path = path,
|
||||
PathHash = PathUtils.GetPathHash(path),
|
||||
Etag = etag,
|
||||
LibraryPathId = libraryPath.Id
|
||||
};
|
||||
|
||||
await dbContext.SaveChangesAsync();
|
||||
try
|
||||
{
|
||||
await dbContext.LibraryFolders.AddAsync(newFolder);
|
||||
await dbContext.SaveChangesAsync();
|
||||
}
|
||||
catch (DbUpdateException ex) when (TvContext.IsUniqueConstraintViolation(ex))
|
||||
{
|
||||
// ersatztv#491: a concurrent caller created this folder between the caller's lookup and
|
||||
// this insert. The etag write is the whole point of the call, so apply it to the winner's
|
||||
// row rather than failing the scan.
|
||||
dbContext.Entry(newFolder).State = EntityState.Detached;
|
||||
LibraryFolder winner = await GetFolder(dbContext, libraryPath.Id, path);
|
||||
if (winner is null)
|
||||
{
|
||||
throw;
|
||||
}
|
||||
|
||||
await dbContext.Connection.ExecuteAsync(
|
||||
"UPDATE LibraryFolder SET Etag = @Etag WHERE Id = @Id",
|
||||
new { winner.Id, Etag = etag });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -174,11 +195,44 @@ public class LibraryRepository(IFileSystem fileSystem, IDbContextFactory<TvConte
|
||||
// local scan path (via GetLibrary) and is null on the remote (Jellyfin) sync path, which used
|
||||
// to NRE every Jellyfin music-video scan here (ersatztv#488). The local scanners already hit
|
||||
// the db once per folder via GetParentFolderId, so this adds no new query pattern.
|
||||
LibraryFolder knownFolder = await dbContext.LibraryFolders
|
||||
.AsNoTracking()
|
||||
.Filter(f => f.LibraryPathId == libraryPath.Id && f.Path == folder)
|
||||
.FirstOrDefaultAsync()
|
||||
?? CreateNewFolder(libraryPath, maybeParentFolder, folder);
|
||||
LibraryFolder knownFolder = await GetFolder(dbContext, libraryPath.Id, folder);
|
||||
|
||||
// add new folder to library path
|
||||
if (knownFolder is null)
|
||||
{
|
||||
LibraryFolder newFolder = CreateNewFolder(libraryPath, maybeParentFolder, folder);
|
||||
try
|
||||
{
|
||||
await dbContext.LibraryFolders.AddAsync(newFolder);
|
||||
await dbContext.SaveChangesAsync();
|
||||
knownFolder = newFolder;
|
||||
}
|
||||
catch (DbUpdateException ex) when (TvContext.IsUniqueConstraintViolation(ex))
|
||||
{
|
||||
// ersatztv#491: the lookup above is not atomic with this insert, so a concurrent caller
|
||||
// scanning the same folder can slip its row in between. The unique index on
|
||||
// (LibraryPathId, PathHash) turns that lost race into a constraint violation instead of a
|
||||
// duplicate row; adopt the winner's row rather than failing the scan. Detach first so the
|
||||
// failed insert is not retried by anything reusing this context.
|
||||
dbContext.Entry(newFolder).State = EntityState.Detached;
|
||||
knownFolder = await GetFolder(dbContext, libraryPath.Id, folder);
|
||||
if (knownFolder is null)
|
||||
{
|
||||
// no winner to adopt — the violation came from somewhere else, so don't swallow it
|
||||
throw;
|
||||
}
|
||||
}
|
||||
}
|
||||
else if (string.IsNullOrEmpty(knownFolder.PathHash))
|
||||
{
|
||||
// heal a row created before the PathHash column existed, so it participates in the unique
|
||||
// index from here on (a null hash is distinct from every other value, so it does not).
|
||||
knownFolder.PathHash = PathUtils.GetPathHash(folder);
|
||||
|
||||
await dbContext.Connection.ExecuteAsync(
|
||||
"UPDATE LibraryFolder SET PathHash = @PathHash WHERE Id = @Id",
|
||||
new { knownFolder.PathHash, knownFolder.Id });
|
||||
}
|
||||
|
||||
// update parent folder if not present
|
||||
foreach (int parentFolder in maybeParentFolder)
|
||||
@@ -193,13 +247,6 @@ public class LibraryRepository(IFileSystem fileSystem, IDbContextFactory<TvConte
|
||||
}
|
||||
}
|
||||
|
||||
// add new folder to library path
|
||||
if (knownFolder.Id < 1)
|
||||
{
|
||||
await dbContext.LibraryFolders.AddAsync(knownFolder);
|
||||
await dbContext.SaveChangesAsync();
|
||||
}
|
||||
|
||||
return knownFolder;
|
||||
}
|
||||
|
||||
@@ -221,6 +268,12 @@ public class LibraryRepository(IFileSystem fileSystem, IDbContextFactory<TvConte
|
||||
new { Path = normalizedLibraryPath, libraryPath.Id });
|
||||
}
|
||||
|
||||
private static Task<LibraryFolder> GetFolder(TvContext dbContext, int libraryPathId, string folder) =>
|
||||
dbContext.LibraryFolders
|
||||
.AsNoTracking()
|
||||
.Filter(f => f.LibraryPathId == libraryPathId && f.Path == folder)
|
||||
.FirstOrDefaultAsync();
|
||||
|
||||
private static LibraryFolder CreateNewFolder(LibraryPath libraryPath, Option<int> maybeParentFolder, string folder)
|
||||
{
|
||||
int? parentId = null;
|
||||
@@ -232,6 +285,7 @@ public class LibraryRepository(IFileSystem fileSystem, IDbContextFactory<TvConte
|
||||
return new LibraryFolder
|
||||
{
|
||||
Path = folder,
|
||||
PathHash = PathUtils.GetPathHash(folder),
|
||||
Etag = null,
|
||||
LibraryPathId = libraryPath.Id,
|
||||
ParentId = parentId
|
||||
|
||||
Reference in New Issue
Block a user