PGListener lifecycle: unbounded rebuilds with no backoff, leak window before Thread.start(), stop() allocates, flag-based idempotency guard
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 970
- Forks
- 486
- Avg merge
- 3d 33m
- Merged PRs (30d)
- 170
Description
Problem Statement
The PGListener lifecycle inside JDBCPubSubImpl has four independent defects that together turned a transient connection failure into the unbounded rebuild storm analysed in spike #36544 (~3,687 listener instantiations in one 600s window).
1. Rebuilds are unbounded — no backoff, no rate limit, no cap. listener() constructs a fresh PGListener whenever the current one is not listening, and six call sites reach it, including publish() (:291) — so ordinary cache-invalidation traffic drives reconstruction. PostgresPubSubImpl already has the right shape: linear backoff min(delay + 1000, 10000), reset on success (:189, :99).
2. A real leak window between the constructor and Thread.start(). The connection is opened in the constructor (:128) but the thread only starts at :69. Since run()'s finally { stopListening(); } (:177-184) is what closes the connection, anything throwing in that window leaks it permanently — e.g. unwrap(PGConnection.class) failing at :122, or Thread.start() failing under a thread storm.
Note for anyone reading the original hypothesis in #36544: the ordinary death paths do not leak.
runInternal()'sreturnon!connectionAlive()and theKILL_ON_FAILURESthrow are both covered by thatfinally.subscribeTopic()failure is also safe — it closes before re-throwing (:155). This narrow pre-start()window is the only genuine leak.
3. stop() can construct a listener in order to destroy one. stop() is listener().stopListening() (:102). If the listener is already dead, listener() builds a complete replacement — borrow + LISTEN every topic + Thread.start() — purely to stop it again. Shutdown paths must not allocate.
4. The idempotency guard tests a flag, not the resource. isListening() reads runstate (:172-174), which lags reality by up to SLEEP_BETWEEN_RUNS (500ms) and diverges entirely when the connection dies underneath the thread. PostgresPubSubImpl checks the real resource: connection != null && !connection.isClosed() (:146).
Impact: any clustered install on the default provider. Amplifies #36544's pool exhaustion; independently causes connection churn and wasted LISTEN traffic even on a healthy cluster.
Steps to Reproduce
Harness and reproducer 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 && ./churn.sh
Observe that pg_stat_statements.calls for LISTEN cluster_actions climbs ~1:1 with kill cycles, with no backoff between attempts:
SELECT calls, query FROM pg_stat_statements WHERE query ILIKE 'LISTEN %';
For defects 3 and 4 specifically, the integration reproducer isolates them (remove @Ignore to run):
./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false \
-Dit.test=JDBCPubSubImplConnectionChurnReproTest
repro_stopOnDeadListenerMustNotBuildAReplacement asserts that a second stop() does not replace the listener instance — it currently does.
Acceptance Criteria
- Listener reconstruction applies incremental backoff with a ceiling, reset on success — pattern available at
PostgresPubSubImpl.java:189. - A monotonic rebuild counter exists and is observable, so runaway reconstruction is visible rather than inferred from
pg_stat_statements. - The connection opened in the
PGListenerconstructor is closed on every failure path before the thread starts, includingunwrap()failure (:122) andThread.start()failure (:69). Acquisition and release must live in the same scope. -
stop()on an already-stopped or never-started provider is a no-op: it does not construct a listener, borrow a connection, issueLISTEN, or start a thread. -
unsubscribe()(:284-285) is reviewed for the same stop-then-immediately-rebuild pattern. - The reuse/idempotency guard is based on the connection's actual state rather than the
runstateflag, so a listener whose connection died underneath it is not reported as listening. -
repro_listenerChurnMustNotAccumulateConnectionsOrThreadsandrepro_stopOnDeadListenerMustNotBuildAReplacementboth pass with@Ignoreremoved, and the class is registered in the appropriate integration suite. - Verified with the harness: under sustained connection kills, the
LISTENcounter grows sub-linearly (backoff observable) and noPGListener Pub/Sub Threadthreads accumulate.
dotCMS Version
Incident on 26.07.06-01 (commit 5efb47a). JDBCPubSubImpl default since #26706 / #27117 (Dec 2023) — released in 24.01.26, backported to LTS 22.03.14, 23.01.11, 23.10.24 v3. Code re-verified against main.
Severity
High - Major functionality broken
Links
- Spike with full RCA: #36544 — see
docs/core/incidents/36544-pubsub-connection-churn.mdon branchissue-36544-pubsub-connection-churn-spike - Parent epic: #34837
- Freshdesk ticket #38172
- Reference implementation for backoff and idempotency:
PostgresPubSubImpl.java:146, 189
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with JDBCPubSubImpl and PGListener, then compare the backoff and resource-state handling in PostgresPubSubImpl. Run JDBCPubSubImplConnectionChurnReproTest and inspect the listed churn harness and integration tests. Done means both reproducers pass, failure paths close connections, stop and unsubscribe avoid replacement construction, and sustained kills show bounded churn without accumulating threads.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, postgresql
- Domain
- backend, databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100