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

[FW][FIX] bus: split large NOTIFY payloads #159569

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Mar 27, 2024

Description

On a database with heavy activity, it is possible for the payload size we pass for the NOTIFY query on the bus to be larger than what PostgreSQL allows by default, 8000 B. This limit is defined as src1 and is used to compare the size of the payload before processing src2

Fix

  • Binary split the payload into multiple chunks if it's exceeding the above-mentioned limit. The unit of splitting is a channel, so if the content of one channel itself is larger than said limit, it is not handled, but shouldn't occur under normal circumstances.
  • Introduce also a new ENV variable ODOO_NOTIFY_PAYLOAD_MAX_LENGTH to allow tweaking of the limit if they are running a handrolled custom PostgreSQL cluster.

Reference

opw-3650618


I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

Forward-Port-Of: #159469
Forward-Port-Of: #154463

Description:
On a database with heavy activity, it is possible for the payload
size we pass for the `NOTIFY` query on the bus to be larger than what
PostgreSQL allows by default, 8000 B. This limit is defined as:
https://github.com/postgres/postgres/blob/6686e9676c8faff4ee04c1574e117ae38f117efa/src/backend/commands/async.c#L158-L166
and it is used to compare the size of the payload before processing:
https://github.com/postgres/postgres/blob/6686e9676c8faff4ee04c1574e117ae38f117efa/src/backend/commands/async.c#L654-L657

Fix:
- Binary split the payload into multiple chunks if it's exceeding
  the above-mentioned limit. The unit of splitting is a channel, so
  if the content of one channel itself is larger than said limit, it
  is not handled, but shouldn't occur under normal circumstances.
- Introduce also a new ENV variable `ODOO_NOTIFY_PAYLOAD_MAX_LENGTH`
  to allow tweaking of the limit if they are running a handrolled
  custom PostgreSQL cluster.

Reference:
opw-3650618

X-original-commit: 6101700
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
Signed-off-by: Piryns Victor (pivi) <pivi@odoo.com>
@robodoo
Copy link
Contributor

robodoo commented Mar 27, 2024

@fw-bot
Copy link
Contributor Author

fw-bot commented Mar 27, 2024

This PR targets saas-17.1 and is part of the forward-port chain. Further PRs will be created up to master.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@robodoo robodoo added the forwardport This PR was created by @fw-bot label Mar 27, 2024
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Mar 27, 2024
robodoo pushed a commit that referenced this pull request Mar 28, 2024
Description:
On a database with heavy activity, it is possible for the payload
size we pass for the `NOTIFY` query on the bus to be larger than what
PostgreSQL allows by default, 8000 B. This limit is defined as:
https://github.com/postgres/postgres/blob/6686e9676c8faff4ee04c1574e117ae38f117efa/src/backend/commands/async.c#L158-L166
and it is used to compare the size of the payload before processing:
https://github.com/postgres/postgres/blob/6686e9676c8faff4ee04c1574e117ae38f117efa/src/backend/commands/async.c#L654-L657

Fix:
- Binary split the payload into multiple chunks if it's exceeding
  the above-mentioned limit. The unit of splitting is a channel, so
  if the content of one channel itself is larger than said limit, it
  is not handled, but shouldn't occur under normal circumstances.
- Introduce also a new ENV variable `ODOO_NOTIFY_PAYLOAD_MAX_LENGTH`
  to allow tweaking of the limit if they are running a handrolled
  custom PostgreSQL cluster.

Reference:
opw-3650618

closes #159569

X-original-commit: 6101700
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
Signed-off-by: Piryns Victor (pivi) <pivi@odoo.com>
@robodoo robodoo closed this Mar 28, 2024
@fw-bot fw-bot deleted the saas-17.1-15.0-perf-3650618-pivi-sDcn-fw branch April 11, 2024 14:47
alialfie pushed a commit to odoo-dev/odoo that referenced this pull request Apr 22, 2024
Description:
On a database with heavy activity, it is possible for the payload
size we pass for the `NOTIFY` query on the bus to be larger than what
PostgreSQL allows by default, 8000 B. This limit is defined as:
https://github.com/postgres/postgres/blob/6686e9676c8faff4ee04c1574e117ae38f117efa/src/backend/commands/async.c#L158-L166
and it is used to compare the size of the payload before processing:
https://github.com/postgres/postgres/blob/6686e9676c8faff4ee04c1574e117ae38f117efa/src/backend/commands/async.c#L654-L657

Fix:
- Binary split the payload into multiple chunks if it's exceeding
  the above-mentioned limit. The unit of splitting is a channel, so
  if the content of one channel itself is larger than said limit, it
  is not handled, but shouldn't occur under normal circumstances.
- Introduce also a new ENV variable `ODOO_NOTIFY_PAYLOAD_MAX_LENGTH`
  to allow tweaking of the limit if they are running a handrolled
  custom PostgreSQL cluster.

Reference:
opw-3650618

closes odoo#159569

X-original-commit: 6101700
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
Signed-off-by: Piryns Victor (pivi) <pivi@odoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
forwardport This PR was created by @fw-bot OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants