Skip to content

EF persistence for failed messages and streamline known endpoints - #5638

Merged
johnsimons merged 5 commits into
masterfrom
john/ingest_again
Jul 24, 2026
Merged

EF persistence for failed messages and streamline known endpoints#5638
johnsimons merged 5 commits into
masterfrom
john/ingest_again

Conversation

@johnsimons

Copy link
Copy Markdown
Member

This change introduces the core EF Core schema and entities for storing failed messages, including their associated groups and retry information. It also sets up full-text search capabilities for failed message headers, bodies, and message types in both PostgreSQL and SQL Server.

The separate 'insert-only' table and background reconciler for KnownEndpoints have been removed, simplifying how known endpoints are managed. A new MaxBodySizeToStore setting is added to control message body storage.

@johnsimons
johnsimons marked this pull request as ready for review July 23, 2026 01:56

@rbev rbev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

Comment thread docs/error-ingestion-design.md Outdated
Comment thread docs/error-ingestion-design.md Outdated
const string CatalogName = "ServiceControlFullTextCatalog";

// Both statements are guarded: an instance without Full-Text Search installed still migrates,
// it just has no full text index. They also cannot run inside a transaction, so the migration

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we actually need to support instances without FTS?
If so we probably need to surface that as an error and have a way to recreate this index so that there is an upgrade path.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am not sure, I remember as part of the spikes we allowed customers to configure it.
@warwickschroeder any thoughts?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We can make a decision, later on this?
Without FTS, they would get no search at all.
I don't think we want to support LIKE %blah% !
From our spikes FTS was not an issue at all, so i think we can make it a required thing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not having it as an option gives the user the option to not having to store the body in the DB and only use cheap external storage. That may be worth it?

Comment thread src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs Outdated
This change introduces the core EF Core schema and entities for storing failed messages, including their associated groups and retry information. It also sets up full-text search capabilities for failed message headers, bodies, and message types in both PostgreSQL and SQL Server.

The separate 'insert-only' table and background reconciler for KnownEndpoints have been removed, simplifying how known endpoints are managed. A new `MaxBodySizeToStore` setting is added to control message body storage.
This change completes the error ingestion process for PostgreSQL and SQL Server persisters, enabling robust and concurrent storage of failed messages. It introduces a detailed design document (`error-ingestion-design.md`) outlining the complex write path.
Adds a `RetentionSweeper` background service to automatically delete resolved and archived failed messages that have exceeded the configured retention period. The sweeper runs periodically in batches, handling external message bodies and tolerating body deletion failures.

@warwickschroeder warwickschroeder left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review so far. Not finished yet. Will review the rest tonight/tomorrow.

Comment thread docs/error-ingestion-design.md
<Compile Include="..\ServiceControl.Persistence.Tests\**\*.cs" LinkBase="Shared" />

<!-- Tests of behaviour specific to the EF Core persisters, shared by their two test projects only -->
<Compile Remove="..\ServiceControl.Persistence.Tests\EFCore\**\*.cs" />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@DavidBoike the reason to go with this pattern is so we can share tests between both PostgreSQL and SqlServer tests.
What do you mean by further split?

@johnsimons
johnsimons merged commit 3060221 into master Jul 24, 2026
37 checks passed
@johnsimons
johnsimons deleted the john/ingest_again branch July 24, 2026 03:42
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.

4 participants