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 )