From e6629bcfb8515d3bd2a757b1bf65aa54fa6c88ba Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Wed, 16 Sep 2026 13:30:40 +0800 Subject: [PATCH 1/5] Enable paging on group query in store, move hardcoded 200 limit into api while it does not support paging --- .../Implementation/GroupsDataStore.cs | 23 +++-- .../Infrastructure/FailureGroupQueries.cs | 2 - .../Recoverability/GroupsDataStore.cs | 13 +-- .../ArchiveCancellationTests.cs | 3 +- .../ArchivedGroupVersionTests.cs | 15 ++-- .../Recoverability/GroupsDataStoreTests.cs | 88 +++++++++++++++++-- .../IGroupsDataStore.cs | 4 +- .../Api/ArchiveMessagesController.cs | 4 +- .../API/FailureGroupsController.cs | 3 +- .../Recoverability/API/GroupFetcher.cs | 5 +- 10 files changed, 122 insertions(+), 38 deletions(-) diff --git a/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs b/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs index 861487a218..8db80e5fe6 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,19 +22,25 @@ 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 views = await GetGroupViews(groups, WithStatus(dbContext, FailedMessageStatus.Archived), token); + var views = await GetGroupViews( + ByClassifier(dbContext, classifier), + WithStatus(dbContext, FailedMessageStatus.Archived), + pagingInfo, + token); return new QueryResult>(views, views.ToQueryStatsInfo("groups", views.Count)); }, cancellationToken); @@ -128,7 +134,7 @@ static async Task AttachComments(ServiceControlDbContext dbContext, IList> GetGroupViews(IQueryable 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..f4ada9d81f 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 @@ -28,7 +29,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, new PagingInfo(page: 1, pageSize: 200)); // 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 +39,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, new PagingInfo(page: 1, pageSize: 200)); using (Assert.EnterMultipleScope()) { @@ -63,13 +64,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, new PagingInfo(page: 1, pageSize: 200)); var second = InGroup(group, Newest); await Insert(second); await Archive(second); - var after = await GroupsStore.GetArchivedGroupsByClassifier(Classifier); + var after = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, new PagingInfo(page: 1, pageSize: 200)); VersionAssert.Moved(before.QueryStats.Version, after.QueryStats.Version, "the archived group gained a message, so its validator cannot stay put"); @@ -84,8 +85,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, new PagingInfo(page: 1, pageSize: 200)); + var second = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, new PagingInfo(page: 1, pageSize: 200)); 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 +95,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, new PagingInfo(page: 1, pageSize: 200)); using (Assert.EnterMultipleScope()) { diff --git a/src/ServiceControl.Persistence.Tests/Recoverability/GroupsDataStoreTests.cs b/src/ServiceControl.Persistence.Tests/Recoverability/GroupsDataStoreTests.cs index 6502efbb4a..8b3a1a7d75 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,83 @@ 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); + } + } + [Test] public async Task Returns_a_single_group_by_id() { @@ -295,14 +367,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..469c402d0e 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,8 @@ 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); + 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..76ff24ab02 100644 --- a/src/ServiceControl/Recoverability/API/FailureGroupsController.cs +++ b/src/ServiceControl/Recoverability/API/FailureGroupsController.cs @@ -67,7 +67,8 @@ public async Task GetAllGroups(string classifier = "Exception classifierFilter = null; } - var results = await fetcher.GetGroups(classifier, classifierFilter, cancellationToken); + 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); From 8036153104620c697daffbaca92ecfc72ca25483 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Wed, 16 Sep 2026 16:06:48 +0800 Subject: [PATCH 2/5] Correctly include page size for group query in EF --- .../Implementation/GroupsDataStore.cs | 13 +++++++------ .../Recoverability/GroupsDataStoreTests.cs | 4 ++++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs b/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs index 8db80e5fe6..0529ab4d4b 100644 --- a/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs +++ b/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs @@ -36,13 +36,14 @@ public Task> GetUnresolvedGroupsByClassifier(string clas public Task>> GetArchivedGroupsByClassifier(string classifier, PagingInfo pagingInfo, CancellationToken cancellationToken = default) => ExecuteWithDbContext(async (dbContext, token) => { - var views = await GetGroupViews( - ByClassifier(dbContext, classifier), - WithStatus(dbContext, FailedMessageStatus.Archived), - pagingInfo, - token); + var groups = ByClassifier(dbContext, classifier); + var messages = WithStatus(dbContext, FailedMessageStatus.Archived); + + var views = await GetGroupViews(groups, messages, pagingInfo, token); + + var totalCount = await groups.AggregateGroupSummaries(messages).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) => diff --git a/src/ServiceControl.Persistence.Tests/Recoverability/GroupsDataStoreTests.cs b/src/ServiceControl.Persistence.Tests/Recoverability/GroupsDataStoreTests.cs index 8b3a1a7d75..c8ea445559 100644 --- a/src/ServiceControl.Persistence.Tests/Recoverability/GroupsDataStoreTests.cs +++ b/src/ServiceControl.Persistence.Tests/Recoverability/GroupsDataStoreTests.cs @@ -183,6 +183,10 @@ await Insert( 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"); } } From d9fe22bb2048f8a9db28b4b85fd0b88ff0111c67 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Wed, 16 Sep 2026 16:13:50 +0800 Subject: [PATCH 3/5] Share the paging info between all tests --- .../Recoverability/ArchivedGroupVersionTests.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/ServiceControl.Persistence.Tests/Recoverability/ArchivedGroupVersionTests.cs b/src/ServiceControl.Persistence.Tests/Recoverability/ArchivedGroupVersionTests.cs index f4ada9d81f..b1ba64df89 100644 --- a/src/ServiceControl.Persistence.Tests/Recoverability/ArchivedGroupVersionTests.cs +++ b/src/ServiceControl.Persistence.Tests/Recoverability/ArchivedGroupVersionTests.cs @@ -15,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() @@ -29,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, new PagingInfo(page: 1, pageSize: 200)); + 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. @@ -39,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, new PagingInfo(page: 1, pageSize: 200)); + var after = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, pagingInfo); using (Assert.EnterMultipleScope()) { @@ -64,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, new PagingInfo(page: 1, pageSize: 200)); + var before = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, pagingInfo); var second = InGroup(group, Newest); await Insert(second); await Archive(second); - var after = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, new PagingInfo(page: 1, pageSize: 200)); + 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"); @@ -85,8 +86,8 @@ public async Task Version_is_stable_while_nothing_changes() await Insert(failure); await Archive(failure); - var first = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, new PagingInfo(page: 1, pageSize: 200)); - var second = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, new PagingInfo(page: 1, pageSize: 200)); + 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"); @@ -95,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, new PagingInfo(page: 1, pageSize: 200)); + var result = await GroupsStore.GetArchivedGroupsByClassifier(Classifier, pagingInfo); using (Assert.EnterMultipleScope()) { From b1efad105c118baf8eef56758b3435efa7ff256f Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Thu, 17 Sep 2026 12:16:58 +0800 Subject: [PATCH 4/5] Add comment about the deliberate choice on failure groups --- .../MessageFailures/Api/ArchiveMessagesController.cs | 2 ++ .../Recoverability/API/FailureGroupsController.cs | 3 +++ 2 files changed, 5 insertions(+) diff --git a/src/ServiceControl/MessageFailures/Api/ArchiveMessagesController.cs b/src/ServiceControl/MessageFailures/Api/ArchiveMessagesController.cs index 469c402d0e..f74a748d95 100644 --- a/src/ServiceControl/MessageFailures/Api/ArchiveMessagesController.cs +++ b/src/ServiceControl/MessageFailures/Api/ArchiveMessagesController.cs @@ -48,6 +48,8 @@ await auditLog.AuditedOperation(user, MessageActionKind.Archive, Permissions.Err [HttpGet] public async Task GetArchiveMessageGroups(string classifier = "Exception Type and Stack Trace", CancellationToken cancellationToken = default) { + // 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); diff --git a/src/ServiceControl/Recoverability/API/FailureGroupsController.cs b/src/ServiceControl/Recoverability/API/FailureGroupsController.cs index 76ff24ab02..bf105537a1 100644 --- a/src/ServiceControl/Recoverability/API/FailureGroupsController.cs +++ b/src/ServiceControl/Recoverability/API/FailureGroupsController.cs @@ -67,6 +67,9 @@ public async Task GetAllGroups(string classifier = "Exception classifierFilter = null; } + // 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)); From ecb4762f73f8191ebadafb4591d827d5c433c041 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Thu, 17 Sep 2026 12:24:25 +0800 Subject: [PATCH 5/5] Fix up bad rebase --- .../Implementation/GroupsDataStore.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs b/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs index 0529ab4d4b..3de10bb85a 100644 --- a/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs +++ b/src/ServiceControl.Persistence.EFCore/Implementation/GroupsDataStore.cs @@ -41,7 +41,7 @@ public Task>> GetArchivedGroupsByClassifier( var views = await GetGroupViews(groups, messages, pagingInfo, token); - var totalCount = await groups.AggregateGroupSummaries(messages).LongCountAsync(token); + var totalCount = await groups.LongCountAsync(token); return new QueryResult>(views, views.ToQueryStatsInfo("groups", totalCount)); }, cancellationToken); @@ -139,7 +139,7 @@ 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)