From 63c959f1d39ccda9a5263856f4c57f3eb735e0cd Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 14 Nov 2024 09:45:52 +0530 Subject: [PATCH] Fail fast while connecting to replicas (#29212) Since we have the auto-retry mechanism for replicas, there is not much benefit in trying 5 times before deciding to move on. In the earlier model, we would fail the server startup, so it made sense to check as many times as possible. Also reducing the sleep interval so that we can improve the boot up time in case of a bad replica. ```release-note NONE ``` * fix: Add job name to the publish report step --------- Co-authored-by: Mattermost Build Co-authored-by: Antonis Stamatiou --- .github/workflows/server-test-template.yml | 1 + server/channels/store/sqlstore/store.go | 7 ++++--- server/public/utils/sql/sql_utils.go | 9 +++++---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.github/workflows/server-test-template.yml b/.github/workflows/server-test-template.yml index 77c7268464..542c17eb63 100644 --- a/.github/workflows/server-test-template.yml +++ b/.github/workflows/server-test-template.yml @@ -79,6 +79,7 @@ jobs: with: report_paths: server/report.xml check_name: ${{ inputs.name }} (Results) + job_name: ${{ inputs.name }} require_tests: true - name: Report retried tests via webhook (master || release-*) if: ${{ steps.report.outputs.retried > 0 && (github.ref_name == 'master' || startsWith(github.ref_name, 'release-')) }} diff --git a/server/channels/store/sqlstore/store.go b/server/channels/store/sqlstore/store.go index b09690992a..5cefca3bbd 100644 --- a/server/channels/store/sqlstore/store.go +++ b/server/channels/store/sqlstore/store.go @@ -46,6 +46,7 @@ const ( PGDuplicateObjectErrorCode = "42710" MySQLDuplicateObjectErrorCode = 1022 DBPingAttempts = 5 + DBReplicaPingAttempts = 2 // This is a numerical version string by postgres. The format is // 2 characters for major, minor, and patch version prior to 10. // After 10, it's major and minor only. @@ -289,7 +290,7 @@ func (ss *SqlStore) initConnection() error { ss.ReplicaXs = make([]*atomic.Pointer[sqlxDBWrapper], len(ss.settings.DataSourceReplicas)) for i, replica := range ss.settings.DataSourceReplicas { ss.ReplicaXs[i] = &atomic.Pointer[sqlxDBWrapper]{} - handle, err = sqlUtils.SetupConnection(ss.Logger(), fmt.Sprintf("replica-%v", i), replica, ss.settings, DBPingAttempts) + handle, err = sqlUtils.SetupConnection(ss.Logger(), fmt.Sprintf("replica-%v", i), replica, ss.settings, DBReplicaPingAttempts) if err != nil { // Initializing to be offline ss.ReplicaXs[i].Store(&sqlxDBWrapper{isOnline: &atomic.Bool{}}) @@ -304,7 +305,7 @@ func (ss *SqlStore) initConnection() error { ss.searchReplicaXs = make([]*atomic.Pointer[sqlxDBWrapper], len(ss.settings.DataSourceSearchReplicas)) for i, replica := range ss.settings.DataSourceSearchReplicas { ss.searchReplicaXs[i] = &atomic.Pointer[sqlxDBWrapper]{} - handle, err = sqlUtils.SetupConnection(ss.Logger(), fmt.Sprintf("search-replica-%v", i), replica, ss.settings, DBPingAttempts) + handle, err = sqlUtils.SetupConnection(ss.Logger(), fmt.Sprintf("search-replica-%v", i), replica, ss.settings, DBReplicaPingAttempts) if err != nil { // Initializing to be offline ss.searchReplicaXs[i].Store(&sqlxDBWrapper{isOnline: &atomic.Bool{}}) @@ -321,7 +322,7 @@ func (ss *SqlStore) initConnection() error { if src.DataSource == nil { continue } - ss.replicaLagHandles[i], err = sqlUtils.SetupConnection(ss.Logger(), fmt.Sprintf(replicaLagPrefix+"-%d", i), *src.DataSource, ss.settings, DBPingAttempts) + ss.replicaLagHandles[i], 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 diff --git a/server/public/utils/sql/sql_utils.go b/server/public/utils/sql/sql_utils.go index a1029f314a..337804ad1d 100644 --- a/server/public/utils/sql/sql_utils.go +++ b/server/public/utils/sql/sql_utils.go @@ -17,7 +17,8 @@ import ( ) const ( - DBPingTimeoutSecs = 10 + DBPingTimeout = 10 * time.Second + DBConnRetrySleep = 2 * time.Second replicaLagPrefix = "replica-lag" ) @@ -65,15 +66,15 @@ func SetupConnection(logger mlog.LoggerIFace, connType string, dataSource string for i := 0; i < attempts; i++ { logger.Info("Pinging SQL") - ctx, cancel := context.WithTimeout(context.Background(), DBPingTimeoutSecs*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), DBPingTimeout) defer cancel() err = db.PingContext(ctx) if err != nil { if i == attempts-1 { return nil, err } - logger.Error("Failed to ping DB", mlog.Int("retrying in seconds", DBPingTimeoutSecs), mlog.Err(err)) - time.Sleep(DBPingTimeoutSecs * time.Second) + logger.Error("Failed to ping DB", mlog.Float("retrying in seconds", DBConnRetrySleep.Seconds()), mlog.Err(err)) + time.Sleep(DBConnRetrySleep) continue } break