Skip to content

fix(journaling): bound Azure Table catalog key literals - #11257

Merged
ReubenBond merged 1 commit into
dotnet:mainfrom
ReubenBond:rb-feat-journaling-catalog-queries
Sep 15, 2026
Merged

ReubenBond merged 1 commit into
dotnet:mainfrom
ReubenBond:rb-feat-journaling-catalog-queries

Conversation

@ReubenBond

@ReubenBond ReubenBond commented Sep 15, 2026 •

Copy link
Copy Markdown
Member

Problem

Default-mapped Azure Table journal IDs support up to 512 characters, which encode to 1,024-character partition keys. Catalog prefix filters appended a sentinel beyond that limit, and longer range bounds also produced oversized PartitionKey literals.

Solution

Keep every default-mapping PartitionKey filter literal within 1,024 characters while preserving ordinal range semantics:

  • A 512-character prefix uses an inclusive upper comparison against its exact encoded key. Longer prefixes return an empty result before client access.
  • Longer bounds use their first 512 characters, with a strict lower comparison and an inclusive upper comparison. Existing unsupported-character boundary projection remains in effect.
  • Custom mappings retain their canonical JournalId property filters.

Regression coverage enforces literal lengths and exact native result sets for maximum-length prefixes, oversized bounds, unsupported characters around the cutoff, and long custom-mapped IDs. The provider README describes these query semantics.

Scope

The catalog foundation originally extracted from #11210 landed through #11258. After rebasing onto main, this PR contains only the remaining Azure Table key-limit correction, its focused regressions, and provider documentation.

Microsoft Reviewers: Open in CodeFlow

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

The critical compatibility and migration issue for existing data remains unresolved.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 1 High severity

Open (1)
What changed in this PR

Introduces a journaling catalog API with range-aware enumeration, metadata snapshots, and optimized provider-specific storage layouts and queries.

Changes:

  • Adds catalog entries, range options, and shared filtering logic.
  • Optimizes Volatile, Azure Blob/Table, Redis, and S3 enumeration.
  • Updates Durable Jobs integration, tests, documentation, and generated APIs.

Critical finding: New default layouts may make existing data undiscoverable after upgrades. Add dual-read compatibility or explicit migration/backfill guidance, especially for Redis.

File Reviewed change
test/​Orleans.Journaling.Tests/​VolatileJournalStorageProviderTests.cs Volatile catalog tests
test/​Orleans.Journaling.Tests/​S3JournalStorageTests.cs S3 listing and bounds tests
test/​Orleans.Journaling.Tests/​JournalStorageCatalogTests.cs Cross-provider catalog tests
test/​Orleans.Journaling.Tests/​JournalCatalogRangeTests.cs Range behavior tests
test/​Orleans.Journaling.Tests/​AzureTableJournalStorageTests.cs Azure Table persistence tests
test/​Orleans.Journaling.Tests/​AzureTableJournalStorageProviderTests.cs Azure Table catalog tests
test/​Orleans.Journaling.Tests/​AzureTableJournalStorageOptionsTests.cs Table mapping tests
test/​Orleans.Journaling.Tests/​AzureBlobJournalStorageTests.cs Azure Blob storage tests
test/​Orleans.DurableJobs.Tests/​DurableJobs/​JournaledJobShardManagerTests.cs Durable Jobs catalog bridge tests
test/​Extensions/​Orleans.Redis.Tests/​Journaling/​RedisJournalStorageTests.cs Redis storage tests
test/​Extensions/​Orleans.Redis.Tests/​Journaling/​RedisJournalStorageCatalogTests.cs Redis catalog tests
src/​Redis/​Orleans.Journaling.Redis/​RedisJournalStorageProvider.cs Redis catalog scanning and filtering
src/​Redis/​Orleans.Journaling.Redis/​RedisJournalStorageOptions.cs Redis key mapping options
src/​Redis/​Orleans.Journaling.Redis/​RedisJournalStorage.cs Redis key layout and storage
src/​Redis/​Orleans.Journaling.Redis/​README.md Redis layout documentation
src/​Orleans.Journaling/​VolatileJournalStorage.cs Ordered enumeration and snapshots
src/​Orleans.Journaling/​README.md Catalog and provider documentation
src/​Orleans.Journaling/​Orleans.Journaling.csproj Journaling project configuration
src/​Orleans.Journaling/​ListOptions.cs Catalog filtering options
src/​Orleans.Journaling/​JournalCatalogRange.cs Shared range logic
src/​Orleans.Journaling/​JournalCatalogEntry.cs Catalog entry model
src/​Orleans.Journaling/​IJournalStorageCatalog.cs Catalog interface
src/​Orleans.DurableJobs/​JournaledJobShardManager.cs Catalog entry integration
src/​Azure/​Orleans.Journaling.AzureStorage/​README.md Azure storage documentation
src/​Azure/​Orleans.Journaling.AzureStorage/​AzureTableJournalStorageProvider.cs Table range queries and projections
src/​Azure/​Orleans.Journaling.AzureStorage/​AzureTableJournalStorageOptions.cs Table partition mapping
src/​Azure/​Orleans.Journaling.AzureStorage/​AzureTableJournalStorage.cs Table persistence and metadata
src/​Azure/​Orleans.Journaling.AzureStorage/​AzureBlobJournalStorageProvider.cs Blob listing and metadata projection
src/​Azure/​Orleans.Journaling.AzureStorage/​AzureBlobJournalStorageOptions.cs Blob listing options
src/​Azure/​Orleans.Journaling.AzureStorage/​AzureBlobJournalStorageLayout.cs Blob naming layout
src/​Azure/​Orleans.Journaling.AzureStorage/​AzureBlobJournalStorage.cs Blob storage and namespaces
src/​AWS/​Orleans.Journaling.S3/​S3JournalStorageProvider.cs S3 pagination and bounds
src/​AWS/​Orleans.Journaling.S3/​S3JournalStorageOptions.cs S3 mapping and listing options
src/​AWS/​Orleans.Journaling.S3/​README.md S3 layout documentation
src/​api/​Orleans.Journaling/​Orleans.Journaling.cs Generated core API
src/​api/​AWS/​Orleans.Journaling.S3/​Orleans.Journaling.S3.cs Generated S3 API
docs/​site/​src/​content/​docs/​grains/​journaling/​azure-storage.md Azure user documentation

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Orleans.Journaling/README.md
@github-actions

