dotCMS / dotCMS/core

Pub/sub listener holds a permanent connection from the shared JDBC pool, exhausting jdbc/dotCMSPool

Open
#36,801 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

dotCMS : Clustering OKR : Customer Support Team : Maintenance Type : Defect
Dominant language
Java
Stars
970
Forks
486
Avg merge
3d 33m
Merged PRs (30d)
170

Description

Problem Statement

JDBCPubSubImpl — the default pub/sub provider since 24.01.26 — borrows its permanently-held Postgres LISTEN connection from the shared application pool (jdbc/dotCMSPool):

// JDBCPubSubImpl.java:121
private final Lazy<Connection> connection =
    Lazy.of(() -> Try.of(() -> DbConnectionFactory.getDataSource().getConnection())...);

That single design decision is the root cause behind spike #36544. Because Postgres LISTEN/NOTIFY delivers notifications only to the specific backend session that subscribed, and because vanilla pgjdbc has no async notification callback, the connection must be held open and kept perpetually busy (a SELECT 1 poll every 500ms, :197-201). It can therefore never be returned to the pool, even momentarily.

The consequence in production (customer firstmac, pod dotcms-firstmac-prod-1-1, 26.07.06-01, 2026-07-08): repeated listener rebuilds each borrowed a fresh pooled connection — ~3,687 borrows in a single 600s window — until the pool was exhausted and every DB consumer starved simultaneously (DbConnectionFactory, RulesEngine, PostgresJobQueue, VelocityServlet). Startup never completed and pods were SIGKILLed in a crash-loop.

Cluster messaging must not compete with page rendering for connections. Note the sibling implementation PostgresPubSubImpl — the default until December 2023 — already gets this right, acquiring a dedicated connection outside the pool (:226) with a POSTGRES_PUBSUB_JDBC_URL override (:217).

Impact: any clustered install on the default provider. PubSubCacheTransport is the only functioning CacheTransport in the tree, so all cluster cache coherence flows through this one connection.

Steps to Reproduce

Full harness on branch issue-36544-pubsub-connection-churn-spike:

git fetch origin issue-36544-pubsub-connection-churn-spike
git checkout issue-36544-pubsub-connection-churn-spike
cd docker/docker-compose-examples/pubsub-connection-churn
docker compose up -d          # wait for "Startup completed" on node 1
./churn.sh                    # 30 kill cycles

churn.sh terminates the Postgres backend holding the LISTEN connection, then requests a page to force a cache invalidation → publish()listener() → rebuild. On current code the instantiation counter climbs ~1:1 with kill cycles.

The metric to watch — pg_stat_statements.calls for LISTEN cluster_actions is a direct PGListener-instantiation counter, because listener() re-issues LISTEN for every subscribed topic on each rebuild (:63-66):

SELECT calls, query FROM pg_stat_statements WHERE query ILIKE 'LISTEN %';
SELECT count(*) FROM pg_stat_activity WHERE datname='dotcms';

Acceptance Criteria

  • The pub/sub listener connection is no longer drawn from jdbc/dotCMSPool; listener churn cannot reduce connections available to request-serving code.
  • Implemented as a dedicated HikariDataSource sized 1–2, not raw DriverManager. Rationale: it is equally isolated but preserves HikariCP metrics and Glowroot connection-acquisition instrumentation, and allows leakDetectionThreshold and maxLifetime to be tuned for this pool alone.
  • leakDetectionThreshold is disabled for the pub/sub pool — the connection is held forever by design, so today it trips ProxyLeakTask on every healthy boot and produced a misleading "leak" signal in this incident.
  • maxLifetime for the pub/sub pool is effectively infinite, so nothing attempts to retire the listener connection.
  • A connection URL/credential override is supported, mirroring POSTGRES_PUBSUB_JDBC_URL (PostgresPubSubImpl.java:217).
  • DatabaseMetrics reports the new pool. It currently hardcodes a single pool — resolving DbConnectionFactory.getDataSource() and defaulting the name to "HikariPool-1" (DatabaseMetrics.java:61-75) — so a second pool needs explicit registration or metrics silently omit it.
  • New metrics are exposed: dotcms.pubsub.listener.rebuilds (counter — this is the 3,687, promoted to a first-class signal), dotcms.pubsub.listener.listening (0/1 gauge), and the existing DotPubSubTopic sent/received counters which nothing currently surfaces.
  • A PubSubHealthCheck is registered in CoreHealthCheckProvider and reports degraded when the listener is not listening. There is currently no pub/sub or cache-transport health check; com.dotcms.health already supplies HealthCheckBase, tolerance config and failure windows.
  • Verified with the harness: LISTEN instantiation count no longer tracks kill cycles 1:1, and pg_stat_activity connection count stays flat under churn.
  • JDBCPubSubImplConnectionChurnReproTest passes with @Ignore removed, and is registered in the appropriate integration suite.

