Skip to content

Conversation

@OGKevin
Copy link

@OGKevin OGKevin commented Jul 30, 2025

🤖 Generated body based on the diff 😁


Summary

This merge request addresses a critical ClickHouse URL corruption bug that occurred during connection retries and introduces several improvements to database connection handling, including context support with timeouts.

Bug Fix

  • Fixed ClickHouse URL corruption bug that occurred on retry attempts
    • Root cause: connectClickHouse() was mutating the connection struct's driver field
    • Solution: Modified to use local variables instead of modifying the original connection object
    • Before fix: Second connection attempt failed with dial tcp: lookup clickhouse+https: no such host
    • After fix: All connection attempts process URLs correctly
    • Bug affected all ClickHouse connection types (HTTP, HTTPS, TCP)

Connection Handling Improvements

  • Added context support throughout the database connection call chain
    • Updated method signatures to accept and propagate context
    • Replaced blocking Ping() calls with context-aware PingContext(ctx)
    • Added configurable timeout support for database operations
    • Default 30-second timeout if not explicitly configured

TLS Configuration Enhancements

  • Refactored ClickHouse TLS configuration for better testability and maintainability
    • Created MTLSSetup interface for dependency injection
    • Added TLSConfigResult structure to encapsulate TLS configuration results
    • Extracted TLS configuration logic into configureClickHouseTLS()
    • Unified ClickHouse connection handling in connectClickHouse()

Testing Improvements

  • Enhanced test coverage for ClickHouse TLS configuration
    • Added integration tests that use the actual production code path
    • Implemented proper dependency injection with mock objects
    • Created comprehensive test cases for URL parameter handling
    • Removed redundant test (TestURLParameterRemovalUnit)

Compatibility

  • Maintains backward compatibility with all existing connection string formats
  • Preserves all connection pool settings and StartupSQL execution
  • No breaking API changes

Testing

  • All tests pass, including the new ClickHouse URL bug reproduction test
  • Code compiles successfully with go build .
  • All existing functionality remains intact

Configuration Example

Users can now configure timeouts in their job YAML:

jobs:
- name: clickhouse_job
  timeout: 10s  # Custom timeout for database operations
  connections:
    - "clickhouse+http://localhost:9000/default"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants