From a51a8e9b60efbb58c26a8ba0786db240eb98a921 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 20 Mar 2025 11:38:29 +0530 Subject: [PATCH] MM-62964: Skip setting replica lag handle in case of an error (#30498) Previously, we would set it to nil pointer which would eventually cause a panic when ReplicaLagAbs/ReplicaLagTime would get called. https://mattermost.atlassian.net/browse/MM-62964 ```release-note NONE ``` Co-authored-by: Mattermost Build --- server/channels/store/sqlstore/store.go | 5 ++- server/channels/store/sqlstore/store_test.go | 44 ++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/server/channels/store/sqlstore/store.go b/server/channels/store/sqlstore/store.go index b8b97c8e11..fc2e547f9d 100644 --- a/server/channels/store/sqlstore/store.go +++ b/server/channels/store/sqlstore/store.go @@ -341,16 +341,17 @@ func (ss *SqlStore) initConnection() error { } if len(ss.settings.ReplicaLagSettings) > 0 { - ss.replicaLagHandles = make([]*dbsql.DB, len(ss.settings.ReplicaLagSettings)) + ss.replicaLagHandles = make([]*dbsql.DB, 0, len(ss.settings.ReplicaLagSettings)) for i, src := range ss.settings.ReplicaLagSettings { if src.DataSource == nil { continue } - ss.replicaLagHandles[i], err = sqlUtils.SetupConnection(ss.Logger(), fmt.Sprintf(replicaLagPrefix+"-%d", i), *src.DataSource, ss.settings, DBReplicaPingAttempts) + replicaLagHandle, err := sqlUtils.SetupConnection(ss.Logger(), fmt.Sprintf(replicaLagPrefix+"-%d", i), *src.DataSource, ss.settings, DBReplicaPingAttempts) if err != nil { mlog.Warn("Failed to setup replica lag handle. Skipping..", mlog.String("db", fmt.Sprintf(replicaLagPrefix+"-%d", i)), mlog.Err(err)) continue } + ss.replicaLagHandles = append(ss.replicaLagHandles, replicaLagHandle) } } return nil diff --git a/server/channels/store/sqlstore/store_test.go b/server/channels/store/sqlstore/store_test.go index a0f55253c8..64bd551538 100644 --- a/server/channels/store/sqlstore/store_test.go +++ b/server/channels/store/sqlstore/store_test.go @@ -795,6 +795,50 @@ func TestReplicaLagQuery(t *testing.T) { } } +func TestInvalidReplicaLagDataSource(t *testing.T) { + logger := mlog.CreateConsoleTestLogger(t) + + testDrivers := []string{ + model.DatabaseDriverPostgres, + model.DatabaseDriverMysql, + } + + for _, driver := range testDrivers { + t.Run(driver, func(t *testing.T) { + settings, err := makeSqlSettings(driver) + if err != nil { + t.Skip(err) + } + + // Set an invalid DataSource that will fail to connect + settings.ReplicaLagSettings = []*model.ReplicaLagSettings{{ + DataSource: model.NewPointer("invalid://connection/string"), + QueryAbsoluteLag: model.NewPointer("SELECT 1"), + QueryTimeLag: model.NewPointer("SELECT 1"), + }} + + mockMetrics := &mocks.MetricsInterface{} + mockMetrics.On("RegisterDBCollector", mock.AnythingOfType("*sql.DB"), "master") + + store := &SqlStore{ + rrCounter: 0, + srCounter: 0, + settings: settings, + metrics: mockMetrics, + logger: logger, + quitMonitor: make(chan struct{}), + wgMonitor: &sync.WaitGroup{}, + } + + require.NoError(t, store.initConnection()) + defer store.Close() + + // Verify no replica lag handles were added despite having ReplicaLagSettings + assert.Equal(t, 0, len(store.replicaLagHandles)) + }) + } +} + var errDriverMismatch = errors.New("database drivers mismatch") var errDriverUnsupported = errors.New("database driver not supported")