Skip to content

Add custom check for failing retention - #5903

Merged
rbev merged 7 commits into
masterfrom
rentention-notify
Sep 23, 2026
Merged

rbev merged 7 commits into
masterfrom
rentention-notify

Conversation

@rbev

@rbev rbev commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

This pull request introduces a new custom check for monitoring retention sweep failures in the EF Core persistence layer. It tracks and reports failures in the retention process, ensuring that any issues are surfaced via the custom checks infrastructure and are only cleared after a fully successful sweep. The changes include the implementation of the custom check, supporting logic in the sweeper, updates to configuration, and comprehensive tests.

Retention Sweep Monitoring and Reporting:

  • Introduced a new RetentionSweepCustomCheck class that reports failures in the retention sweep process as a custom check, failing after any retention pass fails and recovering after the next fully successful sweep. This check summarizes failures per entity and integrates with the custom check infrastructure. [1] [2]
  • Updated RetentionSweeper to track active failures per entity, provide methods for recording and retrieving failures, and clear failures only after all passes succeed in a sweep. The sweeper now returns pass/fail status for each retention entity and records reasons for failures. [1] [2] [3]

Testing and Verification:

  • Added a new test suite RetentionSweepCustomCheckTests to verify the behavior of the custom check, including its response to failures, recovery, time persistence, and deduplication of state change events.
  • Extended existing retention sweep tests to verify that failing passes are correctly tracked and reported.

Configuration and Integration:

  • Registered the new custom check in the DI container and updated the list of internal custom checks to include "ServiceControl Retention". [1] [2] [3]
  • Updated app configuration to use SQL Server persistence and provide necessary connection strings and storage paths for local development. [1] [2]

Test Artifacts:

  • Updated approval files for PostgreSQL and SQL Server to reflect the presence of the new custom check in the list of available checks. [1] [2]

@rbev
rbev marked this pull request as ready for review September 17, 2026 06:39

@johnsimons johnsimons left a comment

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.

From a coding perspective this PR is fine, no issues.
But displaying a custom check without an action plan for the customer to address it is not going to help, we may as well tell them to raise a support case.

This may need more thought.

catch (Exception ex) when (ex is not OperationCanceledException)
{
logger.LogError(ex, "Error during the {RetentionEntity} retention pass", entity);
RecordFailure(entity, ex.Message);

@johnsimons johnsimons Sep 17, 2026 •

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.

We are in essence displaying ex.Message to customers, is that going to be useful for them to solve the problem?
Will it prevent them from raising a support case with us?
What action should they take if they see such error in ServicePulse?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you have any suggestions to what action we could give them?

The main thing that I'd see this failing on would either be deadlocks or timeouts from an overloaded DB, a completely unreachable DB takes ServicePulse offline equivalent to the SC instance disappearing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a link pointing to the troubleshooting docs

Comment thread src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs Outdated
}
}

public RetentionSweepCurrentStatus GetStatus() => new(isRunning, lastStartedAt, lastFinishedAt, lastErrorCutoff, lastEventsCutoff);

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.

Will the custom check pass if the retention never starts? Could we also raise a failure for this as well?

Comment thread src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs Outdated

return Task.FromResult(state.ConsecutiveFailedSweeps < 3
? CheckResult.Pass
: CheckResult.Failed($"Retention processing has failures. Last failure per entity: {failureSummary}. See https://docs.particular.net/servicecontrol/troubleshooting for guidance on resolving the issue."));

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.

We may need to adjust ServicePulse to render these links correctly

@rbev
rbev merged commit f17ea66 into master Sep 23, 2026
69 of 70 checks passed
@rbev
rbev deleted the rentention-notify branch September 23, 2026 05:00
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