Skip to content

Comments

feat: support pgbouncer for multitenant database#865

Open
fenos wants to merge 2 commits intomasterfrom
feat/multitenant-db-pool-support
Open

feat: support pgbouncer for multitenant database#865
fenos wants to merge 2 commits intomasterfrom
feat/multitenant-db-pool-support

Conversation

@fenos
Copy link
Contributor

@fenos fenos commented Feb 19, 2026

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?

  • Support pooler on multitenant database
  • Pre-configured pgbouncer using AWS instance size calculations to determine the right configuration
  • Configuring Queue to use the connection pooler
  • Added optional configuration to AWS S3 Client to turn off Checksums
  • Updated CI to publish and build the new image storage-otel

@fenos fenos requested a review from a team as a code owner February 19, 2026 14:27
@coderabbitai
Copy link

coderabbitai bot commented Feb 19, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Adds a production-ready PgBouncer container with automatic connection-pool tuning, TLS handling, entrypoint automation, user auth generation, and a health check.
    • Adds detailed PgBouncer documentation and quick-start examples.
  • New Configuration

    • Adds multitenant DB pool settings with configurable pool sizing and option to disable automatic migrations.
    • Adds option to disable S3 checksum validation for storage operations.
  • Chores

    • CI now builds and publishes the PgBouncer image on releases.

Walkthrough

Adds a production-ready pgBouncer container (Dockerfile, entrypoint script, ini template, README) that auto-tunes pool settings from AWS RDS instance type, supports TLS and healthchecks, and generates runtime config and credentials. Adds a GitHub Actions job to build and 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 using DATABASE_URL (may include TLS)
  PgBouncer->>Postgres: Open or reuse pooled backend connection (auth via userlist, server TLS if configured)
  Postgres-->>PgBouncer: Connection accepted / query result
  PgBouncer-->>Client: Return query result
  Note over PgBouncer,Postgres: Healthcheck: nc to Postgres:port -> validate connectivity/version
Loading

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@ferhatelmas
Copy link
Member

Updated CI to publish and build the new image storage-otel

I guess meant pgbouncer but checking if you didn't forget anything you intended

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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.

@fenos fenos force-pushed the feat/multitenant-db-pool-support branch from b2554a2 to b19cc9b Compare February 20, 2026 11:56
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would drop printf since it doesn't validate protocol anyway

@ferhatelmas
Copy link
Member

There is a code path where pgbouncer has an issue:

  • when async migrations is using full fleet mode, multitenant_knex is used where it's trying to get pg_try_advisory_lock but it's not guaranteed to hit the same backend or unlock can silently fail and leak the lock
  • full fleet mode isn't used according to config I see but it's possible so would be good to address (even if not right now)

query_timeout = 0
query_wait_timeout = 120
client_idle_timeout = 0
ignore_startup_parameters = extra_float_digits,statement_timeout,lock_timeout,idle_in_transaction_session_timeout,options
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since statement_timeout ignored, don't we need to pass query_timeout to enforce PG_QUEUE_READ_WRITE_TIMEOUT config?

@fenos fenos force-pushed the feat/multitenant-db-pool-support branch from 6578169 to aa831cb Compare February 23, 2026 12:31
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
.github/workflows/release.yml (1)

130-163: ⚠️ Potential issue | 🟠 Major

publish_pgbouncer uses action majors that actionlint says won’t run.

These versions are flagged as too old for current runners, so the release workflow will fail until they’re bumped.

