Skip to content

fix: copying buffer max size, grpc pg connection - #1499

Open
matheusfrancisco wants to merge 1 commit into
mainfrom
fix/copying-buffer-max
Open

fix: copying buffer max size, grpc pg connection#1499
matheusfrancisco wants to merge 1 commit into
mainfrom
fix/copying-buffer-max

Conversation

@matheusfrancisco

@matheusfrancisco matheusfrancisco commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Important

Every PR MUST have exactly one of these labels, the merge is blocked otherwise:

  • major / minor / patch publishes a new release on merge with the corresponding semver bump.
  • skip-release merges without publishing a release (use for docs, CI changes, refactors, etc.).

📝 Description

hoop connect to a Postgres connection rejects any single wire message larger than 16 MiB with fail to decode typed packet, err=max size (16777216) reached. A large single statement (e.g. a generated INSERT ... VALUES (...)) is one Postgres message, so it never gets sent — while native psql/IDEs accept it.

This adds an opt-in feature flag, experimental.pg_large_query, that raises the Postgres packet cap from 16 MiB to 32 MiB across the client proxy and the agent. The flag is off by default, so existing behavior is unchanged unless an operator enables it.

🔗 Related Issue

Fixes #1496

🚀 Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🎨 Style/UI update
  • ♻️ Code refactor
  • ⚡ Performance improvement
  • ✅ Test update
  • 🔧 Build configuration change
  • 🧹 Chore

📋 Changes Made

  • Register experimental.pg_large_query feature flag (common/featureflag/featureflag.go), components Client + Agent, default false, experimental stability.
  • common/pgtypes: add LargeBufferSize (32 MiB) alongside DefaultBufferSize (16 MiB); add DecodeWithMaxSize(reader, maxSize) and make Decode delegate to it with the default cap. A non-positive maxSize falls back to DefaultBufferSize.
  • Client PG proxy (client/proxy/pg.go): NewPGServer now takes a maxPacketSize, stored on the server and applied via DecodeWithMaxSize in copyPGBuffer.
  • Client wiring (client/cmd/connect.go, client/cmd/proxymanager.go): pass LargeBufferSize when clientconfig.IsFeatureEnabled(config, "experimental.pg_large_query") (flag is read from the gateway /serverinfo response), otherwise DefaultBufferSize. runAutoConnect now receives the client config.
  • Agent (agent/controller/postgres.go): when featureflagstate.IsEnabled("experimental.pg_large_query"), pass max_packet_size = LargeBufferSize into the libhoop Postgres proxy opts so libhoop honors the same ceiling; libhoop uses its own default when the option is absent.

🧪 Testing

Test Configuration:

  • Browser(s): N/A (CLI / agent path)
  • OS:

Tests performed:

  • Unit tests pass
  • Integration tests pass
  • Manual testing completed

Reproduction / manual verification:

  1. Enable experimental.pg_large_query (client, gateway, and agent must all run a build that registers the flag).
  2. hoop connect <postgres-connection>.
  3. Execute a single statement between 16 MiB and the new cap (e.g. a generated multi-row INSERT).
  4. With the flag off → request fails with max size (16777216) reached. With the flag on → packet is accepted by the client decoder.

✅ Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes
  • I have checked my code and corrected any misspellings

📄 Additional Notes

  • All three components (client, gateway, agent) must run a build that registers the flag and have it enabled; a mixed fleet leaves the lower cap in effect on whichever side is older.
  • Trade-off: larger packets increase peak memory and session-recording size per large query on every hop.
  • The end-to-end ceiling also depends on the proprietary libhoop Postgres proxy accepting max_packet_size; the OSS tree only carries a no-op stub, so this needs verifying against a real libhoop build.

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Migration Safety Analysis

No database migrations were changed in this PR. Safe to deploy to sandbox.

@matheusfrancisco matheusfrancisco added the patch Bumps the patch version on release (bug fixes) label Jun 1, 2026
@matheusfrancisco
matheusfrancisco force-pushed the fix/copying-buffer-max branch from 0616f9e to f3c263e Compare June 2, 2026 13:45
@sandromello

Copy link
Copy Markdown
Contributor

✅ Build Completed with Success, Version=1499.0.0-gf3c263e

@matheusfrancisco
matheusfrancisco marked this pull request as ready for review June 2, 2026 14:09
@agrrh

agrrh commented Jun 24, 2026

Copy link
Copy Markdown

Just confirming, seems that this patch would resolve our issue

@matheusfrancisco

Copy link
Copy Markdown
Contributor Author

@agrrh I am doing a test, but it will not solve because the max on grpc is 16MB I will need to stream the pkt by chunk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch Bumps the patch version on release (bug fixes)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: failed copying buffer, max size (16777216) reached

4 participants