Feature/durable#2
Conversation
jsedlak
commented
Feb 26, 2026
jsedlak
commented
- Adds use of Durable framework back in
- Updates to .NET 10
There was a problem hiding this comment.
Pull request overview
Reintroduces Orleans Durable framework support in JournaledGrain while upgrading the library and test projects to .NET 10 / Orleans 10-era dependencies.
Changes:
- Upgrade target frameworks to
net10.0and update core/test package references. - Refactor
JournaledGrainto useDurableGrain, durable collections, logging, and reminder-based outbox retry. - Remove old (commented-out) Sidecar test scaffolding and add test cluster console logging.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Strata/Strata.csproj | Updates TFMs/packages and adds reminders dependency for reminder-based outbox handling. |
| src/Strata/JournaledGrain.cs | Switches to Durable state primitives, adds logging, refactors outbox processing + reminder retry. |
| src/Strata/IJournaledGrain.cs | Removes ProcessOutbox() from the public interface. |
| src/Strata/EventEnvelope.cs | Changes event timestamp type to DateTimeOffset. |
| src/Strata.Journaling.Tests/Strata.Journaling.Tests.csproj | Updates test TFM and test-related package references. |
| src/Strata.Journaling.Tests/JournalingTests/JournalingTestFixture.cs | Configures console logging for the in-process Orleans test cluster. |
| src/Strata.Journaling.Tests/SidecarTests/* | Removes legacy commented-out Sidecar test files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public async Task ReceiveReminder(string reminderName, TickStatus status) | ||
| { | ||
| if(reminderName == OutboxCleanupReminderName) | ||
| { | ||
| _logger.LogInformation("Received reminder for outbox cleanup."); | ||
|
|
||
| var reminder = await this.GetReminder(OutboxCleanupReminderName); | ||
| if(reminder is not null) | ||
| { | ||
| await this.UnregisterReminder(reminder); | ||
| } | ||
|
|
||
| await TryProcessOutbox(); | ||
| } |
There was a problem hiding this comment.
The new reminder-based retry path (OnDestroyState registering OutboxCleanupReminderName + ReceiveReminder triggering TryProcessOutbox) isn't covered by tests. Consider adding a test which forces an outbox delivery failure, deactivates the grain, and asserts the reminder reactivates the grain and retries (and then unregisters itself).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 13 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| //private Task? _outboxProcessingTask = null; | ||
| // private ConcurrentBag<Task> _backgroundTasks = new(); |
There was a problem hiding this comment.
These commented-out lines of code should be removed. Leaving large blocks of commented code in production code reduces readability and maintainability. If this functionality may be needed in the future, it should be tracked in an issue or removed and recovered from version control if needed.
| // _backgroundTasks.Add(newProcessingTask); | ||
|
|
There was a problem hiding this comment.
This commented-out line should be removed for the same reasons as the other commented code - it reduces code readability and maintainability.
| // _backgroundTasks.Add(newProcessingTask); |
