feat: support pgbouncer for multitenant database#865
Conversation
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a production-ready pgBouncer container (Dockerfile, entrypoint script, ini template, README) that auto-tunes pool settings from AWS RDS instance type and supports TLS and healthchecks. Adds a GitHub Actions job to build/publish the pgBouncer image. Extends application config with storageS3DisableChecksum, multitenantDatabasePoolUrl, and multitenantMaxConnections; updates multitenant DB pooling and queue startup to use an optional pool URL and adjustable pool sizing. S3 client checksum behavior can be toggled via config. Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant PgBouncer as PgBouncer (container)
participant Postgres as Postgres (RDS)
Client->>PgBouncer: Connect (uses DATABASE_URL, TLS settings)
PgBouncer->>Postgres: Open/Reuse pooled backend connection (auth via userlist, server_tls_ca_file if present)
Postgres-->>PgBouncer: Connection accepted / query result
PgBouncer-->>Client: Return query result
Note over PgBouncer,Postgres: Healthcheck: nc to Postgres:port -> validate connectivity/version
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.docker/pgbouncer/entrypoint.sh:
- Around line 204-249: The TLS_CONFIG variable is being built with embedded
newlines and then injected via sed -e "s|{{TLS_CONFIG}}|${TLS_CONFIG}|g", which
breaks when TLS_CONFIG is multiline; instead write TLS_CONFIG to a temporary
file (e.g., /tmp/pgbouncer_tls.conf) when DATABASE_SSL_ROOT_CERT is set (the
same logic around TLS_CONFIG and map_sslmode), then generate
/etc/pgbouncer/pgbouncer.ini by using sed to replace the {{TLS_CONFIG}}
placeholder by reading that file (use sed '/{{TLS_CONFIG}}/ { r
/tmp/pgbouncer_tls.conf' -e 'd' }' or equivalent) so the multiline content is
inserted safely from the temp file; ensure the rest of the sed substitutions for
single-line fields remain the same and remove the temp file after creation.
- Around line 14-24: The parsing logic using variables like userinfo, rest,
DB_USER and DB_PASS must validate that userinfo contains a ":" and both username
and password are non-empty (reject formats like "admin@host" with a clear error
and exit), and must URL-decode percent-encoded credentials before assigning
DB_USER and DB_PASS (or before writing to userlist.txt) so e.g.
"admin%40company" becomes "admin@company"; implement a check after extracting
userinfo to verify the presence of ":" and non-empty parts, perform
percent-decoding on the credential string (then split into DB_USER and DB_PASS),
and on failure print an error and exit to prevent using an incorrect DB_PASS.
In @.github/workflows/release.yml:
- Around line 119-171: The publish_pgbouncer job uses outdated action majors
(docker/metadata-action@v4, docker/setup-buildx-action@v2,
docker/login-action@v2, docker/build-push-action@v3,
aws-actions/configure-aws-credentials@v1); update those action references inside
the publish_pgbouncer job (and all other workflow jobs) to the supported majors:
docker/metadata-action@v5, docker/setup-buildx-action@v3,
docker/login-action@v3, docker/build-push-action@v6, and
aws-actions/configure-aws-credentials@v5, keeping the same inputs and step ids
(e.g., step id meta, the two configure aws credentials steps, Login to ECR
account steps, and the build-push step) to preserve behavior and outputs.
I guess meant pgbouncer but checking if you didn't forget anything you intended |
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/release.yml:
- Around line 119-171: The publish_pgbouncer job uses outdated action versions;
update the uses entries within the publish_pgbouncer steps (e.g., the
actions/checkout, docker/metadata-action (step id meta),
docker/setup-buildx-action, aws-actions/configure-aws-credentials,
docker/login-action, and docker/build-push-action steps) to match the required
versions used elsewhere: actions/checkout@v6, docker/metadata-action@v5,
docker/setup-buildx-action@v3, aws-actions/configure-aws-credentials@v5,
docker/login-action@v3, and docker/build-push-action@v6 so static analysis and
GitHub Actions accept the job.
…enant-db-pool-support
b2554a2 to
b19cc9b
Compare
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/release.yml:
- Around line 130-163: Update the deprecated action versions used in the
publish_pgbouncer job: replace docker/metadata-action@v4 with
docker/metadata-action@v5, docker/setup-buildx-action@v2 with
docker/setup-buildx-action@v3, docker/login-action@v2 with
docker/login-action@v3, docker/build-push-action@v3 with
docker/build-push-action@v6, and aws-actions/configure-aws-credentials@v1 with
aws-actions/configure-aws-credentials@v5; edit the steps that reference those
exact identifiers (docker/metadata-action, docker/setup-buildx-action,
docker/login-action, docker/build-push-action,
aws-actions/configure-aws-credentials) so the workflow uses the supported majors
to avoid actionlint failures.
| USER postgres | ||
|
|
||
| HEALTHCHECK --interval=10s --timeout=3s --start-period=5s --retries=3 \ | ||
| CMD printf "SHOW VERSION;\n" | nc 127.0.0.1 6432 >/dev/null 2>&1 || exit 1 |
There was a problem hiding this comment.
I would drop printf since it doesn't validate protocol anyway
What kind of change does this PR introduce?
Feat
What is the current behavior?
No support for pooler on multitenant database
What is the new behavior?