Fix health check registration in SQL persistence hosting extensions #549
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…egistration This refactoring addresses the issue where health checks were never registered because AkkaPersistenceJournalBuilder.Build() was never being called in the hosting extensions. Changes: - Refactored bottom-level WithSqlPersistence method to use Akka.Persistence.Hosting's unified API (.WithJournal, .WithSnapshot, .WithJournalAndSnapshot) - Updated top-level overloads to chain to the unified implementation, eliminating intermediate builder assignment pattern - The unified API properly calls .Build() internally, ensuring health checks are registered Benefits: - Health checks now work correctly for SQL persistence - Single source of truth - all overloads chain to one implementation - Reduced code duplication (~30 lines eliminated) - Maintains 100% backward compatibility with existing method signatures Testing: - All 9 existing tests continue to pass - Added BaselineJournalBuilderSpec to validate basic persistence functionality - Added HealthCheckSpec to verify health checks are registered and healthy - Total: 11 tests passing
…ions
- Upgrade AkkaHostingVersion from 1.5.51 to 1.5.51.1
- This version includes the fix for issue #666 where journal health checks
require event adapters to be registered
- Update HealthCheckSpec to verify exactly 2 health checks (journal + snapshot)
instead of "at least one"
- Add specific assertions for both journal and snapshot health checks
- All 12 hosting tests now pass
- Update parameter names from 'journal'/'snapshot' to 'journalBuilder'/'snapshotBuilder' - Correct version requirement to v1.5.51.1 (not v1.5.51) - Add note about version requirement for health checks without event adapters - Simplify code examples and fix formatting
Aaronontheweb
commented
Oct 2, 2025
| PersistenceMode mode = PersistenceMode.Both, | ||
| string? schemaName = null, | ||
| Action<AkkaPersistenceJournalBuilder>? journalBuilder = null, | ||
| Action<AkkaPersistenceSnapshotBuilder>? snapshotBuilder = null, |
There was a problem hiding this comment.
Technically a bit of a breaking change but honestly this method is awful as-is. Would be way better off deprecating and removing it - just force people to use the options types.
| public static AkkaConfigurationBuilder WithSqlPersistence( | ||
| this AkkaConfigurationBuilder builder, | ||
| SqlJournalOptions? journalOptions = null, | ||
| SqlSnapshotOptions? snapshotOptions = null) |
There was a problem hiding this comment.
This is now the "source of truth" method for the rest of the calls.
| journalOpt.DatabaseOptions.JournalTable.UseWriterUuidColumn = useWriterUuidColumn; | ||
| } | ||
|
|
||
| var adapters = new AkkaPersistenceJournalBuilder(journalOpt.Identifier, builder); |
There was a problem hiding this comment.
This is why health checks didn't work - we weren't calling down to the appropriate base method to apply the journal builder correctly.
| /// <summary> | ||
| /// Validates that health checks are properly registered after the refactoring | ||
| /// </summary> | ||
| public class HealthCheckSpec : Akka.Hosting.TestKit.TestKit, IClassFixture<SqliteContainer> |
There was a problem hiding this comment.
Validates that health checks actually get registered and run
| Output?.WriteLine($" - {entry.Key}: {entry.Value.Status}"); | ||
| } | ||
|
|
||
| // We should have exactly 2 health checks: journal and snapshot |
There was a problem hiding this comment.
Validate that BOTH health checks (journal + snapshot store) run: this is what helped me find akkadotnet/Akka.Hosting#666
| <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally> | ||
| <AkkaVersion>1.5.51</AkkaVersion> | ||
| <AkkaHostingVersion>1.5.51</AkkaHostingVersion> | ||
| <AkkaHostingVersion>1.5.51.1</AkkaHostingVersion> |
There was a problem hiding this comment.
Had to upgrade to a newer version of Akka.Hosting to resolve the health check registration bug.
| snapshot: s => s.WithHealthCheck( | ||
| unHealthyStatus: HealthStatus.Degraded, | ||
| name: "sql-snapshot")); | ||
| builder.WithSqlPersistence( |
There was a problem hiding this comment.
Cleaned up the documentation some
Aaronontheweb
commented
Oct 2, 2025
| journalOpt.DatabaseOptions.JournalTable.UseWriterUuidColumn = useWriterUuidColumn; | ||
| } | ||
|
|
||
| var adapters = new AkkaPersistenceJournalBuilder(journalOpt.Identifier, builder); |
There was a problem hiding this comment.
We now use standard base methods from Akka.Hosting for this
Merged
This was referenced Oct 9, 2025
This was referenced Oct 10, 2025
This was referenced Oct 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes health check registration in
Akka.Persistence.Sql.Hostingby refactoring to use the unified API fromAkka.Persistence.Hosting(v1.5.51).Problem
Health checks were never being registered because
AkkaPersistenceJournalBuilder.Build()was never called. The hosting extension methods created builder instances and assigned them tojournalOpt.Adapters, but never invoked.Build(), which is responsible for:Solution
Refactored all
WithSqlPersistenceoverloads to use the unified API methods:.WithJournal(JournalOptions, Action<AkkaPersistenceJournalBuilder>?).WithSnapshot(SnapshotOptions, Action<AkkaPersistenceSnapshotBuilder>?).WithJournalAndSnapshot(...)These methods properly call
.Build()internally, ensuring health checks are registered.Changes
WithSqlPersistence(SqlJournalOptions?, SqlSnapshotOptions?, ...)to use unified APIBenefits
Testing
BaselineJournalBuilderSpecto validate basic persistence functionalityHealthCheckSpecto verify health checks are registered and return healthy statusBreaking Changes
None - this is fully backward compatible.