Skip to content

refactor: add thread-safety to FireboltDriver with RWMutex #157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

refactor: add thread-safety to FireboltDriver with RWMutex #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 34 additions & 22 deletions driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"database/sql"
"database/sql/driver"
"sync"

"github.com/firebolt-db/firebolt-go-sdk/client"

Expand All @@ -12,6 +13,7 @@ import (
)

type FireboltDriver struct {
mu sync.RWMutex
engineUrl string
cachedParams map[string]string
client client.Client
Expand All @@ -38,29 +40,39 @@ func copyMap(original map[string]string) map[string]string {
func (d *FireboltDriver) OpenConnector(dsn string) (driver.Connector, error) {
logging.Infolog.Println("Opening firebolt connector")

if d.lastUsedDsn != dsn || d.lastUsedDsn == "" {

d.lastUsedDsn = "" //nolintd
logging.Infolog.Println("constructing new client")
// parsing dsn string to get configuration settings
settings, err := ParseDSNString(dsn)
if err != nil {
return nil, errors.Wrap(errors.DSNParseError, err)
}

// authenticating and getting access token
logging.Infolog.Println("dsn parsed correctly, trying to authenticate")
d.client, err = client.ClientFactory(settings, client.GetHostNameURL())
if err != nil {
return nil, errors.ConstructNestedError("error during initializing client", err)
}

d.engineUrl, d.cachedParams, err = d.client.GetConnectionParameters(context.TODO(), settings.EngineName, settings.Database)
if err != nil {
return nil, errors.ConstructNestedError("error during getting connection parameters", err)
}
d.lastUsedDsn = dsn //nolint
d.mu.RLock()
if d.lastUsedDsn == dsn && d.lastUsedDsn != "" {
connector := &FireboltConnector{d.engineUrl, d.client, copyMap(d.cachedParams), d}
d.mu.RUnlock()
return connector, nil
}
d.mu.RUnlock()

d.mu.Lock()
defer d.mu.Unlock()

if d.lastUsedDsn == dsn && d.lastUsedDsn != "" {
return &FireboltConnector{d.engineUrl, d.client, copyMap(d.cachedParams), d}, nil
}

d.lastUsedDsn = ""
logging.Infolog.Println("constructing new client")
settings, err := ParseDSNString(dsn)
if err != nil {
return nil, errors.Wrap(errors.DSNParseError, err)
}

logging.Infolog.Println("dsn parsed correctly, trying to authenticate")
d.client, err = client.ClientFactory(settings, client.GetHostNameURL())
if err != nil {
return nil, errors.ConstructNestedError("error during initializing client", err)
}

d.engineUrl, d.cachedParams, err = d.client.GetConnectionParameters(context.TODO(), settings.EngineName, settings.Database)
if err != nil {
return nil, errors.ConstructNestedError("error during getting connection parameters", err)
}
d.lastUsedDsn = dsn

return &FireboltConnector{d.engineUrl, d.client, copyMap(d.cachedParams), d}, nil
}
Expand Down
14 changes: 12 additions & 2 deletions driver_options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@ func NoError(option driverOption) driverOptionWithError {
// WithEngineUrl defines engine url for the driver
func WithEngineUrl(engineUrl string) driverOption {
return func(d *FireboltDriver) {
d.mu.Lock()
defer d.mu.Unlock()
d.engineUrl = engineUrl
}
}

// WithDatabaseName defines database name for the driver
func WithDatabaseName(databaseName string) driverOption {
return func(d *FireboltDriver) {
d.mu.Lock()
defer d.mu.Unlock()
if d.cachedParams == nil {
d.cachedParams = map[string]string{}
}
Expand All @@ -37,6 +41,8 @@ func WithDatabaseName(databaseName string) driverOption {
// WithAccountID defines account ID for the driver
func WithAccountID(accountID string) driverOption {
return func(d *FireboltDriver) {
d.mu.Lock()
defer d.mu.Unlock()
if d.cachedParams == nil {
d.cachedParams = map[string]string{}
}
Expand All @@ -48,11 +54,12 @@ func WithAccountID(accountID string) driverOption {

func withClientOption(setter func(baseClient *client.BaseClient)) driverOption {
return func(d *FireboltDriver) {
d.mu.Lock()
defer d.mu.Unlock()
if d.client != nil {
if clientImpl, ok := d.client.(*client.ClientImpl); ok {
setter(&clientImpl.BaseClient)
}
// ignore V0 client since it's not supported
} else {
cl := &client.ClientImpl{
ConnectedToSystemEngine: true,
Expand Down Expand Up @@ -95,11 +102,12 @@ func WithClientParams(accountID string, token string, userAgent string) driverOp
// WithAccountName defines account name for the driver
func WithAccountName(accountName string) driverOptionWithError {
return func(d *FireboltDriver) error {
d.mu.Lock()
defer d.mu.Unlock()
if d.client != nil {
if clientImpl, ok := d.client.(*client.ClientImpl); ok {
clientImpl.AccountName = accountName
}
// ignore V0 client since it's not supported
} else {
cl := &client.ClientImpl{
ConnectedToSystemEngine: true,
Expand All @@ -118,6 +126,8 @@ func WithAccountName(accountName string) driverOptionWithError {
// WithDatabaseAndEngineName defines database name and engine name for the driver
func WithDatabaseAndEngineName(databaseName, engineName string) driverOptionWithError {
return func(d *FireboltDriver) error {
d.mu.Lock()
defer d.mu.Unlock()
if d.client == nil {
return errors.New("client must be initialized before setting database and engine name")
}
Expand Down
Loading