Skip to content

BUG: Resource Leak - JDBC Connections Never Closed in ClickHouseBatchWriter #1252

@minguyen9988

Description

@minguyen9988

Summary

ClickHouseBatchWriter.java creates JDBC connections in createConnection() and getClickHouseConnection() but never closes them. There are no .close() calls, no try-with-resources, and no shutdown/cleanup method. This leads to connection pool exhaustion over time. The same issue was fixed in ClickHouseBatchRunnable.java which has proper close() calls with exception handling.

Location

  • File: sink-connector/src/main/java/com/altinity/clickhouse/sink/connector/executor/ClickHouseBatchWriter.java
  • Lines: 55 (databaseToConnectionMap), 125-134 (createConnection), 144-158 (getClickHouseConnection)

Current Code

// Line 55: Connections stored but never cleaned up
private Map<String, Connection> databaseToConnectionMap = new HashMap<>();

// Lines 125-134: Connection created, no cleanup on failure
private Connection createConnection(String database) {
    String jdbcUrl = BaseDbWriter.getConnectionString(...);
    return BaseDbWriter.createConnection(jdbcUrl, ...);
}

// Lines 144-158: Check-then-act with no cleanup
private Connection getClickHouseConnection(String databaseName) {
    if (this.databaseToConnectionMap.containsKey(databaseName)) {
        return this.databaseToConnectionMap.get(databaseName);
    }
    // ... creates connection, stores it, but never closes on error
    Connection conn = BaseDbWriter.createConnection(...);
    this.databaseToConnectionMap.put(databaseName, conn);
    return conn;
}

Comparison with Fixed Code (ClickHouseBatchRunnable.java)

ClickHouseBatchRunnable.getClickHouseConnection() (lines 203-274) has:

  • Proper try/catch/finally blocks
  • newConn.close() when another thread created the connection first (line 245)
  • newConn.close() in catch blocks to prevent leaks (line 258)
  • systemConn.close() for temporary system connections (line 268)

Problem

  1. No close() method exists on ClickHouseBatchWriter to clean up connections when the connector shuts down
  2. If getClickHouseConnection() fails partway through (after creating system connection but before storing), the system connection leaks
  3. The systemConnection field (line 49) is created in the constructor but never closed
  4. Over time in long-running deployments, this leads to connection pool exhaustion (CRASH-6)

Impact

  • Severity: High (P1)
  • Effect: Gradual connection pool exhaustion, eventually causing SQLTransientConnectionException
  • Scope: All lightweight connector deployments running for extended periods
  • Symptom: Connector becomes unresponsive after hours/days of operation

Suggested Fix

  1. Add a close() / shutdown() method that iterates databaseToConnectionMap and closes all connections
  2. Wrap getClickHouseConnection() in try-with-resources or try/finally for intermediate connections
  3. Mirror the cleanup pattern used in ClickHouseBatchRunnable.getClickHouseConnection()
  4. Implement AutoCloseable interface on ClickHouseBatchWriter

References

  • doc/issues/CONCURRENCY-BUGS.md (BUG-CONC-4)
  • doc/issues/CRASH-SCENARIOS.md (CRASH-6)
  • Fixed in ClickHouseBatchRunnable.java but not ClickHouseBatchWriter.java

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinglightweightIssues related to Lightweight versionp1

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions