From 5eae7d3eeba7952c39bf95e637ad3cbd44219514 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Mon, 8 Jul 2024 07:30:59 -0600 Subject: [PATCH 1/2] dbconn: improve MDL lock output --- pkg/dbconn/metadatalock.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pkg/dbconn/metadatalock.go b/pkg/dbconn/metadatalock.go index 933348f..69a710c 100644 --- a/pkg/dbconn/metadatalock.go +++ b/pkg/dbconn/metadatalock.go @@ -6,6 +6,7 @@ import ( "fmt" "time" + "github.com/cashapp/spirit/pkg/dbconn/sqlescape" "github.com/siddontang/loggers" ) @@ -51,7 +52,8 @@ func NewMetadataLock(ctx context.Context, dsn string, lockName string, logger lo getLock := func() error { // https://dev.mysql.com/doc/refman/8.0/en/locking-functions.html#function_get-lock var answer int - if err := dbConn.QueryRowContext(ctx, "SELECT GET_LOCK(?, ?)", lockName, getLockTimeout.Seconds()).Scan(&answer); err != nil { + stmt := sqlescape.MustEscapeSQL("SELECT GET_LOCK(%?, %?)", lockName, getLockTimeout.Seconds()) + if err := dbConn.QueryRowContext(ctx, stmt).Scan(&answer); err != nil { return fmt.Errorf("could not acquire metadata lock: %s", err) } if answer == 0 { @@ -66,7 +68,8 @@ func NewMetadataLock(ctx context.Context, dsn string, lockName string, logger lo } // Acquire the lock or return an error immediately - logger.Infof("attempting to acquire metadata lock: %s", lockName) + // We only Infof the initial acquisition. + logger.Infof("attempting to acquiring metadata lock: %s", lockName) if err = getLock(); err != nil { return nil, err } @@ -87,9 +90,15 @@ func NewMetadataLock(ctx context.Context, dsn string, lockName string, logger lo return case <-ticker.C: if err = getLock(); err != nil { - logger.Errorf("could not refresh metadata lock: %s", err) + // if we can't refresh the lock, it's okay. + // We have other safety mechanisms in place to prevent corruption + // for example, we watch the binary log to see metadata changes + // that we did not make. This makes it a warning, not an error, + // and we can try again on the next tick interval. + logger.Warnf("could not refresh metadata lock: %s", err) + } else { + logger.Debugf("refreshed metadata lock: %s", lockName) } - logger.Infof("refreshed metadata lock: %s", lockName) } } }() From 87d898f7411188bec03fca7071d4b8463392464c Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Mon, 8 Jul 2024 07:47:53 -0600 Subject: [PATCH 2/2] revert error message change --- pkg/dbconn/metadatalock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/dbconn/metadatalock.go b/pkg/dbconn/metadatalock.go index 69a710c..231bb9f 100644 --- a/pkg/dbconn/metadatalock.go +++ b/pkg/dbconn/metadatalock.go @@ -69,7 +69,7 @@ func NewMetadataLock(ctx context.Context, dsn string, lockName string, logger lo // Acquire the lock or return an error immediately // We only Infof the initial acquisition. - logger.Infof("attempting to acquiring metadata lock: %s", lockName) + logger.Infof("attempting to acquire metadata lock: %s", lockName) if err = getLock(); err != nil { return nil, err }