Skip to content

fix(downloads): orphan cleanup skipped live empty-queue snapshots, stranding deleted downloads and blocking re-grabs with 409 #755

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.

Sign up for GitHub

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

Open
t3chnaztea wants to merge 1 commit into
base: canary
Choose a base branch
from
Open

fix(downloads): orphan cleanup skipped live empty-queue snapshots, stranding deleted downloads and blocking re-grabs with 409 #755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions listenarr.application/Downloads/Queue/DownloadOrphanCleanupService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,14 @@ public async Task RemoveOrphansAsync(
return;
}

// A snapshot that reaches this point is guaranteed live: the UsedCachedSnapshot
// and IsUnavailable guards above already excluded every unreachable path (the
// poller surfaces timeout/cancel/error as cached or unavailable, never as a live
// empty snapshot). An empty live queue therefore means the items really are gone,
// so an empty snapshot must be allowed to terminalize orphans - otherwise deleting
// your only active torrent strands its Download record forever and blocks re-grabs
// with 409. The per-item grace period below still protects fresh adds.
var clientQueue = clientQueueResult.QueueItems;
if (clientQueue.Count == 0 && clientDownloads.Any())
{
logger.LogWarning(
"Skipping orphan cleanup for client {ClientName}: client returned 0 queue items but {Count} downloads are tracked. Client may be temporarily unreachable.",
client.Name,
clientDownloads.Count);
return;
}

var liveClientItemIds = BuildLiveClientItemIds(clientQueue, mappedQueueItems);
var now = DateTime.UtcNow;
var cleanupCandidates = clientDownloads
Expand Down
92 changes: 91 additions & 1 deletion tests/Features/Application/Downloads/Queue/DownloadOrphanCleanupServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ await service.RemoveOrphansAsync(

[Fact]
[Trait("Method", "RemoveOrphansAsync")]
public async Task RemoveOrphansAsync_DoesNotRemoveIdlessDownloadWhenLiveSnapshotIsEmpty()
public async Task RemoveOrphansAsync_RemovesIdlessDownloadWhenLiveSnapshotIsEmpty()
{
var client = CreateClient();
var download = await AddDownloadAsync(
Expand All @@ -102,6 +102,96 @@ await service.RemoveOrphansAsync(
[],
[download]);

Assert.Null(await _downloadRepository.GetByIdAsync(download.Id));
_metricsMock.Verify(m => m.Increment("download.orphan.unlinked_removed", 1), Times.Once);
_metricsMock.Verify(m => m.Increment("download.orphan.removed", It.IsAny<double>()), Times.Never);
}

[Fact]
[Trait("Method", "RemoveOrphansAsync")]
public async Task RemoveOrphansAsync_RemovesOnlyTrackedDownloadWhenLiveSnapshotIsEmpty()
{
var client = CreateClient();
var download = await AddDownloadAsync(
id: "sole-active-orphan",
clientId: client.Id,
status: DownloadStatus.Downloading,
startedAt: DateTime.UtcNow.AddMinutes(-10),
metadata: new Dictionary<string, object>
{
["ClientDownloadId"] = "deleted-in-client"
});
var service = _provider.GetRequiredService<DownloadOrphanCleanupService>();

await service.RemoveOrphansAsync(
client,
CreateLiveSnapshot(client, []),
[],
[download]);

Assert.Null(await _downloadRepository.GetByIdAsync(download.Id));
_metricsMock.Verify(m => m.Increment("download.orphan.removed", 1), Times.Once);
_metricsMock.Verify(m => m.Increment("download.orphan.unlinked_removed", It.IsAny<double>()), Times.Never);
}

[Fact]
[Trait("Method", "RemoveOrphansAsync")]
public async Task RemoveOrphansAsync_DoesNotRemoveRecentDownloadWhenLiveSnapshotIsEmpty()
{
var client = CreateClient();
var download = await AddDownloadAsync(
id: "recent-empty-snapshot-download",
clientId: client.Id,
status: DownloadStatus.Downloading,
startedAt: DateTime.UtcNow.AddMinutes(-1),
metadata: new Dictionary<string, object>
{
["ClientDownloadId"] = "deleted-in-client"
});
var service = _provider.GetRequiredService<DownloadOrphanCleanupService>();

await service.RemoveOrphansAsync(
client,
CreateLiveSnapshot(client, []),
[],
[download]);

Assert.NotNull(await _downloadRepository.GetByIdAsync(download.Id));
_metricsMock.Verify(m => m.Increment(It.IsAny<string>(), It.IsAny<double>()), Times.Never);
}

[Theory]
[Trait("Method", "RemoveOrphansAsync")]
[InlineData(true, false)]
[InlineData(false, true)]
public async Task RemoveOrphansAsync_DoesNotRemoveWhenEmptySnapshotIsNotTrusted(bool usedCachedSnapshot, bool isUnavailable)
{
var client = CreateClient();
var download = await AddDownloadAsync(
id: "untrusted-empty-snapshot-download",
clientId: client.Id,
status: DownloadStatus.Downloading,
startedAt: DateTime.UtcNow.AddMinutes(-10),
metadata: new Dictionary<string, object>
{
["ClientDownloadId"] = "deleted-in-client"
});
var service = _provider.GetRequiredService<DownloadOrphanCleanupService>();

await service.RemoveOrphansAsync(
client,
new ClientQueueFetchResult(
client,
[],
usedCachedSnapshot,
isUnavailable,
snapshotAge: null,
failureReason: isUnavailable ? "unavailable" : null,
snapshotState: isUnavailable ? "unavailable" : "cached",
snapshotRefreshedAtUtc: DateTimeOffset.UtcNow),
[],
[download]);

Assert.NotNull(await _downloadRepository.GetByIdAsync(download.Id));
_metricsMock.Verify(m => m.Increment(It.IsAny<string>(), It.IsAny<double>()), Times.Never);
}
Expand Down