Use pointer when passing the connection manager around (#3152)

As otherwise existing connections aren't reused.
This commit is contained in:
Till 2023-07-19 13:37:04 +02:00 committed by GitHub
parent a01faee17c
commit 297479ea49
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
31 changed files with 143 additions and 79 deletions

View file

@ -29,24 +29,26 @@ type Connections struct {
processContext *process.ProcessContext
}
func NewConnectionManager(processCtx *process.ProcessContext, globalConfig config.DatabaseOptions) Connections {
return Connections{
func NewConnectionManager(processCtx *process.ProcessContext, globalConfig config.DatabaseOptions) *Connections {
return &Connections{
globalConfig: globalConfig,
processContext: processCtx,
}
}
func (c *Connections) Connection(dbProperties *config.DatabaseOptions) (*sql.DB, Writer, error) {
writer := NewDummyWriter()
if dbProperties.ConnectionString.IsSQLite() {
writer = NewExclusiveWriter()
}
var err error
if dbProperties.ConnectionString == "" {
// if no connectionString was provided, try the global one
dbProperties = &c.globalConfig
}
if dbProperties.ConnectionString != "" || c.db == nil {
writer := NewDummyWriter()
if dbProperties.ConnectionString.IsSQLite() {
writer = NewExclusiveWriter()
}
if dbProperties.ConnectionString != "" && c.db == nil {
// Open a new database connection using the supplied config.
c.db, err = Open(dbProperties, writer)
if err != nil {

View file

@ -6,51 +6,113 @@ import (
"github.com/matrix-org/dendrite/internal/sqlutil"
"github.com/matrix-org/dendrite/setup/config"
"github.com/matrix-org/dendrite/setup/process"
"github.com/matrix-org/dendrite/test"
)
func TestConnectionManager(t *testing.T) {
test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) {
conStr, close := test.PrepareDBConnectionString(t, dbType)
t.Cleanup(close)
cm := sqlutil.NewConnectionManager(nil, config.DatabaseOptions{})
dbProps := &config.DatabaseOptions{ConnectionString: config.DataSource(conStr)}
db, writer, err := cm.Connection(dbProps)
if err != nil {
t.Fatal(err)
}
t.Run("component defined connection string", func(t *testing.T) {
test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) {
conStr, close := test.PrepareDBConnectionString(t, dbType)
t.Cleanup(close)
cm := sqlutil.NewConnectionManager(nil, config.DatabaseOptions{})
switch dbType {
case test.DBTypeSQLite:
_, ok := writer.(*sqlutil.ExclusiveWriter)
if !ok {
t.Fatalf("expected exclusive writer")
dbProps := &config.DatabaseOptions{ConnectionString: config.DataSource(conStr)}
db, writer, err := cm.Connection(dbProps)
if err != nil {
t.Fatal(err)
}
case test.DBTypePostgres:
_, ok := writer.(*sqlutil.DummyWriter)
if !ok {
t.Fatalf("expected dummy writer")
switch dbType {
case test.DBTypeSQLite:
_, ok := writer.(*sqlutil.ExclusiveWriter)
if !ok {
t.Fatalf("expected exclusive writer")
}
case test.DBTypePostgres:
_, ok := writer.(*sqlutil.DummyWriter)
if !ok {
t.Fatalf("expected dummy writer")
}
}
}
// test global db pool
dbGlobal, writerGlobal, err := cm.Connection(&config.DatabaseOptions{})
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(db, dbGlobal) {
t.Fatalf("expected database connection to be reused")
}
if !reflect.DeepEqual(writer, writerGlobal) {
t.Fatalf("expected database writer to be reused")
}
// test invalid connection string configured
cm2 := sqlutil.NewConnectionManager(nil, config.DatabaseOptions{})
_, _, err = cm2.Connection(&config.DatabaseOptions{ConnectionString: "http://"})
if err == nil {
t.Fatal("expected an error but got none")
}
// reuse existing connection
db2, writer2, err := cm.Connection(dbProps)
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(db, db2) {
t.Fatalf("expected database connection to be reused")
}
if !reflect.DeepEqual(writer, writer2) {
t.Fatalf("expected database writer to be reused")
}
})
})
t.Run("global connection pool", func(t *testing.T) {
test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) {
conStr, close := test.PrepareDBConnectionString(t, dbType)
t.Cleanup(close)
cm := sqlutil.NewConnectionManager(nil, config.DatabaseOptions{ConnectionString: config.DataSource(conStr)})
dbProps := &config.DatabaseOptions{}
db, writer, err := cm.Connection(dbProps)
if err != nil {
t.Fatal(err)
}
switch dbType {
case test.DBTypeSQLite:
_, ok := writer.(*sqlutil.ExclusiveWriter)
if !ok {
t.Fatalf("expected exclusive writer")
}
case test.DBTypePostgres:
_, ok := writer.(*sqlutil.DummyWriter)
if !ok {
t.Fatalf("expected dummy writer")
}
}
// reuse existing connection
db2, writer2, err := cm.Connection(dbProps)
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(db, db2) {
t.Fatalf("expected database connection to be reused")
}
if !reflect.DeepEqual(writer, writer2) {
t.Fatalf("expected database writer to be reused")
}
})
})
t.Run("shutdown", func(t *testing.T) {
test.WithAllDatabases(t, func(t *testing.T, dbType test.DBType) {
conStr, close := test.PrepareDBConnectionString(t, dbType)
t.Cleanup(close)
processCtx := process.NewProcessContext()
cm := sqlutil.NewConnectionManager(processCtx, config.DatabaseOptions{ConnectionString: config.DataSource(conStr)})
dbProps := &config.DatabaseOptions{}
_, _, err := cm.Connection(dbProps)
if err != nil {
t.Fatal(err)
}
processCtx.ShutdownDendrite()
processCtx.WaitForComponentsToFinish()
})
})
// test invalid connection string configured
cm2 := sqlutil.NewConnectionManager(nil, config.DatabaseOptions{})
_, _, err := cm2.Connection(&config.DatabaseOptions{ConnectionString: "http://"})
if err == nil {
t.Fatal("expected an error but got none")
}
}