fix: compute payload size correctly for pg_notify#2873
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect payload size calculations for PostgreSQL pg_notify by removing double base64-encoding, centralizing message wrapping, and enforcing the 8KB limit on the actual on-wire payload, with test coverage around the new behavior.
Changes:
- Introduces
multiplexedListener.wrapMessageto wrap a queue payload intoPubSubMessageusingjson.RawMessage, eliminating the extra base64 layer and making size calculations reflect the true JSON sent overpg_notify. - Updates
messageQueueRepository.Notifyto usewrapMessage, enforce the 8000-byte limit on the wrapped payload, and fall back to database storage when exceeded;pubNonDurableMessagesnow delegates all size handling toNotify. - Adds tests around wrapped message sizing and empty payload handling in
multiplexer_test.go, and changesPubSubMessage.Payloadtojson.RawMessageto align with the new encoding approach.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pkg/repository/multiplexer_test.go |
Adds tests that exercise wrapMessage for various payload sizes and empty payload behavior, plus a helper to synthesize JSON payloads of a target size. |
pkg/repository/multiplexer.go |
Extracts wrapMessage to build the PubSubMessage JSON using json.RawMessage, and simplifies notify to send pre-wrapped payloads. |
pkg/repository/mq.go |
Changes PubSubMessage.Payload to json.RawMessage and updates Notify to use wrapMessage, enforce the 8KB pg_notify limit, and fall back to AddMessage when exceeded. |
internal/msgqueue/postgres/exchange.go |
Simplifies non-durable publish path to always call repo.Notify, relying on the repository’s new size-aware logic instead of performing its own 8KB check. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| QueueName string `json:"queue_name"` | ||
| Payload json.RawMessage `json:"payload"` |
There was a problem hiding this comment.
Changing PubSubMessage.Payload from a []byte to json.RawMessage means existing code that constructs PubSubMessage values with a []byte payload (for example in multiplexer_test.go where Payload: []byte("test-payload") is used) will no longer compile. Those call sites need to be updated to construct a json.RawMessage (or otherwise pass a value of the correct type) to keep tests and any other callers building cleanly.
| QueueName string `json:"queue_name"` | |
| Payload json.RawMessage `json:"payload"` | |
| QueueName string `json:"queue_name"` | |
| Payload []byte `json:"payload"` |
| // createJSONPayload creates a JSON string of exactly the specified size | ||
| func createJSONPayload(size int) string { | ||
| // Create a simple JSON object with a large string field | ||
| // The JSON structure will be: {"data":"<padding>"} | ||
| // Structure overhead: { (1) + "data" (6) + : (1) + " (1) + " (1) + } (1) = 11 bytes | ||
| const jsonStructureOverhead = 11 |
There was a problem hiding this comment.
The comment on createJSONPayload says it "creates a JSON string of exactly the specified size", but the implementation only approximates the size (e.g. it hardcodes jsonStructureOverhead and clamps paddingSize to >= 0), so for many inputs the resulting JSON length will differ from size. Either adjust the implementation to guarantee the exact length or relax the comment to describe that it produces a payload of approximately the requested size to avoid misleading future readers and test authors.
Description
Users running in Postgres-only mode will occasionally see errors like the following:
We have a check for 8000 bytes, but this check was failing to account for the extra marshaling we do when sending the message through
pg_notify. We wrap the message inPubSubMessagewhich has some overhead with additional fields and queue names, and we're also base64-encoding the raw payload a second time, which adds overhead. As a result, even ~6kb payloads could occasionally see this behavior.This fix does a few things:
json.RawMessageinstead of[]byteType of change