MM-56402: Initialize replica conn pool on-demand (#26410)

Previously, we would setup both pools only when
GetMasterDB was called. This was inefficient and
would waste open connections if the replica wasn't used
at all.

We fix it to initialize the pools as they are called.

https://mattermost.atlassian.net/browse/MM-56402
```release-note
NONE
```

Co-authored-by: Jesse Hallam <jesse.hallam@gmail.com>
This commit is contained in:
Agniva De Sarker 2024-03-08 13:11:48 +05:30 committed by GitHub
parent bc887441d0
commit 89f5a0deec
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 32 additions and 32 deletions

View File

@ -15,10 +15,11 @@ import (
// StoreService exposes the underlying database. // StoreService exposes the underlying database.
type StoreService struct { type StoreService struct {
initialized bool initializedMaster bool
api plugin.API initializedReplica bool
driver plugin.Driver api plugin.API
mutex sync.Mutex driver plugin.Driver
mutex sync.Mutex
masterDB *sql.DB masterDB *sql.DB
replicaDB *sql.DB replicaDB *sql.DB
@ -31,7 +32,7 @@ func (s *StoreService) GetMasterDB() (*sql.DB, error) {
s.mutex.Lock() s.mutex.Lock()
defer s.mutex.Unlock() defer s.mutex.Unlock()
if err := s.initialize(); err != nil { if err := s.initializeMaster(); err != nil {
return nil, err return nil, err
} }
@ -46,7 +47,7 @@ func (s *StoreService) GetReplicaDB() (*sql.DB, error) {
s.mutex.Lock() s.mutex.Lock()
defer s.mutex.Unlock() defer s.mutex.Unlock()
if err := s.initialize(); err != nil { if err := s.initializeReplica(); err != nil {
return nil, err return nil, err
} }
@ -62,20 +63,18 @@ func (s *StoreService) Close() error {
s.mutex.Lock() s.mutex.Lock()
defer s.mutex.Unlock() defer s.mutex.Unlock()
if !s.initialized {
return nil
}
if err := s.masterDB.Close(); err != nil {
return err
}
if s.replicaDB != nil { if s.replicaDB != nil {
if err := s.replicaDB.Close(); err != nil { if err := s.replicaDB.Close(); err != nil {
return err return err
} }
} }
if s.masterDB != nil {
if err := s.masterDB.Close(); err != nil {
return err
}
}
return nil return nil
} }
@ -84,8 +83,8 @@ func (s *StoreService) DriverName() string {
return *s.api.GetConfig().SqlSettings.DriverName return *s.api.GetConfig().SqlSettings.DriverName
} }
func (s *StoreService) initialize() error { func (s *StoreService) initializeMaster() error {
if s.initialized { if s.initializedMaster {
return nil return nil
} }
@ -93,25 +92,33 @@ func (s *StoreService) initialize() error {
return errors.New("no db driver was provided") return errors.New("no db driver was provided")
} }
config := s.api.GetUnsanitizedConfig()
// Set up master db // Set up master db
db := sql.OpenDB(driver.NewConnector(s.driver, true)) db := sql.OpenDB(driver.NewConnector(s.driver, true /* IsMaster */))
if err := db.Ping(); err != nil { if err := db.Ping(); err != nil {
return errors.Wrap(err, "failed to connect to master db") return errors.Wrap(err, "failed to connect to master db")
} }
s.masterDB = db s.masterDB = db
s.initializedMaster = true
return nil
}
func (s *StoreService) initializeReplica() error {
if s.initializedReplica {
return nil
}
config := s.api.GetUnsanitizedConfig()
// Set up replica db // Set up replica db
if len(config.SqlSettings.DataSourceReplicas) > 0 { if len(config.SqlSettings.DataSourceReplicas) > 0 {
db := sql.OpenDB(driver.NewConnector(s.driver, false)) db := sql.OpenDB(driver.NewConnector(s.driver, false /* IsMaster */))
if err := db.Ping(); err != nil { if err := db.Ping(); err != nil {
return errors.Wrap(err, "failed to connect to replica db") return errors.Wrap(err, "failed to connect to replica db")
} }
s.replicaDB = db s.replicaDB = db
} }
s.initialized = true s.initializedReplica = true
return nil return nil
} }

View File

@ -12,18 +12,10 @@ import (
func TestStore(t *testing.T) { func TestStore(t *testing.T) {
t.Run("master db singleton", func(t *testing.T) { t.Run("master db singleton", func(t *testing.T) {
config := &model.Config{
SqlSettings: model.SqlSettings{
DriverName: model.NewString("test"),
DataSource: model.NewString("TestStore-master-db"),
},
}
api := &plugintest.API{} api := &plugintest.API{}
defer api.AssertExpectations(t)
api.On("GetUnsanitizedConfig").Return(config)
driver := &plugintest.Driver{} driver := &plugintest.Driver{}
defer driver.AssertExpectations(t)
driver.On("Conn", true).Return("test", nil) driver.On("Conn", true).Return("test", nil)
driver.On("ConnPing", "test").Return(nil) driver.On("ConnPing", "test").Return(nil)
driver.On("ConnClose", "test").Return(nil) driver.On("ConnClose", "test").Return(nil)
@ -52,6 +44,7 @@ func TestStore(t *testing.T) {
} }
driver := &plugintest.Driver{} driver := &plugintest.Driver{}
defer driver.AssertExpectations(t)
driver.On("Conn", true).Return("test", nil) driver.On("Conn", true).Return("test", nil)
driver.On("ConnPing", "test").Return(nil) driver.On("ConnPing", "test").Return(nil)
driver.On("ConnClose", "test").Return(nil) driver.On("ConnClose", "test").Return(nil)
@ -88,7 +81,7 @@ func TestStore(t *testing.T) {
api.On("GetUnsanitizedConfig").Return(config) api.On("GetUnsanitizedConfig").Return(config)
driver := &plugintest.Driver{} driver := &plugintest.Driver{}
driver.On("Conn", true).Return("test", nil) defer driver.AssertExpectations(t)
driver.On("Conn", false).Return("test", nil) driver.On("Conn", false).Return("test", nil)
driver.On("ConnPing", "test").Return(nil) driver.On("ConnPing", "test").Return(nil)
driver.On("ConnClose", "test").Return(nil) driver.On("ConnClose", "test").Return(nil)