fix(adonet): return scalar SQLite persistence versions - #11322
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The query changes preserve mutation and conflict behavior while ensuring a single scalar result, with comprehensive regression coverage.
Review effort: Lite
Findings: None
What changed in this PR
Fixes SQLite ADO.NET persistence version results by returning scalar mutation-derived versions, preventing duplicate storage rows from producing multiple results.
Changes:
- Updated SQLite write and clear queries.
- Added isolated regression tests for versions, duplicates, conflicts, and hash collisions.
- Documented query updates for existing databases.
| File | Description |
|---|---|
Sqlite-Persistence.sql |
Computes scalar persistence versions. |
SqlitePersistenceQueryTests.cs |
Adds SQLite query regressions. |
relational-storage.md |
Documents the contract and migration process. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code coverage
Report-only conclusion: current-main baseline stale. The newest successful coverage run tested 665c966, not current main dcf2925. Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities. The comparison remains report-only while normal line and branch variance is calibrated. Coverage details |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Existing SQLite databases are not explicitly instructed to replace both stored query rows before restarting providers.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
| The provider supplies the common identity parameters `GrainIdHash`, `GrainIdN0`, `GrainIdN1`, `GrainTypeHash`, `GrainTypeString`, `GrainIdExtensionString`, and `ServiceId`. `WriteToStorageKey` also receives `GrainStateVersion` and `PayloadBinary`; `ClearStorageKey` and the optional `DeleteStorageKey` receive `GrainStateVersion`. | ||
|
|
||
| `ReadFromStorageKey` must return columns named `PayloadBinary` and `Version`. `WriteToStorageKey` must return the resulting version as `NewGrainStateVersion`. Clear and delete queries return one version value in their first result column; its name is ignored. A successful operation returns an advanced version, while an unchanged or missing value signals an ETag conflict. Versions must be representable as a signed 32-bit integer. | ||
| `ReadFromStorageKey` must return columns named `PayloadBinary` and `Version`. `WriteToStorageKey` must return the resulting version as `NewGrainStateVersion`. Clear and delete queries return one version value in their first result column; its name is ignored. A successful operation returns exactly one row across all result sets, containing the advanced version. An unchanged or missing value signals an ETag conflict. Versions must be representable as a signed 32-bit integer. |
Problem
SQLite persistence reads the successful write/clear version back from every
OrleansStoragerow with the same full grain identity. Duplicate identities therefore produce multiple result rows, violatingAdoNetGrainStorage's single-result contract. With unequal duplicate versions, the result can also include a version from a row which the mutation did not match.Solution
Compute the successful result from the mutation:
1for a null-version insert, and the expected version plus one for a version-matched update or clear. Keep the version-qualified mutations, guarded insert, transaction boundaries, and conflict-result branches unchanged.Add isolated regressions which execute the installed SQLite query batches and consume all result sets, covering equal, unequal, and null duplicate versions, hash collisions, null-version inserts, stale/missing versions, and write-clear-write progression. Document updating both stored
OrleansQueryentries and restarting providers for existing databases.The full-identity uniqueness contract remains the same. Duplicate-data investigation and reconciliation remain a separate maintenance responsibility.
Refs #11303.
Fixes: #11303
Microsoft Reviewers: Open in CodeFlow