Skip to content

Implement retry failure store - #5730

Merged
rbev merged 3 commits into
masterfrom
rhys/retry-failures
Aug 12, 2026
Merged

rbev merged 3 commits into
masterfrom
rhys/retry-failures

Conversation

@rbev

@rbev rbev commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

This pull request implements the EF Core-backed FailedMessageRetryDataStore with full support for retry claim management and message body retrieval, replacing previous stubs. It also adds tests to verify the new behaviors, ensuring correct and idempotent handling of retry and message bodies.

EF Core Data Store Implementation:

  • Implemented all methods in FailedMessageRetryDataStore, including RemoveFailedMessageRetry, GetRetryPendingMessages, ProcessPendingRetries, and GetFailedMessageBody, using efficient EF Core queries and ensuring correct, idempotent behavior in line with RavenDB semantics.
  • GetFailedMessageBody now fetches message bodies from external storage when needed and throws clear exceptions for missing bodies or invalid requests.

Testing Enhancements:

  • Added FailedMessageRetryDataStoreTests
  • Added FailedMessageRetryBodyDataStoreTests

@rbev rbev self-assigned this Aug 11, 2026
@rbev
rbev marked this pull request as ready for review August 11, 2026 04:38

@johnsimons johnsimons left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine, just a small comment regarding allocations

}
await using (result.Stream)
{
using var memoryStream = new MemoryStream();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to avoid this allocation? We may need to change the signature!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also missing the in-table body storage logic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After additional investigation this is fine, but this method may be better off being removed in favor of directly using IBodyStorage at the call site.

@rbev
rbev force-pushed the rhys/retry-failures branch from b7f7800 to 140f024 Compare August 12, 2026 01:43
@rbev
rbev enabled auto-merge August 12, 2026 01:44
@rbev
rbev merged commit efb7f20 into master Aug 12, 2026
50 checks passed
@rbev
rbev deleted the rhys/retry-failures branch August 12, 2026 01: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