github-actions Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage

Metric Pull request
Lines 81.78% (110,816 / 135,503)
Branches 70.80% (31,665 / 44,722)

Report-only conclusion: current-main baseline stale.

The newest successful coverage run tested 4fa1669, not current main 437bb05.

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

Copilot AI review requested due to automatic review settings September 15, 2026 09:20

Copilot AI left a comment

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.

Copilot review overview

🔵 Needs a closer look

Two unresolved moderate findings remain in Azure Blob container enumeration and Redis journal-ID encoding.

Review tier: Lite
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Redis key encoding rejects accepted JournalId values

src/​Redis/​Orleans.Journaling.Redis/​RedisJournalStorage.cs:631

JournalId(string) accepts any non-blank UTF-16 value, including unpaired surrogates, but Uri.EscapeDataString throws for those values. Since GetJournalBaseKey is reached by CreateStorage, this layout change makes otherwise accepted journal ids fail before any Redis operation; use a reversible encoding which preserves UTF-16 code units (and update the decode path), or explicitly validate this input at the public boundary.

Copilot AI review requested due to automatic review settings September 15, 2026 14:53

Copilot AI left a comment

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.

Copilot review overview

🔵 Needs a closer look

It changes multiple storage providers, layouts, and pagination semantics, warranting final human validation.

Review tier: Lite
Findings: None

Copilot AI review requested due to automatic review settings September 15, 2026 17:52

Copilot AI left a comment

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

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.

Copilot review overview

🔵 Needs a closer look

Redis key encoding fails for valid raw journal IDs containing unpaired UTF-16 surrogates.

Review tier: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Handle unpaired surrogates in Redis key names

src/​Redis/​Orleans.Journaling.Redis/​RedisJournalStorage.cs:631

JournalId(string) permits raw UTF-16 values containing unpaired surrogates, but Uri.EscapeDataString(keyName) throws UriFormatException for those values. Since the constructor computes these keys immediately, a previously accepted journal ID now fails in CreateStorage, and the new catalog range code explicitly preserves such ordinal IDs. Either use a reversible encoding for arbitrary UTF-16 code units or validate and document a Redis-specific restriction before key construction.

Copilot AI review requested due to automatic review settings September 15, 2026 19:31
@ReubenBond
ReubenBond force-pushed the rb-feat-journaling-catalog-queries branch from 0e6ee43 to 783d695 Compare September 15, 2026 19:31

Copilot AI left a comment

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI review requested due to automatic review settings September 15, 2026 19:43
@ReubenBond
ReubenBond force-pushed the rb-feat-journaling-catalog-queries branch from 783d695 to 413f898 Compare September 15, 2026 19:43
@ReubenBond

Copy link
Copy Markdown
Member Author

Addressed the Redis raw UTF-16 review feedback in 413f898. Unicode scalars retain URI escaping; unpaired surrogates use reversible %uXXXX escapes, with literal percent signs escaped separately. Stored $journal-id values use the same encoding, preserving identities through Redis string transport and custom-mapped discovery. Canonical decoding rejects malformed keys explicitly.

The focused catalog and storage-transport tests passed on .NET 8 and .NET 10. Both real-Redis CI jobs also passed at 783d695, and their TRX reports confirm RawUtf16Identities_RoundTripThroughStorageAndCatalog passed for both default and custom mappings on both frameworks.

The branch is now rebased onto current main 8905026. That final synchronization changes only four upstream ADO.NET SQL files; the Redis implementation and tests are byte-identical to the CI-validated version. All five PR patches are unchanged in range-diff. Fresh checks for final head 413f898 are running.

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Azure Table catalog filters can exceed the service’s PartitionKey length limit for supported prefixes and range bounds.

Get a fresh assessment by requesting another Copilot review.

Review tier: Balanced
Findings: 1 Medium severity

Open (1)

Copilot AI review requested due to automatic review settings September 15, 2026 20:33

Copilot AI left a comment

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.

Copilot review overview

🔵 Needs a closer look

Cross-provider layout migrations and ordering assumptions warrant final human validation despite extensive coverage.

Review tier: Balanced
Findings: None

Resolved since last review (1)

Copilot AI review requested due to automatic review settings September 15, 2026 21:52
@ReubenBond
ReubenBond force-pushed the rb-feat-journaling-catalog-queries branch from 73d2a62 to 702d1ff Compare September 15, 2026 21:52
@ReubenBond ReubenBond changed the title feat(journaling): optimize catalog queries and storage layouts fix(journaling): bound Azure Table catalog key literals Sep 15, 2026

Copilot AI left a comment

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.

Copilot review overview

🟢 Approval recommended

The focused implementation preserves range semantics and includes comprehensive boundary coverage.

Review tier: Balanced
Findings: None

@ReubenBond
ReubenBond merged commit 3654f04 into dotnet:main Sep 15, 2026
66 of 67 checks passed
@ReubenBond
ReubenBond deleted the rb-feat-journaling-catalog-queries branch September 15, 2026 21:54
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.

2 participants