-
-
Notifications
You must be signed in to change notification settings - Fork 748
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set limit on serialized message size in Subscriptions.Postgres #6678
Conversation
Postgres has a max limit of 8000 bytes for the paylaod in pg_notify. Exceeding this limit will result in infinite retries. This mitigates the infinite loop by blocking the messages from being queued into the subscription system. Resolves #6658
hey @PascalSenn or @michaelstaib, i was considering how to best approach this.
I think the 8000 byte limit is constant in postgres, this documentation suggests its configurable (emphasis mine)
I can't see where to configure it in postgres, but I allowed for it to be configurable in the provider with the default value set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the pr!
This looks very good and makes sense.
There is only one thing, I personally prefere a public static
factory method on the PostgresMessageEnvelope
instead of the magic constructor.
But let see what @michaelstaib thinks about it.
|
||
public string Format() | ||
private static string Format(string topic, string payload, int maxMessagePayloadSize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally would prefer a public static factory method that does the formatting and then passes topic
and payload
to the envelope ctor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, this is fair, i wasn't sure about doing that extra work in the constructor, a factory would be better. Ill make that change
@williamdenton |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #6678 +/- ##
==========================================
- Coverage 78.98% 78.12% -0.86%
==========================================
Files 2903 2541 -362
Lines 139771 127086 -12685
==========================================
- Hits 110397 99285 -11112
+ Misses 29374 27801 -1573
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Co-authored-by: Pascal Senn <senn.pasc@gmail.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [HotChocolate.Abstractions](https://chillicream.com/) ([source](https://github.com/ChilliCream/graphql-platform)) | `13.7.0` -> `13.8.0` | [![age](https://developer.mend.io/api/mc/badges/age/nuget/HotChocolate.Abstractions/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/HotChocolate.Abstractions/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/HotChocolate.Abstractions/13.7.0/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/HotChocolate.Abstractions/13.7.0/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | | [HotChocolate.AspNetCore](https://chillicream.com/) ([source](https://github.com/ChilliCream/graphql-platform)) | `13.7.0` -> `13.8.0` | [![age](https://developer.mend.io/api/mc/badges/age/nuget/HotChocolate.AspNetCore/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/HotChocolate.AspNetCore/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/HotChocolate.AspNetCore/13.7.0/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/HotChocolate.AspNetCore/13.7.0/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | | [HotChocolate.AspNetCore.Authorization](https://chillicream.com/) ([source](https://github.com/ChilliCream/graphql-platform)) | `13.7.0` -> `13.8.0` | [![age](https://developer.mend.io/api/mc/badges/age/nuget/HotChocolate.AspNetCore.Authorization/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/HotChocolate.AspNetCore.Authorization/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/HotChocolate.AspNetCore.Authorization/13.7.0/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/HotChocolate.AspNetCore.Authorization/13.7.0/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | | [HotChocolate.Data](https://chillicream.com/) ([source](https://github.com/ChilliCream/graphql-platform)) | `13.7.0` -> `13.8.0` | [![age](https://developer.mend.io/api/mc/badges/age/nuget/HotChocolate.Data/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/HotChocolate.Data/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/HotChocolate.Data/13.7.0/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/HotChocolate.Data/13.7.0/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | | [HotChocolate.Data.EntityFramework](https://chillicream.com/) ([source](https://github.com/ChilliCream/graphql-platform)) | `13.7.0` -> `13.8.0` | [![age](https://developer.mend.io/api/mc/badges/age/nuget/HotChocolate.Data.EntityFramework/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/HotChocolate.Data.EntityFramework/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/HotChocolate.Data.EntityFramework/13.7.0/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/HotChocolate.Data.EntityFramework/13.7.0/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | | [HotChocolate.Types](https://chillicream.com/) ([source](https://github.com/ChilliCream/graphql-platform)) | `13.7.0` -> `13.8.0` | [![age](https://developer.mend.io/api/mc/badges/age/nuget/HotChocolate.Types/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/HotChocolate.Types/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/HotChocolate.Types/13.7.0/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/HotChocolate.Types/13.7.0/13.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>ChilliCream/graphql-platform (HotChocolate.Abstractions)</summary> ### [`v13.8.0`](https://github.com/ChilliCream/graphql-platform/releases/tag/13.8.0) [Compare Source](https://github.com/ChilliCream/graphql-platform/compare/13.7.0...13.8.0) ##### What's Changed - Fixed issue with the fusion configuration rewriter. ([https://github.com/ChilliCream/graphql-platform/pull/6670](https://github.com/ChilliCream/graphql-platform/pull/6670)) - Adds example for failing error interface ([https://github.com/ChilliCream/graphql-platform/pull/6665](https://github.com/ChilliCream/graphql-platform/pull/6665)) - Set limit on serialized message size in Subscriptions.Postgres ([https://github.com/ChilliCream/graphql-platform/pull/6678](https://github.com/ChilliCream/graphql-platform/pull/6678)) - Fix JsonElement serialization for StrawberryShake - Reworked JSON scalar usage with StrawberryShake. ([https://github.com/ChilliCream/graphql-platform/pull/6683](https://github.com/ChilliCream/graphql-platform/pull/6683)) - Updated BananaCakePop Default Version ([https://github.com/ChilliCream/graphql-platform/pull/6687](https://github.com/ChilliCream/graphql-platform/pull/6687)) - Make postgres subscription provider compatible with npgsql 8 ([https://github.com/ChilliCream/graphql-platform/pull/6686](https://github.com/ChilliCream/graphql-platform/pull/6686)) - Handle JsonElement JsonValueKind.Null when deserializing ([https://github.com/ChilliCream/graphql-platform/pull/6481](https://github.com/ChilliCream/graphql-platform/pull/6481)) - Fixed Utf8JsonWriterHelper to not ignore null dictionary values ([https://github.com/ChilliCream/graphql-platform/pull/6689](https://github.com/ChilliCream/graphql-platform/pull/6689)) - Fixed the ghost files in Rider when using strawberyy shake ([https://github.com/ChilliCream/graphql-platform/pull/6693](https://github.com/ChilliCream/graphql-platform/pull/6693)) - Fixed dotnet version check filter non version output for Strawberry Shake ([https://github.com/ChilliCream/graphql-platform/pull/6699](https://github.com/ChilliCream/graphql-platform/pull/6699)) - Fixed argument naming conventions for mutation conventions. ([https://github.com/ChilliCream/graphql-platform/pull/6705](https://github.com/ChilliCream/graphql-platform/pull/6705)) - Fixed cache pollution for in/nin filter expressions ([https://github.com/ChilliCream/graphql-platform/pull/6711](https://github.com/ChilliCream/graphql-platform/pull/6711)) - Fixed large json read corruption ([https://github.com/ChilliCream/graphql-platform/pull/6713](https://github.com/ChilliCream/graphql-platform/pull/6713)) - Fixed typo in Banana Cake Pop docs ([https://github.com/ChilliCream/graphql-platform/pull/6715](https://github.com/ChilliCream/graphql-platform/pull/6715)) - Update open api schema snapshot to fix unit tests ([https://github.com/ChilliCream/graphql-platform/pull/6708](https://github.com/ChilliCream/graphql-platform/pull/6708)) - Fixed array writer reset ([https://github.com/ChilliCream/graphql-platform/pull/6719](https://github.com/ChilliCream/graphql-platform/pull/6719)) - \[maintenance] Empty type termination using semicolon. [https://github.com/ChilliCream/graphql-platform/pull/6725](https://github.com/ChilliCream/graphql-platform/pull/6725)5) - Added apollo federation docs ([https://github.com/ChilliCream/graphql-platform/pull/5791](https://github.com/ChilliCream/graphql-platform/pull/5791)) - Fixed example in docs ([https://github.com/ChilliCream/graphql-platform/pull/6733](https://github.com/ChilliCream/graphql-platform/pull/6733)) - StrawberryShake documentation fixes ([https://github.com/ChilliCream/graphql-platform/pull/6742](https://github.com/ChilliCream/graphql-platform/pull/6742)) - Adds more documentation for BCP ([https://github.com/ChilliCream/graphql-platform/pull/6709](https://github.com/ChilliCream/graphql-platform/pull/6709)) - Avoid writing BOM in StrawberryShake Tools ([https://github.com/ChilliCream/graphql-platform/pull/6751](https://github.com/ChilliCream/graphql-platform/pull/6751)) - Fixed transport layer byte-array handling. ([https://github.com/ChilliCream/graphql-platform/pull/6763](https://github.com/ChilliCream/graphql-platform/pull/6763)) - Fixed SSE message corruption ([https://github.com/ChilliCream/graphql-platform/pull/6642](https://github.com/ChilliCream/graphql-platform/pull/6642)) - Do not send omitted arguments with default values to subgraph ([https://github.com/ChilliCream/graphql-platform/pull/6767](https://github.com/ChilliCream/graphql-platform/pull/6767)) - Fixed mutation error dependency registration. ([https://github.com/ChilliCream/graphql-platform/pull/6674](https://github.com/ChilliCream/graphql-platform/pull/6674)) - Fixed issue with default values on mutations. ([https://github.com/ChilliCream/graphql-platform/pull/6776](https://github.com/ChilliCream/graphql-platform/pull/6776)) - Fixed CostAttribute multiplier path signature. ([https://github.com/ChilliCream/graphql-platform/pull/6777](https://github.com/ChilliCream/graphql-platform/pull/6777)) - Fixed issue that caused the tag directive to be ignored. ([https://github.com/ChilliCream/graphql-platform/pull/6746](https://github.com/ChilliCream/graphql-platform/pull/6746)) - Fixed broken postgres tests ([https://github.com/ChilliCream/graphql-platform/pull/6779](https://github.com/ChilliCream/graphql-platform/pull/6779)) - Fixed Keyed Services ([https://github.com/ChilliCream/graphql-platform/pull/6782](https://github.com/ChilliCream/graphql-platform/pull/6782)) - Fixed Tag Composition Tooling ([https://github.com/ChilliCream/graphql-platform/pull/6783](https://github.com/ChilliCream/graphql-platform/pull/6783)) - EF Core middleware must check for IAsyncEnumerable. ([https://github.com/ChilliCream/graphql-platform/pull/6784](https://github.com/ChilliCream/graphql-platform/pull/6784)) **Full Changelog**: ChilliCream/graphql-platform@13.7.0...13.8.0-preview.8 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 10pm every weekday,every weekend,before 5am every weekday" in timezone Europe/Berlin, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/orso-co/Orso.Arpa.Api). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMDMuMSIsInVwZGF0ZWRJblZlciI6IjM3LjEwMy4xIiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Postgres has a max limit of 8000 bytes for the payload in pg_notify. Exceeding this limit will result in infinite retries as messages are requeued internally back on to the channel. Due to batching a single poison (over size) message can break delivery of all messages.
This mitigates the infinite loop by blocking the messages from being queued into the subscription system.
Summary of the changes (Less than 80 chars)
Closes #6658