diff --git a/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs b/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs index 861487a218..3de10bb85a 100644 --- a/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs +++ b/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs @@ -12,7 +12,7 @@ namespace ServiceControl.Persistence.EFCore.Implementation; public class GroupsDataStore(IServiceScopeFactory scopeFactory) : DataStoreBase(scopeFactory), IGroupsDataStore { - public Task> GetUnresolvedGroupsByClassifier(string classifier, string? classifierFilter, CancellationToken cancellationToken = default) => + public Task> GetUnresolvedGroupsByClassifier(string classifier, string? classifierFilter, PagingInfo pagingInfo, CancellationToken cancellationToken = default) => ExecuteWithDbContext>(async (dbContext, token) => { var groups = ByClassifier(dbContext, classifier); @@ -22,21 +22,28 @@ public Task> GetUnresolvedGroupsByClassifier(string clas groups = groups.Where(group => group.Title == classifierFilter); } - var views = await GetGroupViews(groups, WithStatus(dbContext, FailedMessageStatus.Unresolved), token); + var views = await GetGroupViews( + groups, + WithStatus(dbContext, FailedMessageStatus.Unresolved), + pagingInfo, + token); await AttachComments(dbContext, views, token); return views; }, cancellationToken); - public Task>> GetArchivedGroupsByClassifier(string classifier, CancellationToken cancellationToken = default) => + public Task>> GetArchivedGroupsByClassifier(string classifier, PagingInfo pagingInfo, CancellationToken cancellationToken = default) => ExecuteWithDbContext(async (dbContext, token) => { var groups = ByClassifier(dbContext, classifier); + var messages = WithStatus(dbContext, FailedMessageStatus.Archived); + + var views = await GetGroupViews(groups, messages, pagingInfo, token); - var views = await GetGroupViews(groups, WithStatus(dbContext, FailedMessageStatus.Archived), token); + var totalCount = await groups.LongCountAsync(token); - return new QueryResult>(views, views.ToQueryStatsInfo("groups", views.Count)); + return new QueryResult>(views, views.ToQueryStatsInfo("groups", totalCount)); }, cancellationToken); public Task> GetUnresolvedGroup(string groupId, string? status, string? modified, CancellationToken cancellationToken = default) => @@ -128,14 +135,15 @@ static async Task AttachComments(ServiceControlDbContext dbContext, IList> GetGroupViews(IQueryable groups, IQueryable messages, CancellationToken cancellationToken) => + static async Task> GetGroupViews(IQueryable groups, IQueryable messages, PagingInfo pagingInfo, CancellationToken cancellationToken) => await groups .AggregateGroups(messages) .OrderByDescending(summary => summary.Last) - .Take(FailureGroupQueries.MaxGroups) + .Skip(pagingInfo.Offset) + .Take(pagingInfo.PageSize) .ToListAsync(cancellationToken); } diff --git a/src/ServiceControl.Persistence.EFCore/Infrastructure/FailureGroupQueries.cs b/src/ServiceControl.Persistence.EFCore/Infrastructure/FailureGroupQueries.cs index a7f10c6276..4bee6d90ac 100644 --- a/src/ServiceControl.Persistence.EFCore/Infrastructure/FailureGroupQueries.cs +++ b/src/ServiceControl.Persistence.EFCore/Infrastructure/FailureGroupQueries.cs @@ -5,8 +5,6 @@ namespace ServiceControl.Persistence.EFCore.Infrastructure; static class FailureGroupQueries { - public const int MaxGroups = 200; - /// /// The group aggregate: membership rows joined to their message, grouped by (GroupId, Type), /// with Count/First/Last per group. diff --git a/src/ServiceControl.Persistence.RavenDB/Recoverability/GroupsDataStore.cs b/src/ServiceControl.Persistence.RavenDB/Recoverability/GroupsDataStore.cs index d8f97782ad..92fcfdf898 100644 --- a/src/ServiceControl.Persistence.RavenDB/Recoverability/GroupsDataStore.cs +++ b/src/ServiceControl.Persistence.RavenDB/Recoverability/GroupsDataStore.cs @@ -14,7 +14,7 @@ namespace ServiceControl.Persistence.RavenDB.Recoverability class GroupsDataStore(IRavenSessionProvider sessionProvider) : IGroupsDataStore { - public async Task> GetUnresolvedGroupsByClassifier(string classifier, string classifierFilter, CancellationToken cancellationToken = default) + public async Task> GetUnresolvedGroupsByClassifier(string classifier, string classifierFilter, PagingInfo pagingInfo, CancellationToken cancellationToken = default) { using var session = await sessionProvider.OpenSession(cancellationToken: cancellationToken); var query = Queryable.Where(session.Query(), v => v.Type == classifier); @@ -24,8 +24,9 @@ public async Task> GetUnresolvedGroupsByClassifier(strin query = query.Where(v => v.Title == classifierFilter); } - var groups = await query.OrderByDescending(x => x.Last) - .Take(200) + var groups = await query + .OrderByDescending(view => view.Last) + .Paging(pagingInfo) .ToListAsync(cancellationToken); var commentIds = groups.Select(x => MakeId(x.Id)).ToArray(); @@ -40,7 +41,7 @@ public async Task> GetUnresolvedGroupsByClassifier(strin return groups; } - public async Task>> GetArchivedGroupsByClassifier(string classifier, CancellationToken cancellationToken = default) + public async Task>> GetArchivedGroupsByClassifier(string classifier, PagingInfo pagingInfo, CancellationToken cancellationToken = default) { using var session = await sessionProvider.OpenSession(cancellationToken: cancellationToken); var groups = session @@ -49,8 +50,8 @@ public async Task>> GetArchivedGroupsByClass .Where(v => v.Type == classifier); var results = await groups - .OrderByDescending(x => x.Last) - .Take(200) // only show 200 groups + .OrderByDescending(view => view.Last) + .Paging(pagingInfo) .ToListAsync(cancellationToken); return new QueryResult>(results, stats.ToQueryStatsInfo()); diff --git a/src/ServiceControl.Persistence.Tests/Recoverability/ArchiveCancellationTests.cs b/src/ServiceControl.Persistence.Tests/Recoverability/ArchiveCancellationTests.cs index 2fdfea6432..671bb4038f 100644 --- a/src/ServiceControl.Persistence.Tests/Recoverability/ArchiveCancellationTests.cs +++ b/src/ServiceControl.Persistence.Tests/Recoverability/ArchiveCancellationTests.cs @@ -8,6 +8,7 @@ namespace ServiceControl.Persistence.Tests.Recoverability; using NUnit.Framework; using ServiceControl.Infrastructure.DomainEvents; using ServiceControl.MessageFailures; +using ServiceControl.Persistence.Infrastructure; using ServiceControl.Recoverability; [TestFixture] @@ -59,7 +60,7 @@ async Task AssertNoUnresolvedMessagesIn(FailedMessage.FailureGroup group) { await CompleteDatabaseOperation(); - var groups = await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, null); + var groups = await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, null, new PagingInfo(page: 1, pageSize: 200)); Assert.That( groups.Select(view => view.Id), diff --git a/src/ServiceControl.Persistence.Tests/Recoverability/ArchivedGroupVersionTests.cs b/src/ServiceControl.Persistence.Tests/Recoverability/ArchivedGroupVersionTests.cs index 22d2156ce7..b1ba64df89 100644 --- a/src/ServiceControl.Persistence.Tests/Recoverability/ArchivedGroupVersionTests.cs +++ b/src/ServiceControl.Persistence.Tests/Recoverability/ArchivedGroupVersionTests.cs @@ -5,6 +5,7 @@ namespace ServiceControl.Persistence.Tests; using System.Threading.Tasks; using NUnit.Framework; using ServiceControl.MessageFailures; +using ServiceControl.Persistence.Infrastructure; [TestFixture] class ArchivedGroupVersionTests : PersistenceTestBase @@ -14,6 +15,7 @@ class ArchivedGroupVersionTests : PersistenceTestBase static readonly DateTime Oldest = new(2026, 8, 1, 9, 0, 0, DateTimeKind.Utc); static readonly DateTime Middle = new(2026, 8, 1, 13, 0, 0, DateTimeKind.Utc); static readonly DateTime Newest = new(2026, 8, 1, 17, 0, 0, DateTimeKind.Utc); + static readonly PagingInfo pagingInfo = new PagingInfo(page: 1, pageSize: 200); [Test] public async Task Version_changes_when_group_counts_move_but_the_total_and_the_span_hold() @@ -28,7 +30,7 @@ public async Task Version_changes_when_group_counts_move_but_the_total_and_the_s await Insert(oldest, middle, newest); await Archive(oldest, middle, newest); - var before = await GroupsStore.GetArchivedGroupsByClassifier(Classifier); + var before = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, pagingInfo); // The archived set keeps two groups, three messages, and the same earliest and latest failure. // All that moves is how the three are split between the groups, from two and one to one and two. @@ -38,7 +40,7 @@ public async Task Version_changes_when_group_counts_move_but_the_total_and_the_s _ = await FailedMessageLifecycleStore.UnArchiveMessages([middle.UniqueMessageIdString]); await CompleteDatabaseOperation(); - var after = await GroupsStore.GetArchivedGroupsByClassifier(Classifier); + var after = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, pagingInfo); using (Assert.EnterMultipleScope()) { @@ -63,13 +65,13 @@ public async Task Version_changes_when_a_group_gains_a_message() await Insert(first); await Archive(first); - var before = await GroupsStore.GetArchivedGroupsByClassifier(Classifier); + var before = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, pagingInfo); var second = InGroup(group, Newest); await Insert(second); await Archive(second); - var after = await GroupsStore.GetArchivedGroupsByClassifier(Classifier); + var after = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, pagingInfo); VersionAssert.Moved(before.QueryStats.Version, after.QueryStats.Version, "the archived group gained a message, so its validator cannot stay put"); @@ -84,8 +86,8 @@ public async Task Version_is_stable_while_nothing_changes() await Insert(failure); await Archive(failure); - var first = await GroupsStore.GetArchivedGroupsByClassifier(Classifier); - var second = await GroupsStore.GetArchivedGroupsByClassifier(Classifier); + var first = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, pagingInfo); + var second = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, pagingInfo); VersionAssert.Matches(first.QueryStats.Version, second.QueryStats.Version, "nothing changed, so the validator has to stay put or conditional GET never pays off"); @@ -94,7 +96,7 @@ public async Task Version_is_stable_while_nothing_changes() [Test] public async Task A_classifier_with_nothing_archived_still_reports_a_version() { - var result = await GroupsStore.GetArchivedGroupsByClassifier(Classifier); + var result = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, pagingInfo); using (Assert.EnterMultipleScope()) { diff --git a/src/ServiceControl.Persistence.Tests/Recoverability/GroupsDataStoreTests.cs b/src/ServiceControl.Persistence.Tests/Recoverability/GroupsDataStoreTests.cs index 6502efbb4a..c8ea445559 100644 --- a/src/ServiceControl.Persistence.Tests/Recoverability/GroupsDataStoreTests.cs +++ b/src/ServiceControl.Persistence.Tests/Recoverability/GroupsDataStoreTests.cs @@ -25,7 +25,7 @@ await Insert( InGroup(group, failedAt: Noon), InGroup(group, failedAt: Noon.AddHours(2))); - var view = (await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, null)).Single(); + var view = (await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, null, new PagingInfo(page: 1, pageSize: 200))).Single(); using (Assert.EnterMultipleScope()) { @@ -46,7 +46,7 @@ public async Task Returns_only_the_groups_of_the_requested_classifier() await Insert(InGroup(requested), InGroup(other)); - var groups = await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, null); + var groups = await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, null, new PagingInfo(page: 1, pageSize: 200)); Assert.That(groups.Select(group => group.Id), Is.EqualTo(new[] { requested.Id })); } @@ -59,7 +59,7 @@ public async Task Narrows_the_groups_to_the_classifier_filter() await Insert(InGroup(matching), InGroup(other)); - var groups = await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, "OrderPlaced"); + var groups = await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, "OrderPlaced", new PagingInfo(page: 1, pageSize: 200)); Assert.That(groups.Select(group => group.Id), Is.EqualTo(new[] { matching.Id })); } @@ -74,7 +74,7 @@ await Insert( InGroup(group).ToFailedMessage(FailedMessageStatus.Archived), InGroup(group).ToFailedMessage(FailedMessageStatus.Resolved)); - var view = (await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, null)).Single(); + var view = (await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, null, new PagingInfo(page: 1, pageSize: 200))).Single(); Assert.That(view.Count, Is.EqualTo(1)); } @@ -88,7 +88,7 @@ await Insert( InGroup(group).ToFailedMessage(), InGroup(group).ToFailedMessage(FailedMessageStatus.Archived)); - var view = (await GroupsStore.GetArchivedGroupsByClassifier(Classifier)).Results.Single(); + var view = (await GroupsStore.GetArchivedGroupsByClassifier(Classifier, new PagingInfo(page: 1, pageSize: 200))).Results.Single(); using (Assert.EnterMultipleScope()) { @@ -109,11 +109,87 @@ await Insert( InGroup(newest, failedAt: Noon.AddHours(4)), InGroup(middle, failedAt: Noon.AddHours(2))); - var groups = await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, null); + var groups = await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, null, new PagingInfo(page: 1, pageSize: 200)); Assert.That(groups.Select(group => group.Title), Is.EqualTo(new[] { "Newest", "Middle", "Oldest" })); } + [Test] + public async Task Can_page_unresolved_groups() + { + var oldest = NewGroup("Oldest"); + var older = NewGroup("Older"); + var middle = NewGroup("Middle"); + var newer = NewGroup("Newer"); + var newest = NewGroup("Newest"); + + await Insert( + InGroup(oldest, failedAt: Noon), + InGroup(older, failedAt: Noon.AddHours(1)), + InGroup(middle, failedAt: Noon.AddHours(2)), + InGroup(newer, failedAt: Noon.AddHours(3)), + InGroup(newest, failedAt: Noon.AddHours(4))); + + var firstPage = await GroupsStore.GetUnresolvedGroupsByClassifier( + Classifier, + null, + new PagingInfo(page: 1, pageSize: 2)); + + var secondPage = await GroupsStore.GetUnresolvedGroupsByClassifier( + Classifier, + null, + new PagingInfo(page: 2, pageSize: 2)); + + using (Assert.EnterMultipleScope()) + { + Assert.That(firstPage, Has.Count.EqualTo(2)); + Assert.That(secondPage, Has.Count.EqualTo(2)); + Assert.That(firstPage, Is.Ordered.By(nameof(FailureGroupView.Last)).Descending); + Assert.That( + secondPage.Select(group => group.Id).Intersect(firstPage.Select(group => group.Id)), + Is.Empty); + } + } + + [Test] + public async Task Can_page_archived_groups() + { + var oldest = NewGroup("Oldest"); + var older = NewGroup("Older"); + var middle = NewGroup("Middle"); + var newer = NewGroup("Newer"); + var newest = NewGroup("Newest"); + + await Insert( + InGroup(oldest, failedAt: Noon).ToFailedMessage(FailedMessageStatus.Archived), + InGroup(older, failedAt: Noon.AddHours(1)).ToFailedMessage(FailedMessageStatus.Archived), + InGroup(middle, failedAt: Noon.AddHours(2)).ToFailedMessage(FailedMessageStatus.Archived), + InGroup(newer, failedAt: Noon.AddHours(3)).ToFailedMessage(FailedMessageStatus.Archived), + InGroup(newest, failedAt: Noon.AddHours(4)).ToFailedMessage(FailedMessageStatus.Archived)); + + var firstPage = await GroupsStore.GetArchivedGroupsByClassifier( + Classifier, + new PagingInfo(page: 1, pageSize: 2)); + + var secondPage = await GroupsStore.GetArchivedGroupsByClassifier( + Classifier, + new PagingInfo(page: 2, pageSize: 2)); + + using (Assert.EnterMultipleScope()) + { + Assert.That(firstPage.Results, Has.Count.EqualTo(2)); + Assert.That(secondPage.Results, Has.Count.EqualTo(2)); + Assert.That(firstPage.Results, Is.Ordered.By(nameof(FailureGroupView.Last)).Descending); + Assert.That( + secondPage.Results.Select(group => group.Id).Intersect(firstPage.Results.Select(group => group.Id)), + Is.Empty); + Assert.That(firstPage.QueryStats.TotalCount, Is.EqualTo(5), + "the total count should reflect all matching groups, not just the page"); + Assert.That(secondPage.QueryStats.TotalCount, Is.EqualTo(5), + "the total count should reflect all matching groups, not just the page"); + } + } + [Test] public async Task Returns_a_single_group_by_id() { @@ -295,14 +371,14 @@ public async Task Leaves_archived_groups_without_their_comment() await Insert(InGroup(group).ToFailedMessage(FailedMessageStatus.Archived)); await EditComment(group.Id, "Only shown on the open group"); - var view = (await GroupsStore.GetArchivedGroupsByClassifier(Classifier)).Results.Single(); + var view = (await GroupsStore.GetArchivedGroupsByClassifier(Classifier, new PagingInfo(page: 1, pageSize: 200))).Results.Single(); Assert.That(view.Comment, Is.Null); } async Task CommentFor(FailedMessage.FailureGroup group) { - var groups = await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, null); + var groups = await GroupsStore.GetUnresolvedGroupsByClassifier(Classifier, null, new PagingInfo(page: 1, pageSize: 200)); return groups.Single(view => view.Id == group.Id).Comment; } diff --git a/src/ServiceControl.Persistence/IGroupsDataStore.cs b/src/ServiceControl.Persistence/IGroupsDataStore.cs index 2d1ffc547e..d892e36c82 100644 --- a/src/ServiceControl.Persistence/IGroupsDataStore.cs +++ b/src/ServiceControl.Persistence/IGroupsDataStore.cs @@ -9,8 +9,8 @@ namespace ServiceControl.Persistence public interface IGroupsDataStore { - Task> GetUnresolvedGroupsByClassifier(string classifier, string? classifierFilter, CancellationToken cancellationToken = default); - Task>> GetArchivedGroupsByClassifier(string classifier, CancellationToken cancellationToken = default); + Task> GetUnresolvedGroupsByClassifier(string classifier, string? classifierFilter, PagingInfo pagingInfo, CancellationToken cancellationToken = default); + Task>> GetArchivedGroupsByClassifier(string classifier, PagingInfo pagingInfo, CancellationToken cancellationToken = default); Task> GetUnresolvedGroup(string groupId, string? status, string? modified, CancellationToken cancellationToken = default); Task> GetArchivedGroup(string groupId, string? status, string? modified, CancellationToken cancellationToken = default); diff --git a/src/ServiceControl/MessageFailures/Api/ArchiveMessagesController.cs b/src/ServiceControl/MessageFailures/Api/ArchiveMessagesController.cs index 4c2770ff2f..f74a748d95 100644 --- a/src/ServiceControl/MessageFailures/Api/ArchiveMessagesController.cs +++ b/src/ServiceControl/MessageFailures/Api/ArchiveMessagesController.cs @@ -10,6 +10,7 @@ namespace ServiceControl.MessageFailures.Api using Microsoft.AspNetCore.Mvc; using NServiceBus; using ServiceControl.Persistence; + using ServiceControl.Persistence.Infrastructure; using ServiceControl.Recoverability; [ApiController] @@ -47,7 +48,10 @@ await auditLog.AuditedOperation(user, MessageActionKind.Archive, Permissions.Err [HttpGet] public async Task GetArchiveMessageGroups(string classifier = "Exception Type and Stack Trace", CancellationToken cancellationToken = default) { - var result = await dataStore.GetArchivedGroupsByClassifier(classifier, cancellationToken); + // Returning a fixed first page containing the most recently failed groups is a deliberate choice, the assumption is that an actively maintained production + // system will have a relatively small number of distinct groups and archived messages by default have a relatively short retention window + var pagingInfo = new PagingInfo(page: 1, pageSize: 200); + var result = await dataStore.GetArchivedGroupsByClassifier(classifier, pagingInfo, cancellationToken); Response.WithQueryStatsInfo(result.QueryStats); diff --git a/src/ServiceControl/Recoverability/API/FailureGroupsController.cs b/src/ServiceControl/Recoverability/API/FailureGroupsController.cs index 7244951eec..bf105537a1 100644 --- a/src/ServiceControl/Recoverability/API/FailureGroupsController.cs +++ b/src/ServiceControl/Recoverability/API/FailureGroupsController.cs @@ -67,7 +67,11 @@ public async Task GetAllGroups(string classifier = "Exception classifierFilter = null; } - var results = await fetcher.GetGroups(classifier, classifierFilter, cancellationToken); + // Returning a fixed first page containing the most recently failed groups is a deliberate choice. + // The assumption is that an actively maintained production system will have a relatively small number of active failure + // groups, and the most recently failed are the most important to show + var pagingInfo = new PagingInfo(page: 1, pageSize: 200); + var results = await fetcher.GetGroups(classifier, classifierFilter, pagingInfo, cancellationToken); Response.WithQueryStatsInfo(results.ToQueryStatsInfo("groups", results.Length)); return results; } diff --git a/src/ServiceControl/Recoverability/API/GroupFetcher.cs b/src/ServiceControl/Recoverability/API/GroupFetcher.cs index 949ad59444..b68b2b6efc 100644 --- a/src/ServiceControl/Recoverability/API/GroupFetcher.cs +++ b/src/ServiceControl/Recoverability/API/GroupFetcher.cs @@ -5,6 +5,7 @@ using System.Threading; using System.Threading.Tasks; using ServiceControl.Persistence; + using ServiceControl.Persistence.Infrastructure; using ServiceControl.Persistence.Recoverability; public class GroupFetcher @@ -18,9 +19,9 @@ public GroupFetcher(IGroupsDataStore store, IRetryHistoryDataStore retryStore, I this.archiver = archiver; } - public async Task GetGroups(string classifier, string classifierFilter, CancellationToken cancellationToken = default) + public async Task GetGroups(string classifier, string classifierFilter, PagingInfo pagingInfo, CancellationToken cancellationToken = default) { - var dbGroups = await store.GetUnresolvedGroupsByClassifier(classifier, classifierFilter, cancellationToken); + var dbGroups = await store.GetUnresolvedGroupsByClassifier(classifier, classifierFilter, pagingInfo, cancellationToken); var retryHistory = (await retryStore.GetRetryHistory(cancellationToken)).Results; var unacknowledgedRetries = retryHistory.GetUnacknowledgedByClassifier(classifier);