State migration - #9
Conversation
…ng, Egil.Orleans.Storage, and Egil.StronglyTypedPrimitives with package versions and analyzers
Add BenchmarkDotNet perf tests that compare Egil.Orleans.StateMigration against plain System.Text.Json for no-migration scenarios. Includes minimal and complex state models, source-generated context variants, enveloped vs flattened payload layout benchmarks, and solution migration to .slnx.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new library, Egil.Orleans.StateMigration, which provides versioned JSON type metadata support for Orleans persistent state migrations. The library enables applications to migrate between different state type versions without embedding migration logic directly in grain implementations.
Changes:
- Introduces a complete state migration framework with
Storage<T>wrapper,IMigrateFrom<,>andIMigrate<,>interfaces for defining migrations - Implements System.Text.Json converters supporting both enveloped and flattened payload layouts with optimized hot-path for current-type deserialization
- Adds comprehensive test suite covering serialization, deserialization, migration scenarios, and Orleans in-process cluster integration
- Includes performance benchmarks comparing reflection-based and source-generated approaches
- Provides CI/CD workflow, package configuration, and extensive documentation
Reviewed changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| Egil.Orleans.StateMigration/Directory.Packages.props | Central package version management for the new library |
| Egil.Orleans.StateMigration/version.json | Version configuration for Nerdbank.GitVersioning |
| Egil.Orleans.StateMigration/src/Egil.Orleans.StateMigration/*.cs | Core library implementation including Storage wrapper, migration interfaces, type identity resolution, and DI extensions |
| Egil.Orleans.StateMigration/src/Egil.Orleans.StateMigration/SystemTextJson/*.cs | JSON converter implementations for both enveloped and flattened payload layouts |
| Egil.Orleans.StateMigration/test/Egil.Orleans.StateMigration.Tests/*.cs | Comprehensive test suite covering serialization, deserialization, migration, and Orleans integration |
| Egil.Orleans.StateMigration/perf/Egil.Orleans.StateMigration.PerfTests/*.cs | BenchmarkDotNet performance tests for hot-path scenarios |
| Egil.Orleans.StateMigration/README.md | Detailed documentation of design decisions, usage examples, and benchmarking guidance |
| Egil.Orleans.StateMigration/IMPLEMENTATION_PLAN.md | Implementation roadmap and TDD execution plan |
| .github/workflows/egil-orleans-statemigration-ci.yml | CI/CD workflow for building, testing, and releasing the package |
| Egil.StronglyTypedPrimitives/Directory.Packages.props | Package version configuration for another project (no functional changes) |
| Egil.Orleans.Storage/Directory.Packages.props | Package version configuration for another project (no functional changes) |
| Egil.Orleans.EventSourcing/Directory.Packages.props | Package version configuration for another project (no functional changes) |
Comments suppressed due to low confidence (5)
Egil.Orleans.StateMigration/README.md:1
- The README title includes "(proposal)" which suggests this is still in a proposal stage. However, the PR includes a full implementation with extensive code. Consider either:
- Removing "(proposal)" from the title if this is the final implementation
- Adding a note about the implementation status if this is still evolving
This clarity will help users understand whether the library is production-ready.
## Egil.Orleans.StateMigration (proposal)
Egil.Orleans.StateMigration/Directory.Packages.props:18
- The Microsoft.CodeAnalysis.Common and Microsoft.CodeAnalysis.CSharp package versions (5.0.0) are significantly higher than those used in other projects (4.8.0). While this project targets .NET 10, the comment in other projects indicates "Version 4.8.0 matches with the release of dotnet 8". Since this is targeting .NET 10, version 5.0.0 may be appropriate, but consider whether this version is stable and tested, or if alignment with repository conventions is more important for maintainability.
<PackageVersion Include="Microsoft.CodeAnalysis.Common" Version="5.0.0" />
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp" Version="5.0.0" />
Egil.Orleans.StateMigration/Directory.Packages.props:20
- The Microsoft.Extensions.DependencyInjection.Abstractions package version is inconsistent. This project uses version "10.0.3", while other projects use "9.0.3". For cross-project compatibility and consistency, consider aligning this version with the rest of the repository.
<PackageVersion Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="10.0.3" />
Egil.Orleans.StateMigration/README.md:54
- The guidance section recommends emitting logs/metrics when CLR-name fallback is used, but the implementation doesn't appear to include any logging infrastructure or instrumentation. Consider either:
- Implementing the logging/metrics mentioned in the documentation
- Updating the documentation to reflect that this is a recommendation for users to implement in their own code
- Providing extension points or events where users can hook in their own monitoring
Guidance:
- Aliases should be treated as immutable once data is persisted.
- Validate at startup:
- duplicate aliases
- unresolved `$type` mappings
- Emit logs/metrics when CLR-name fallback is used so systems can migrate toward aliases.
Egil.Orleans.StateMigration/src/Egil.Orleans.StateMigration/Egil.Orleans.StateMigration.csproj:28
- Package validation is disabled (
EnablePackageValidation>false</EnablePackageValidation>). For a library intended for public release, consider enabling package validation to catch potential issues with package structure, compatibility, and breaking changes. This is especially important for libraries that will have multiple versions in production.
<EnablePackageValidation>false</EnablePackageValidation>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Addressed the review findings in this branch:
Validation run:
Both pass locally. |
|
Implemented the Files updated:
What changed:
Validation:
|
|
Follow-up workflow fix pushed in commit Root cause:
Fix:
Local validation after the change:
|
No description provided.