Proposed fix
-      - id: meta
-        uses: docker/metadata-action@v4
+      - id: meta
+        uses: docker/metadata-action@v5
@@
-      - uses: docker/setup-buildx-action@v2
+      - uses: docker/setup-buildx-action@v3
@@
-      - name: configure aws credentials - staging
-        uses: aws-actions/configure-aws-credentials@v1
+      - name: configure aws credentials - staging
+        uses: aws-actions/configure-aws-credentials@v5
@@
-      - name: Login to ECR account - staging
-        uses: docker/login-action@v2
+      - name: Login to ECR account - staging
+        uses: docker/login-action@v3
@@
-      - name: configure aws credentials - prod
-        uses: aws-actions/configure-aws-credentials@v1
+      - name: configure aws credentials - prod
+        uses: aws-actions/configure-aws-credentials@v5
@@
-      - name: Login to ECR account - prod
-        uses: docker/login-action@v2
+      - name: Login to ECR account - prod
+        uses: docker/login-action@v3
@@
-      - uses: docker/build-push-action@v3
+      - uses: docker/build-push-action@v6
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/release.yml around lines 130 - 163, Update the GitHub
Actions usages to supported major versions so actionlint stops flagging them:
replace docker/metadata-action@v4 with docker/metadata-action@v5,
docker/setup-buildx-action@v2 with docker/setup-buildx-action@v3,
aws-actions/configure-aws-credentials@v1 with
aws-actions/configure-aws-credentials@v2, and docker/build-push-action@v3 with
docker/build-push-action@v4 (leave docker/login-action@v2 if already current);
update each use: lines referencing docker/metadata-action,
docker/setup-buildx-action, aws-actions/configure-aws-credentials, and
docker/build-push-action to the new majors and run the workflow to verify no
breaking input changes are required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/internal/queue/queue.ts`:
- Around line 33-46: The current multitenant branch throws if
multitenantDatabaseUrl is absent even when multitenantDatabasePoolUrl is
provided, causing createPgBoss to crash for pooler-only configs; change the
logic in the isMultitenant block to only throw when neither
multitenantDatabasePoolUrl nor multitenantDatabaseUrl is set, set url =
multitenantDatabasePoolUrl if present (fall back to multitenantDatabaseUrl
otherwise), and set migrate = false when using multitenantDatabasePoolUrl so
createPgBoss is invoked with the pool URL and correct migrate flag; update any
use of the url and migrate variables (e.g., the createPgBoss invocation)
accordingly.

---

Duplicate comments:
In @.github/workflows/release.yml:
- Around line 130-163: Update the GitHub Actions usages to supported major
versions so actionlint stops flagging them: replace docker/metadata-action@v4
with docker/metadata-action@v5, docker/setup-buildx-action@v2 with
docker/setup-buildx-action@v3, aws-actions/configure-aws-credentials@v1 with
aws-actions/configure-aws-credentials@v2, and docker/build-push-action@v3 with
docker/build-push-action@v4 (leave docker/login-action@v2 if already current);
update each use: lines referencing docker/metadata-action,
docker/setup-buildx-action, aws-actions/configure-aws-credentials, and
docker/build-push-action to the new majors and run the workflow to verify no
breaking input changes are required.

ℹ️ Review info

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Cache: Disabled due to Reviews > Disable Cache setting

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6578169 and aa831cb.

📒 Files selected for processing (9)
  • .docker/pgbouncer/Dockerfile
  • .docker/pgbouncer/README.md
  • .docker/pgbouncer/entrypoint.sh
  • .docker/pgbouncer/pgbouncer.ini.template
  • .github/workflows/release.yml
  • src/config.ts
  • src/internal/database/multitenant-db.ts
  • src/internal/queue/queue.ts
  • src/storage/backend/s3/adapter.ts
✅ Files skipped from review due to trivial changes (1)
  • .docker/pgbouncer/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/internal/database/multitenant-db.ts
  • src/storage/backend/s3/adapter.ts

Comment on lines 33 to +46
let url = pgQueueConnectionURL ?? databaseURL
let migrate = true

if (isMultitenant && !pgQueueConnectionURL) {
if (!multitenantDatabaseUrl) {
throw new Error(
'running storage in multi-tenant but DB_MULTITENANT_DATABASE_URL is not set'
)
}
url = multitenantDatabaseUrl
url = multitenantDatabasePoolUrl || multitenantDatabaseUrl

if (multitenantDatabasePoolUrl) {
migrate = false
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Pooler-only multitenant config still fails in createPgBoss.

createPgBoss throws when multitenantDatabaseUrl is missing even if multitenantDatabasePoolUrl is set, so pooler-only configs crash on startup. start() already allows the pool URL—this should be consistent.

Proposed fix
-    if (isMultitenant && !pgQueueConnectionURL) {
-      if (!multitenantDatabaseUrl) {
+    if (isMultitenant && !pgQueueConnectionURL) {
+      if (!multitenantDatabaseUrl && !multitenantDatabasePoolUrl) {
         throw new Error(
-          'running storage in multi-tenant but DB_MULTITENANT_DATABASE_URL is not set'
+          'running storage in multi-tenant but DB_MULTITENANT_DATABASE_URL or DB_MULTITENANT_DATABASE_POOL_URL is not set'
         )
       }
       url = multitenantDatabasePoolUrl || multitenantDatabaseUrl
       if (multitenantDatabasePoolUrl) {
         migrate = false
       }
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/internal/queue/queue.ts` around lines 33 - 46, The current multitenant
branch throws if multitenantDatabaseUrl is absent even when
multitenantDatabasePoolUrl is provided, causing createPgBoss to crash for
pooler-only configs; change the logic in the isMultitenant block to only throw
when neither multitenantDatabasePoolUrl nor multitenantDatabaseUrl is set, set
url = multitenantDatabasePoolUrl if present (fall back to multitenantDatabaseUrl
otherwise), and set migrate = false when using multitenantDatabasePoolUrl so
createPgBoss is invoked with the pool URL and correct migrate flag; update any
use of the url and migrate variables (e.g., the createPgBoss invocation)
accordingly.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants