Skip to content

Commit

Permalink
fix: Clickhouse driver should always use native protocol unless a DSN…
Browse files Browse the repository at this point in the history
… is set directly (#6293)

* always use native protocol unless specified otherwise in DSN

* switch to http port if connection with native port failed
  • Loading branch information
k-anshul authored Dec 20, 2024
1 parent 9061a24 commit 774395f
Showing 1 changed file with 21 additions and 10 deletions.
31 changes: 21 additions & 10 deletions runtime/drivers/clickhouse/clickhouse.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/tls"
"errors"
"fmt"
"strings"

"github.com/ClickHouse/clickhouse-go/v2"
"github.com/XSAM/otelsql"
Expand Down Expand Up @@ -149,14 +150,11 @@ func (d driver) Open(instanceID string, config map[string]any, st *storage.Clien
host = fmt.Sprintf("%s:%d", conf.Host, conf.Port)
}
opts.Addr = []string{host}
opts.Protocol = clickhouse.Native
if conf.SSL {
opts.Protocol = clickhouse.HTTP
opts.TLS = &tls.Config{
MinVersion: tls.VersionTLS12,
InsecureSkipVerify: false,
MinVersion: tls.VersionTLS12,
}
} else {
opts.Protocol = clickhouse.Native
}

// username password
Expand Down Expand Up @@ -190,6 +188,24 @@ func (d driver) Open(instanceID string, config map[string]any, st *storage.Clien
}

db := sqlx.NewDb(otelsql.OpenDB(clickhouse.Connector(opts)), "clickhouse")
err = db.Ping()
if err != nil {
if !strings.Contains(err.Error(), "unexpected packet") && !strings.Contains(err.Error(), "i/o timeout") {
return nil, err
}
if conf.DSN != "" {
return nil, err
}
// may be the port is http, also try with http protocol if DSN is not provided
opts.Protocol = clickhouse.HTTP
db = sqlx.NewDb(otelsql.OpenDB(clickhouse.Connector(opts)), "clickhouse")
err := db.Ping()
if err != nil {
return nil, err
}
// connection with http protocol is successful
logger.Warn("clickHouse connection is established with HTTP protocol. Use native port for better performance")
}
// very roughly approximating num queries required for a typical page load
// TODO: copied from druid reevaluate
db.SetMaxOpenConns(maxOpenConnections)
Expand All @@ -199,11 +215,6 @@ func (d driver) Open(instanceID string, config map[string]any, st *storage.Clien
return nil, fmt.Errorf("registering db stats metrics: %w", err)
}

err = db.Ping()
if err != nil {
return nil, fmt.Errorf("connection: %w", err)
}

// group by positional args are supported post 22.7 and we use them heavily in our queries
row := db.QueryRow(`
WITH
Expand Down

0 comments on commit 774395f

Please sign in to comment.