Skip to content
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

Postgres Subscription - Max Message Size #6658

Closed
1 task done
williamdenton opened this issue Nov 1, 2023 · 1 comment · Fixed by #6678
Closed
1 task done

Postgres Subscription - Max Message Size #6658

williamdenton opened this issue Nov 1, 2023 · 1 comment · Fixed by #6678
Labels
Area: Subscriptions Issue is related to Subscriptions or a Subscription Provider 🌶️ hot chocolate

Comments

@williamdenton
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Product

Hot Chocolate

Describe the bug

Subscriptions.Postgres uses postgres pg_notify as the messaging backplane. This DB Function has a hard limit of 8000 bytes for the message payload. https://www.postgresql.org/docs/14/sql-notify.html

Currently this limit is not enforced client side and if it is exceed it poisons the subscription system so that only recovery method is a process restart. Obviously this will loose all inflight messages not yet written to the back plane.

When messages fail to be written to the DB they are retried by enqueuing them into the channel, then immediately consumed again. Messages are written in batches of 256 (if using the default configuration) so it is possible that once there are more than this many messages in the channel/queue the poison message will not be included in a batch and some messages will be published.

The message size includes the topic and payload and some hotchocolate controlled delimiters so it can be hard to tell if the message is going to be oversize before it is published. Users should be aware of this limit and stay under it. However, once published via the ITopicEventSender hot chocolate has no choice but to discard the message in order to preserve the running of the system.

I suggest a length check be added below inside the forloop as the message is formatted to ensure it is less than 8000 bytes, and if it is discard the message with an appropriate diagnostic event. With some refactoring it could be possible validate the message as it is passed into the ITopicEventSender but due to encoding that only occurs as the message is being sent this may not be practical.

The 8000byte limit is not configurable in Postgres and is unlikely to change so this validation can be hard coded into the PostgresChannelWriter

try
{
// we throw instead of checking the cancellation token because we want to requeue the
// firstMessage that was already read from the channel
ct.ThrowIfCancellationRequested();
await using var batch = connection.CreateBatch();
foreach (var message in messages)
{
var command = batch.CreateBatchCommand();
command.CommandText = "SELECT pg_notify(@channel, @message);";
command.Parameters.Add(new NpgsqlParameter("channel", _channelName));
command.Parameters.Add(new NpgsqlParameter("message", message.Format()));
batch.BatchCommands.Add(command);
}
await batch.PrepareAsync(ct);
await batch.ExecuteNonQueryAsync(ct);
}
catch (Exception ex)
{
var msg = string.Format(ChannelWriter_FailedToSend, messages.Count, ex.Message);
_diagnosticEvents.ProviderInfo(msg);
// if we cannot send the message we put it back into the channel
foreach (var message in messages)
{
await _channel.Writer.WriteAsync(message, ct);
}
}

Symptoms of this issue include elevated network and cpu usage on the Database sever. In my case we were observing approx 20k/sec to the database resulting in significant logging in the database error log.
image

Steps to reproduce

  1. Publish a message to into IEventTopicSender of more than 8000bytes
  2. System is now broken, any subsequent publish will not make it to the database

Relevant log output

(Postgres logs not HotChocolate Logs)

2023-11-01 19:28:10 UTC:10.11.0.228(48950):user@db:[22886]:STATEMENT: SELECT pg_notify($3, $4)
2023-11-01 19:28:10 UTC:10.11.0.228(48950):user@db:[22886]:ERROR: payload string too long

Additional Context?

No response

Version

13.7

@tobias-tengler tobias-tengler added 🌶️ hot chocolate Area: Subscriptions Issue is related to Subscriptions or a Subscription Provider labels Nov 1, 2023
@michaelstaib
Copy link
Member

Do you want to do a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Subscriptions Issue is related to Subscriptions or a Subscription Provider 🌶️ hot chocolate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants