Skip to content

revert(database): drop the PgBouncer use-vc dnsConfig - #4149

Merged
Tanguille merged 1 commit into
mainfrom
fix/revert-pgbouncer-use-vc
Jul 23, 2026
Merged

revert(database): drop the PgBouncer use-vc dnsConfig#4149
Tanguille merged 1 commit into
mainfrom
fix/revert-pgbouncer-use-vc

Conversation

@Tanguille

Copy link
Copy Markdown
Owner

Reverts e4e3645 (#4135). Split out of #4147 so the urgent policy fix can merge without a pooler rollout.

Why it never worked

#4135 forced PgBouncer DNS over TCP. PgBouncer never uses TCP for DNS here. Observed across three pooler pods on two nodes, sampled every 2s for 50s:

udp53=1  tcp53=0     # socket inode unchanged throughout

pgbouncer 1.25.1 resolves through libcares 1.34.5.

The mechanism is not established, and the earlier "c-ares not glibc, so it never takes effect" explanation is wrong: c-ares 1.34.5 does parse use-vc (ares_sysconfig_files.c: process_option()) and applies it without an optmask gate, and pgbouncer never passes ARES_OPT_FLAGS to override it. The socket observation is the evidence; I can't source the why.

Why removing it is not cosmetic

use-vc is a pod-level dnsConfig — it applies to every process in the pod, not just pgbouncer. The pooler's pid 1 is the Go /controller/manager, and Go's net dnsconfig parser honours use-vc/usevc/tcp, as does glibc NSS.

Under the CoreDNS reply drops fixed in #4147, TCP is the strictly worse transport: a denied SYN,ACK wedges the caller through the full TCP SYN retry ladder, where a dropped UDP reply just retries in 1-2s. So this removes an aggravating factor.

Blocked on — do not merge yet

This changes template.spec, so CNPG rolls all 7 pooler pods (rw ×3, ro ×2, session ×2). Two gates:

  1. fix(network): allow CoreDNS replies through the database ingress allowlist #4147 must merge first and be verified. Its check is that the poolers recover without deletion. A rollout here would reset the pinned backends and destroy that signal — you'd never know whether the policy fix worked.
  2. control-3 memory headroom. Deployments are maxSurge=1, maxUnavailable=0. pgbouncer-rw requests 128Mi; control-3 currently has ~52Mi free (99% of allocatable requested). Only pgbouncer-rw carries whenUnsatisfiable: DoNotSchedule, so once the topology hits 2/1/0 the maxSkew: 1 constraint forces the next surge pod onto control-3 specifically, where it cannot schedule — stalling the roll indefinitely. That is the same deadlock that cost 20 minutes on 07-21, and headroom is worse now than it was then.

#4144 frees 2Gi/node by cutting vm.nr_hugepages 2048→1024; landing that first clears gate 2. Alternatively batch this with some future pooler change that has to roll anyway.

pgbouncer-ro and pgbouncer-session use ScheduleAnyway and carry no deadlock risk.

Reverts e4e3645 (#4135). It was added to force PgBouncer DNS over TCP, but
PgBouncer never uses TCP for DNS here. Observed across three pooler pods on two
nodes, sampled every 2s for 50s: exactly one connected UDP socket to
10.43.0.10:53 and zero TCP sockets to :53, socket inode unchanged throughout.
pgbouncer 1.25.1 resolves through libcares 1.34.5.

The option is not inert, though: it applies to every other process in the pod.
The pooler's pid 1 is the Go /controller/manager, and Go's net dnsconfig parser
honours use-vc, as does glibc NSS. Under the CoreDNS reply drops fixed in
#4147 TCP is the worse transport, because a denied SYN,ACK wedges the caller
through the full TCP SYN retry ladder while a dropped UDP reply retries in
1-2s.

So this removes an aggravating factor, not dead config. Note the mechanism by
which c-ares ignores use-vc is not established: c-ares 1.34.5 does parse the
option and pgbouncer never passes ARES_OPT_FLAGS to override it. The socket
observation is the evidence, not a source-level explanation.
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@Tanguille, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 15 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5d0d70c8-d0d5-4273-9637-4adff687a062

📥 Commits

Reviewing files that changed from the base of the PR and between bc287c0 and 64dc9ee.

📒 Files selected for processing (3)
  • kubernetes/apps/database/cloudnative-pg/cluster/pooler-ro.yaml
  • kubernetes/apps/database/cloudnative-pg/cluster/pooler-session.yaml
  • kubernetes/apps/database/cloudnative-pg/cluster/pooler.yaml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/revert-pgbouncer-use-vc

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

@tanguille-cluster

Copy link
Copy Markdown
@@ spec.template.spec @@
# postgresql.cnpg.io/v1/Pooler/database/pgbouncer-rw
! - one map entry removed:
- dnsConfig:
-   options:
-   - name: use-vc

@@ spec.template.spec @@
# postgresql.cnpg.io/v1/Pooler/database/pgbouncer-ro
! - one map entry removed:
- dnsConfig:
-   options:
-   - name: use-vc

@@ spec.template.spec @@
# postgresql.cnpg.io/v1/Pooler/database/pgbouncer-session
! - one map entry removed:
- dnsConfig:
-   options:
-   - name: use-vc

@Tanguille
Tanguille marked this pull request as ready for review July 23, 2026 15:16
@Tanguille
Tanguille merged commit d87c9da into main Jul 23, 2026
18 of 19 checks passed
@Tanguille
Tanguille deleted the fix/revert-pgbouncer-use-vc branch July 23, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant