From e6ccda54d2ff3ded0963502991287794d2886fc9 Mon Sep 17 00:00:00 2001 From: Stephen Lee Date: Sat, 29 Aug 2026 08:38:02 -0400 Subject: [PATCH] Fix interaction retention never running on large channels CleanupOldInteractions accumulated every expiring InteractionSessionId into a List, filtering each candidate with List.Contains against the same list it was appending to. That is O(n^2), and it runs before the BulkDeleteInChunks call, so on a channel with tens of millions of expiring interactions the delete was never reached. RunCleanupTask catches the resulting exception into the job result list, so the channel's RetentionDuration was silently never enforced. Observed on a channel that reached 96M rows despite a 60 day retention. Switch the accumulator to a HashSet so the duplicate check is O(1), and use UnionWith to stream the query results into it instead of materializing every id with ToList() before filtering. CleanupUnusedInteractionSessions had two further problems: - The loop bound used integer division ( Count / 1000 ), dropping the final partial chunk on every run and skipping the work entirely when there were fewer than 1000 ids. - Chunks were taken with Skip( x * 1000 ), which re-enumerates the list from the start on each iteration, giving the same O(n^2) behaviour. Round the chunk count up and index into the list with GetRange. Co-Authored-By: Claude Opus 5 (1M context) --- Rock/Jobs/RockCleanup.cs | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/Rock/Jobs/RockCleanup.cs b/Rock/Jobs/RockCleanup.cs index ce9cfd1d2a1..49f3eca930b 100644 --- a/Rock/Jobs/RockCleanup.cs +++ b/Rock/Jobs/RockCleanup.cs @@ -1206,7 +1206,13 @@ private int CleanupOldInteractions() int totalRowsDeleted = 0; var currentDateTime = RockDateTime.Now; - var interactionSessionIdsOfDeletedInteractions = new List(); + // Use a HashSet rather than a List. The duplicate check below runs once per + // expiring session id, and List.Contains is a linear scan of a list that + // grows as ids are added to it, making the accumulation O(n^2). On a channel + // with tens of millions of expiring interactions that never completed, and + // because it runs before BulkDeleteInChunks, the channel's retention duration + // was silently never enforced. + var interactionSessionIdsOfDeletedInteractions = new HashSet(); var interactionChannelsWithRentionDurations = InteractionChannelCache.All().Where( ic => ic.RetentionDuration.HasValue ); using ( var interactionRockContext = new Rock.Data.RockContext() ) @@ -1226,14 +1232,13 @@ private int CleanupOldInteractions() i.InteractionComponent.InteractionChannelId == interactionChannel.Id && i.InteractionDateTime < retentionCutoffDateTime ); - var interactionSessionIdsForInteractionChannel = interactionsToDeleteQuery + // UnionWith streams the query results straight into the set and handles + // the de-duplication itself. The previous code called ToList() first, + // materializing every expiring session id an extra time before filtering. + interactionSessionIdsOfDeletedInteractions.UnionWith( interactionsToDeleteQuery .Where( i => i.InteractionSessionId != null ) .Select( i => ( int ) i.InteractionSessionId ) - .Distinct() - .ToList() - .Where( i => !interactionSessionIdsOfDeletedInteractions.Contains( i ) ); - - interactionSessionIdsOfDeletedInteractions.AddRange( interactionSessionIdsForInteractionChannel ); + .Distinct() ); totalRowsDeleted += BulkDeleteInChunks( interactionsToDeleteQuery, batchAmount, commandTimeout ); } @@ -1241,7 +1246,7 @@ private int CleanupOldInteractions() if ( interactionSessionIdsOfDeletedInteractions.Any() ) { - RunCleanupTask( "Unused Interaction Session Cleanup", () => CleanupUnusedInteractionSessions( interactionSessionIdsOfDeletedInteractions ) ); + RunCleanupTask( "Unused Interaction Session Cleanup", () => CleanupUnusedInteractionSessions( interactionSessionIdsOfDeletedInteractions.ToList() ) ); } return totalRowsDeleted; @@ -1267,9 +1272,18 @@ private int CleanupUnusedInteractionSessions( List interactionSessionIds ) rockContext.Database.CommandTimeout = commandTimeout; // process 1K at a time to prevent the exception "Query processor ran out of internal resources". - for ( int x = 0; x < interactionSessionIds.Count / 1000; x++ ) - { - var interactionSessionIdChunk = interactionSessionIds.Skip( x * 1000 ).Take( 1000 ); + // Round the chunk count up. The previous integer division ( Count / 1000 ) dropped the + // final partial chunk on every run, and skipped the work altogether whenever there + // were fewer than 1000 ids. + int chunkCount = ( interactionSessionIds.Count + 999 ) / 1000; + + for ( int x = 0; x < chunkCount; x++ ) + { + // GetRange indexes straight into the list. Skip( x * 1000 ) re-enumerated from + // the start of the list on every iteration, making the loop O(n^2) over the ids. + int chunkStartIndex = x * 1000; + int chunkSize = Math.Min( 1000, interactionSessionIds.Count - chunkStartIndex ); + var interactionSessionIdChunk = interactionSessionIds.GetRange( chunkStartIndex, chunkSize ); // Find a list of session IDs in the delete list that are being used for other interactions var interactionSessionsIdsToKeep = new InteractionService( rockContext )