Follow-on measurement (not in scope): listener() holds the static PGListener.class monitor across the connection borrow (:59, :121), which in production blocked for the full 30s DB_CONNECTION_TIMEOUT and convoyed every publisher thread. This should be re-measured once the connection is off the shared pool, since an off-pool borrow should no longer block that long. File separately only if it still reproduces.

dotCMS Version

Incident on 26.07.06-01 (commit 5efb47a). JDBCPubSubImpl has been the default provider since #26706 / #27117 (Dec 2023), released in 24.01.26 and backported to LTS 22.03.14, 23.01.11 and 23.10.24 v3. Code re-verified against main.

Severity

High - Major functionality broken

Why this is a bulkhead — design rationale

This is the resource-pool partitioning form of the bulkhead pattern: partition a shared resource so exhaustion in one compartment cannot sink the ship. Recording the rationale here so the design intent survives review.

It is one pattern, cleanly. The related fixes are deliberately separate issues: retry-with-backoff and Steady State (bounded resource acquisition) are #36802; this issue is only the partition.

1. It restores a property dotCMS used to have. PostgresPubSubImpl — the default provider until December 2023 — acquires its listener connection via DriverManager outside the pool (:226), with a POSTGRES_PUBSUB_JDBC_URL override (:217). The bulkhead existed and was removed as a side effect of the default flip in #26706 / #27117. This is a regression fix, not new architecture.

2. The codebase already accepts this reasoning one layer up. DatabaseHealthCheck.isLivenessCheck() returns false (:41-43) precisely so a database problem removes the pod from the load balancer instead of restarting it — HealthCheck.java:54 states "NEVER check external dependencies in liveness probes as this can cause…". That is bulkhead thinking, already ratified, at the probe layer. This issue applies the identical principle at the pool layer.

3. A permanent holder in a transient pool is a category error, bug or no bug. A connection pool is an abstraction for short checkout → return cycles; sizing, maxLifetime, leakDetectionThreshold and idle eviction all assume it. The listener checks out once and never returns, and is busy every 500ms so it cannot be evicted. Three symptoms are visible in production today, with no defect involved:

  • ProxyLeakTask fires on every healthy boot — a permanent false alarm that actively misdirected triage of this incident.
  • dotcms.db.pool.active is permanently ≥ 1, so the metric does not mean what it says.
  • maxLifetime is inert for this connection (HikariCP never retires an in-use one), so it silently opts out of the pool's own lifecycle management.

4. Control plane vs data plane. Cluster cache invalidation is control-plane traffic; page rendering is data-plane. Sharing a single 60-connection budget means a control-plane defect takes out the data plane — which is exactly what happened, with VelocityServlet and HTMLPageAssetAPIImpl starving because of a pub/sub bug. PostgreSQL implements the same idea for the same reason via superuser_reserved_connections: reserved capacity so a privileged class of work survives saturation.

5. Anticipated objection — "just fix the churn in #36802 and this is unnecessary." Bulkhead is not an alternative to fixing the bug; it determines the blast radius of the next one. With the partition in place this incident would have been a degraded cache transport on one node — visible and survivable — rather than a startup crash-loop that also starved the job queue and page rendering. Containment and correction are different jobs; #36801 and #36802 do one each.

References
  • Michael Nygard, Release It! (2nd ed.) — Stability Patterns → Bulkheads (uses connection-pool partitioning as the worked example); Stability Antipatterns → Blocked Threads, Unbalanced Capacities
  • Azure Architecture Center — Bulkhead pattern
  • Google SRE Book — Addressing Cascading Failures (resource exhaustion as a cascade trigger; retry amplification)
  • resilience4j BulkheadSemaphoreBulkhead / FixedThreadPoolBulkhead, the pattern as a first-class JVM concept
  • HikariCP wiki — About Pool Sizing, on why pools assume short-lived checkout
  • PostgreSQL — superuser_reserved_connections (and reserved_connections, PG16+)

Links

  • Spike with full RCA: #36544 — see docs/core/incidents/36544-pubsub-connection-churn.md on branch issue-36544-pubsub-connection-churn-spike
  • Parent epic: #34837
  • Freshdesk ticket #38172
  • Provenance: #26019 (provider added as opt-in), #26706 / #27117 (made default)
  • Related: #34923 (pool sizing/observability), #34840 (validation query), #34921 (DB_MAXWAIT naming)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with JDBCPubSubImpl.java and compare its connection setup with PostgresPubSubImpl.java. Then inspect DatabaseMetrics.java, CoreHealthCheckProvider, and JDBCPubSubImplConnectionChurnReproTest, followed by the supplied Docker churn harness. Done means the listener uses an isolated pool, its metrics and health check are exposed, and the integration test and harness show stable connections under churn.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, postgresql
Domain
backend, databases, observability, testing-qa
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.