From 96de8e1c880090d3927246c836b54d6f1f43c95e Mon Sep 17 00:00:00 2001 From: sm Date: Wed, 20 Sep 2023 13:56:09 +0200 Subject: [PATCH] v1.5.3 --- RELEASENOTES.md | 3 +++ driver/connattrs.go | 17 +++---------- driver/connection.go | 44 +++++++++++++------------------- driver/connector.go | 60 +++++++++++++++++++++++++++++++++----------- driver/driver.go | 2 +- 5 files changed, 70 insertions(+), 56 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 8a4ed640..85c4051d 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -5,6 +5,9 @@ Release Notes ### Minor revisions +#### v1.5.3 +- performance improvements + #### v1.5.2 - fixed race condition in connection conn and stmt methods in case of context cancelling diff --git a/driver/connattrs.go b/driver/connattrs.go index b70efdee..7992110f 100644 --- a/driver/connattrs.go +++ b/driver/connattrs.go @@ -54,7 +54,6 @@ const ( // connAttrs is holding connection relevant attributes. type connAttrs struct { mu sync.RWMutex - _host string _timeout time.Duration _pingInterval time.Duration _bufferSize int @@ -72,7 +71,6 @@ type connAttrs struct { _cesu8Decoder func() transform.Transformer _cesu8Encoder func() transform.Transformer _emptyDateAsNull bool - _databaseName string _logger *slog.Logger } @@ -103,7 +101,6 @@ func (c *connAttrs) clone() *connAttrs { defer c.mu.RUnlock() return &connAttrs{ - _host: c._host, _timeout: c._timeout, _pingInterval: c._pingInterval, _bufferSize: c._bufferSize, @@ -121,7 +118,6 @@ func (c *connAttrs) clone() *connAttrs { _cesu8Decoder: c._cesu8Decoder, _cesu8Encoder: c._cesu8Encoder, _emptyDateAsNull: c._emptyDateAsNull, - _databaseName: c._databaseName, _logger: c._logger, } } @@ -192,9 +188,6 @@ func (c *connAttrs) setDfv(dfv int) { c._dfv = dfv } -// Host returns the host of the connector. -func (c *connAttrs) Host() string { c.mu.RLock(); defer c.mu.RUnlock(); return c._host } - // Timeout returns the timeout of the connector. func (c *connAttrs) Timeout() time.Duration { c.mu.RLock(); defer c.mu.RUnlock(); return c._timeout } @@ -444,13 +437,6 @@ func (c *connAttrs) SetEmptyDateAsNull(emptyDateAsNull bool) { c._emptyDateAsNull = emptyDateAsNull } -// DatabaseName returns the tenant database name of the connector. -func (c *connAttrs) DatabaseName() string { - c.mu.RLock() - defer c.mu.RUnlock() - return c._databaseName -} - // Logger returns the Logger instance of the connector. func (c *connAttrs) Logger() *slog.Logger { c.mu.RLock() @@ -462,5 +448,8 @@ func (c *connAttrs) Logger() *slog.Logger { func (c *connAttrs) SetLogger(logger *slog.Logger) { c.mu.Lock() defer c.mu.Unlock() + if logger == nil { + logger = slog.Default() + } c._logger = logger } diff --git a/driver/connection.go b/driver/connection.go index 678ced74..390fe611 100644 --- a/driver/connection.go +++ b/driver/connection.go @@ -206,17 +206,10 @@ func isAuthError(err error) bool { return hdbErrors.Code() == p.HdbErrAuthenticationFailed } -func connect(ctx context.Context, metrics *metrics, connAttrs *connAttrs, authAttrs *authAttrs) (driver.Conn, error) { - // if database name fetch tenant database host - if connAttrs._databaseName != "" { - if err := fetchHost(ctx, metrics, connAttrs); err != nil { - return nil, err - } - } - +func connect(ctx context.Context, host string, metrics *metrics, connAttrs *connAttrs, authAttrs *authAttrs) (driver.Conn, error) { // can we connect via cookie? if auth := authAttrs.cookieAuth(); auth != nil { - conn, err := newSession(ctx, metrics, connAttrs, auth) + conn, err := newSession(ctx, host, metrics, connAttrs, auth) if err == nil { return conn, nil } @@ -226,14 +219,13 @@ func connect(ctx context.Context, metrics *metrics, connAttrs *connAttrs, authAt authAttrs.invalidateCookie() // cookie auth was not successful - do not try again with the same data } - const maxRetry = 1 - numRetry := 0 + refreshed := false lastVersion := authAttrs.version.Load() for { authHnd := authAttrs.authHnd() - conn, err := newSession(ctx, metrics, connAttrs, authHnd) + conn, err := newSession(ctx, host, metrics, connAttrs, authHnd) if err == nil { if method, ok := authHnd.Selected().(auth.CookieGetter); ok { authAttrs.setCookie(method.Cookie()) @@ -243,7 +235,7 @@ func connect(ctx context.Context, metrics *metrics, connAttrs *connAttrs, authAt if !isAuthError(err) { return nil, err } - if numRetry >= maxRetry { + if refreshed { return nil, err } @@ -257,7 +249,7 @@ func connect(ctx context.Context, metrics *metrics, connAttrs *connAttrs, authAt } lastVersion = version - numRetry++ + refreshed = true } } @@ -295,8 +287,8 @@ func (nvs namedValues) LogValue() slog.Value { // unique connection number. var connNo atomic.Uint64 -func newConn(ctx context.Context, metrics *metrics, attrs *connAttrs) (*conn, error) { - netConn, err := attrs._dialer.DialContext(ctx, attrs._host, dial.DialerOptions{Timeout: attrs._timeout, TCPKeepAlive: attrs._tcpKeepAlive}) +func newConn(ctx context.Context, host string, metrics *metrics, attrs *connAttrs) (*conn, error) { + netConn, err := attrs._dialer.DialContext(ctx, host, dial.DialerOptions{Timeout: attrs._timeout, TCPKeepAlive: attrs._tcpKeepAlive}) if err != nil { return nil, err } @@ -339,24 +331,24 @@ func newConn(ctx context.Context, metrics *metrics, attrs *connAttrs) (*conn, er return c, nil } -func fetchHost(ctx context.Context, metrics *metrics, attrs *connAttrs) error { - c, err := newConn(ctx, metrics, attrs) +func fetchRedirectHost(ctx context.Context, host, databaseName string, metrics *metrics, attrs *connAttrs) (string, error) { + c, err := newConn(ctx, host, metrics, attrs) if err != nil { - return err + return "", err } defer c.Close() - dbi, err := c._dbConnectInfo(ctx, attrs._databaseName) + dbi, err := c._dbConnectInfo(ctx, databaseName) if err != nil { - return err + return "", err } - if !dbi.IsConnected { // if databaseName == "SYSTEMDB" and isConnected == true host and port are initial - attrs._host = net.JoinHostPort(dbi.Host, strconv.Itoa(dbi.Port)) + if dbi.IsConnected { // if databaseName == "SYSTEMDB" and isConnected == true host and port are initial + return host, nil } - return nil + return net.JoinHostPort(dbi.Host, strconv.Itoa(dbi.Port)), nil } -func newSession(ctx context.Context, metrics *metrics, attrs *connAttrs, authHnd *p.AuthHnd) (driver.Conn, error) { - c, err := newConn(ctx, metrics, attrs) +func newSession(ctx context.Context, host string, metrics *metrics, attrs *connAttrs, authHnd *p.AuthHnd) (driver.Conn, error) { + c, err := newConn(ctx, host, metrics, attrs) if err != nil { return nil, err } diff --git a/driver/connector.go b/driver/connector.go index 62dd3b80..713954f6 100644 --- a/driver/connector.go +++ b/driver/connector.go @@ -5,21 +5,29 @@ import ( "database/sql/driver" "os" "path" + "sync" "github.com/SAP/go-hdb/driver/internal/protocol/x509" ) +type redirectCacheKey struct { + host, databaseName string +} + +var redirectCache sync.Map + /* A Connector represents a hdb driver in a fixed configuration. A Connector can be passed to sql.OpenDB (starting from go 1.10) allowing users to bypass a string based data source name. */ type Connector struct { + _host string + _databaseName string + *connAttrs *authAttrs metrics *metrics - - connHook func(driver.Conn) driver.Conn } // NewConnector returns a new Connector instance with default values. @@ -102,27 +110,53 @@ func NewDSNConnector(dsnStr string) (*Connector, error) { // NativeDriver returns the concrete underlying Driver of the Connector. func (c *Connector) NativeDriver() Driver { return stdHdbDriver } -// Connect implements the database/sql/driver/Connector interface. -func (c *Connector) Connect(ctx context.Context) (driver.Conn, error) { - conn, err := connect(ctx, c.metrics, c.connAttrs.clone(), c.authAttrs) +// Host returns the host of the connector. +func (c *Connector) Host() string { return c._host } + +// DatabaseName returns the tenant database name of the connector. +func (c *Connector) DatabaseName() string { return c._databaseName } + +func (c *Connector) redirect(ctx context.Context) (driver.Conn, error) { + connAttrs := c.connAttrs.clone() + + if redirectHost, found := redirectCache.Load(redirectCacheKey{host: c._host, databaseName: c._databaseName}); found { + if conn, err := connect(ctx, redirectHost.(string), c.metrics, connAttrs, c.authAttrs); err == nil { + return conn, nil + } + } + + redirectHost, err := fetchRedirectHost(ctx, c._host, c._databaseName, c.metrics, connAttrs) if err != nil { return nil, err } - if c.connHook != nil { - conn = c.connHook(conn) + conn, err := connect(ctx, redirectHost, c.metrics, connAttrs, c.authAttrs) + if err != nil { + return nil, err } + + redirectCache.Store(redirectCacheKey{host: c._host, databaseName: c._databaseName}, redirectHost) + return conn, err } +// Connect implements the database/sql/driver/Connector interface. +func (c *Connector) Connect(ctx context.Context) (driver.Conn, error) { + if c._databaseName != "" { + return c.redirect(ctx) + } + return connect(ctx, c._host, c.metrics, c.connAttrs.clone(), c.authAttrs) +} + // Driver implements the database/sql/driver/Connector interface. func (c *Connector) Driver() driver.Driver { return stdHdbDriver } func (c *Connector) clone() *Connector { return &Connector{ - connAttrs: c.connAttrs.clone(), - authAttrs: c.authAttrs.clone(), - metrics: c.metrics, - connHook: c.connHook, + _host: c._host, + _databaseName: c._databaseName, + connAttrs: c.connAttrs.clone(), + authAttrs: c.authAttrs.clone(), + metrics: c.metrics, } } @@ -132,7 +166,3 @@ func (c *Connector) WithDatabase(databaseName string) *Connector { nc._databaseName = databaseName return nc } - -// SetConnHook sets a function for intercepting connection creation. -// This is for internal use only and might be changed or disabled in future. -func (c *Connector) SetConnHook(fn func(driver.Conn) driver.Conn) { c.connHook = fn } diff --git a/driver/driver.go b/driver/driver.go index 725b54ea..ccfda7e4 100644 --- a/driver/driver.go +++ b/driver/driver.go @@ -10,7 +10,7 @@ import ( ) // DriverVersion is the version number of the hdb driver. -const DriverVersion = "1.5.2" +const DriverVersion = "1.5.3" // DriverName is the driver name to use with sql.Open for hdb databases. const DriverName = "hdb"