Skip to content

enable SqlOrchestrationService.PurgeInstanceStateAsync to delete more than 1000 instances#308

Merged
cgillum merged 2 commits intomicrosoft:mainfrom
usemam:fix-issue-297
Apr 22, 2026
Merged

enable SqlOrchestrationService.PurgeInstanceStateAsync to delete more than 1000 instances#308
cgillum merged 2 commits intomicrosoft:mainfrom
usemam:fix-issue-297

Conversation

@usemam
Copy link
Copy Markdown
Contributor

@usemam usemam commented Apr 21, 2026

Supposed to fix #297.

Summary of changes

  • New PurgeInstanceStateByFilter stored procedure (logic.sql): Accepts @CreatedTimeFrom, @CreatedTimeTo, and @RuntimeStatusFilter parameters, selects all matching instance IDs server-side, and delegates deletion to the existing PurgeInstanceStateByID sproc. This follows the same pattern as the existing PurgeInstanceStateByTime procedure.
  • Rewritten PurgeInstanceStateAsync(PurgeInstanceFilter) (SqlOrchestrationService.cs): Replaced the two-step query-then-delete approach (limited to 1000) with a single call to the new stored procedure, removing the artificial limit entirely.
  • Integration test (DataRetentionTests.cs): Added PurgesMoreThanOneThousandInstances test that creates 1100 completed orchestrations and verifies all are purged in a single PurgeInstanceStateAsync call.

Copy link
Copy Markdown
Member

@cgillum cgillum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR! One concern I have is that the list of instances to be purged could grow extremely large, in which case this transaction may never be able to succeed due to memory constraints on the database. Should we consider a design where we keep the original implementation (which includes the 1000 PageSize limit) but run it in a loop so that we purge in batches of 1000, thus capping the maximum size of the database transaction?

…ing to keep smaller SQL transactions when purging instances by filter
@usemam
Copy link
Copy Markdown
Contributor Author

usemam commented Apr 22, 2026

Sounds good. I was initially apprehensive about iterative approach, but in the case of extremely large pool of instances to be purged I'd agree that it'll perform better.
Pushed my latest changes - please take a look when possible.
Thank you!

Copy link
Copy Markdown
Member

@cgillum cgillum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@cgillum
Copy link
Copy Markdown
Member

cgillum commented Apr 22, 2026

I just now triggered the CI to validate the changes.

@cgillum cgillum merged commit 1389219 into microsoft:main Apr 22, 2026
2 checks passed
@usemam usemam deleted the fix-issue-297 branch April 23, 2026 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PurgeAllInstancesAsync - Durable SQL Provider - Deletes only 1000 instances maximum

2 participants