fix(telemetry): MetricStatsCollector ThreadLocal connection poisoning — CountOfSitesWithThumbnailsMetricType defeats wrapConnection() fix

Summary

The wrapConnection() fix deployed in PR #34490 (release 26.02.27-01) does not prevent connection leaks from DBMetricType telemetry metrics. Root cause: a non-DBMetricType metric (CountOfSitesWithThumbnailsMetricType) opens a thread-local connection via DbConnectionFactory.getConnection() without cleanup, poisoning the ThreadLocal for all subsequent metrics on the same executor thread. This causes wrapConnection() to skip its closeSilently() cleanup, leaking one connection per telemetry run per pod.

Production impact (Frankfurt cluster, observed 2026-03-10): 46 orphaned idle connections surviving 16+ hours, all showing the experiments ARCHIVED query as the last-executed statement. With 31 evergreen pods running telemetry 3×/day, this produces ~93 leaked connections/day (partially masked by the DB_MAXWAIT 60s bug from #34921 reclaiming some).

Root Cause — ThreadLocal Connection Poisoning

The execution model

MetricStatsCollector.getStats() creates Executors.newSingleThreadExecutor() — a single thread reused for ALL metrics. Metrics are discovered via CDI @Inject Instance<MetricType> with no guaranteed iteration order (Weld classpath-dependent).

The poison pill: CountOfSitesWithThumbnailsMetricType

File: com.dotcms.telemetry.collectors.site.CountOfSitesWithThumbnailsMetricType

This class implements plain MetricType (not DBMetricType), so it does not use wrapConnection(). Its getValue() calls through to DotConnect which acquires a thread-local connection that is never closed:

getValue()
  → getCountOfSitesWithThumbnails()
    → getAllSitesInodes()
      → new DotConnect().setSQL(ALL_SITES_INODES).loadObjectResults()   // no-arg
        → loadResult()                                                   // no-arg
          → executeQuery()                                               // no-arg
            → DbConnectionFactory.getConnection()                        // ← STORES IN THREADLOCAL
            → executeQuery(conn)                                         // uses conn, does NOT close

getAllSitesInodes() — where the connection opens

private List<String> getAllSitesInodes() throws DotDataException {
    final DotConnect db = new DotConnect();
    final List<Map<String, Object>> results = db.setSQL(ALL_SITES_INODES).loadObjectResults();
    return results.stream().map(map -> (String) map.get("inode")).collect(Collectors.toList());
}

DotConnect.executeQuery() (no-arg) — the thread-local connection acquisition

private void executeQuery() throws SQLException {
    Connection conn = DbConnectionFactory.getConnection();  // gets or creates thread-local connection
    executeQuery(conn);                                     // runs query, does NOT close
}

DotConnect.loadObjectResults() (no-arg) → loadResult() (no-arg) → executeQuery() (no-arg) → DbConnectionFactory.getConnection(). This stores the connection in the executor thread's ThreadLocal<HashMap<String, Connection>> connectionsHolder.

DotConnect never closes this connection — that is by design. Normally, connection cleanup is handled by @CloseDBIfOpened annotations or CMSFilter at the end of HTTP requests. But on the telemetry executor thread, neither of these exists. There is no @CloseDBIfOpened on getValue(), no wrapConnection() call, and no servlet filter — so the thread-local connection is abandoned.

getCountOfSitesWithThumbnails() — the full method

private int getCountOfSitesWithThumbnails() {
    int hostsWithThumbnailsCount = 0;
    try {
        final List<String> allSitesInodes = getAllSitesInodes();  // ← opens thread-local connection here
        for (String siteInode : allSitesInodes) {
            final File hostThumbnail = Try.of(() ->
                APILocator.getContentletAPI().getBinaryFile(siteInode,
                    Host.HOST_THUMB_KEY, APILocator.systemUser())).getOrNull();
            if (hostThumbnail != null) {
                hostsWithThumbnailsCount++;
            }
        }
    } catch(Exception e) {
        Logger.debug(this, "Error counting Sites with thumbnails");  // swallows exception, no cleanup
    }
    return hostsWithThumbnailsCount;
}

Note the catch(Exception e) block logs at debug level and performs no connection cleanup, so even if getAllSitesInodes() partially fails, the thread-local connection remains open.

How this defeats wrapConnection()

DbConnectionFactory.wrapConnection() (the #34490 fix) guards cleanup with:

final boolean isNewConnection = !connectionExists();  // checks ThreadLocal
try {
    return delegate.execute();
} finally {
    if (isNewConnection && connectionExists()) {
        closeSilently();  // ONLY runs if WE opened the connection
    }
}

When CountOfSitesWithThumbnailsMetricType runs first on the executor thread:

1. Poison: CountOfSitesWithThumbnailsMetricType.getValue() opens thread-local connection via DotConnect → DbConnectionFactory.getConnection(). No cleanup. Connection sits in ThreadLocal.
2. Propagation: Next metric (e.g., CountVariantsInAllArchivedExperimentsMetricType, a DBMetricType) calls wrapConnection() → connectionExists() returns true → isNewConnection = false
3. Query reuse: The delegate executes using the existing poisoned connection (returned by DbConnectionFactory.getConnection() from ThreadLocal)
4. Skipped cleanup: Finally block sees isNewConnection == false → skips closeSilently()
5. Repeat: All remaining DBMetricType metrics on the same thread reuse and perpetuate the poisoned connection
6. Orphan: executor.shutdown() terminates the thread. The ThreadLocal connection is never returned to HikariCP. PostgreSQL sees it as an idle connection indefinitely.

Why pg_stat_activity shows the experiments ARCHIVED query

pg_stat_activity.query reflects the last query executed on a connection. The poisoned connection runs multiple metrics' queries sequentially. The ARCHIVED experiments metric is likely the last DBMetricType in the CDI iteration order, so its query is what remains visible.

Evidence

Frankfurt RDS (k8s-comm-1-green), 2026-03-10:

 state |  query (fingerprint)                              | count | min_idle    | max_idle
-------+---------------------------------------------------+-------+-------------+-----------
 idle  | SELECT COALESCE(SUM(jsonb_array_length(           |    46 | 00:29:30    | 16:31:47
       |   traffic_proportion->'variants')),0) AS Value    |       |             |
       |   FROM experiment WHERE status = 'ARCHIVED'       |       |             |
 idle  | COMMIT                                            |     7 | 00:30:29    | 05:39:05
 idle  | SELECT experiment.* FROM experiment WHERE status  |     5 | 00:41:22    | 01:03:28
       |   NOT IN ($1,$2) and page_id = $3                 |       |             |
 idle  | create extension if not exists vector...          |     3 | 06:56:16    | 07:00:39
 idle  | select * from multi_tree where child = $1...      |     2 | 04:33:04    | 05:39:05
 idle  | SELECT * FROM workflow_task WHERE webasset = $1...|     2 | 06:56:19    | 07:00:47

Connection age confirms true leak (not idle pool connection):

• connection_age = 16:34:26, idle_since = 16:33:30 — connection used for ~56 seconds, then abandoned for 16+ hours
• HikariCP maxLifetime (even at 60s due to #34921) cannot reclaim checked-out connections

Proposed Fix (two parts)

Part 1: Defensive cleanup + leak detection in MetricStatsCollector

After each metric completes (in the executor thread), check for and close any orphaned thread-local connection. Log a warning if a non-DBMetricType metric left a connection open — this surfaces future poison pills at development time rather than waiting for production leaks.

// After each metric's getValue() returns, on the executor thread:
try {
    result = metricType.getValue();
} finally {
    if (DbConnectionFactory.connectionExists()) {
        if (!(metricType instanceof DBMetricType)) {
            Logger.warn(MetricStatsCollector.class,
                "MetricType " + metricType.getClass().getSimpleName()
                + " left a DB connection open on the telemetry thread. "
                + "This connection will be closed defensively. "
                + "The metric should manage its own connection lifecycle.");
        }
        DbConnectionFactory.closeSilently();
    }
}

Part 2: Fix the known poison pill

Convert CountOfSitesWithThumbnailsMetricType to either:

• Wrap its getValue() in DbConnectionFactory.wrapConnection(), or
• Refactor getAllSitesInodes() to use DotConnect.loadObjectResults(Connection conn) with an explicit try-with-resources connection

Also audit TotalSitesWithAutoIndexContentConfigMetricType and TotalSitesUsingDotaiMetricType for similar patterns.

Other non-DBMetricType implementations to audit

Class	Risk	Reason
CountOfSitesWithThumbnailsMetricType	HIGH — confirmed poison pill	DotConnect.loadObjectResults() → DbConnectionFactory.getConnection(), no cleanup
TotalSitesWithAutoIndexContentConfigMetricType	Moderate	Calls HostAPIImpl.find() which has @CloseDBIfOpened — safe in isolation but won't close if ThreadLocal already poisoned
TotalSitesUsingDotaiMetricType	Moderate	Calls AppsAPIImpl methods — may open connections indirectly
TotalEmbeddingsIndexesMetricType	Low	Uses getPGVectorConnection() (direct DataSource), does NOT use ThreadLocal
OldStyleLanguagesVarialeMetricType	Low	Likely cache/filesystem only

Relationship to other issues

• #34490 (PR): The wrapConnection() fix this issue defeats. That fix is correct in isolation but has a design assumption that the ThreadLocal is clean when it starts — this issue documents the case where it isn't.
• #34837 (Epic): Parent epic for all connection leak work. This issue should be added as Tier 1 — it is the primary active leak on evergreen deployments.
• #34831: Experiments listActive() leak — separate code path (content operations, not telemetry). Still valid and needed.
• #34920: EmbeddingsFactory vector init leak — the create extension if not exists vector leak (3 connections observed). Separate from this issue.
• #34921: DB_MAXWAIT naming bug — causes maxLifetime=60s which accidentally cleans up SOME leaked connections, masking the true scale of this issue.

Start with MetricStatsCollector.getStats(), CountOfSitesWithThumbnailsMetricType, and DbConnectionFactory.wrapConnection(), connectionExists(), and closeSilently(). Trace how each metric uses the executor thread and verify cleanup after every metric, including the known non-DBMetricType poison pill. Done means orphaned ThreadLocal connections are closed, the warning is emitted for non-DBMetricType leaks, and the identified metric no longer leaves a connection open.