Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

Test and correct semantics for MySQL withRetryTxx (#488)

Browse Source 
- Differentiate retryable vs. non-retryable errors.
- Test functionality against mocked DB.
This commit is contained in:
Zach Wasserman
2021-03-17 11:23:52 -07:00
committed by GitHub
parent 1da89ea987
commit db3c73d619
4 changed files with 163 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
go.mod
+1
View File
@@ -5,6 +5,7 @@ go 1.12
require (
cloud.google.com/go v0.37.4
github.com/AbGuthrie/goquery/v2 v2.0.1
github.com/DATA-DOG/go-sqlmock v1.5.0
github.com/VividCortex/gohistogram v1.0.0 // indirect
github.com/VividCortex/mysqlerr v0.0.0-20170204212430-6c6b55f8796f
github.com/WatchBeam/clock v0.0.0-20170901150240-b08e6b4da7ea
go.sum
+2
View File
@@ -6,6 +6,8 @@ github.com/AbGuthrie/goquery/v2 v2.0.1 h1:h0tIhmeRroyqYjT9zxXPXOrheNp1xqNTV+XFWu
github.com/AbGuthrie/goquery/v2 v2.0.1/go.mod h1:xpDLF4kUr+TRFXogclRa7Zzc8bMAB/fYm1zG/XX1WOA=
github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/DATA-DOG/go-sqlmock v1.5.0 h1:Shsta01QNfFxHCfpW6YH2STWB0MudeXXEWMr20OEh60=
github.com/DATA-DOG/go-sqlmock v1.5.0/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM=
github.com/Microsoft/go-winio v0.4.9/go.mod h1:VhR8bwka0BXejwEJY73c50VrPtXAaKcyvVC4A4RozmA=
github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo=
github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI=
server/datastore/mysql/mysql.go
+37 -9
View File
@@ -11,6 +11,7 @@ import (
"strings"
"time"
"github.com/VividCortex/mysqlerr"
"github.com/WatchBeam/clock"
"github.com/cenkalti/backoff/v4"
"github.com/fleetdm/fleet/server/config"
@@ -61,12 +62,28 @@ func (d *Datastore) getTransaction(opts []kolide.OptionalArg) dbfunctions {
type txFn func(*sqlx.Tx) error
// retryableError determines whether a MySQL error can be retried. By default
// errors are considered non-retryable. Only errors that we know have a
// possibility of succeeding on a retry should return true in this function.
func retryableError(err error) bool {
base := errors.Cause(err)
if b, ok := base.(*mysql.MySQLError); ok {
switch b.Number {
// Consider lock related errors to be retryable
case mysqlerr.ER_LOCK_DEADLOCK, mysqlerr.ER_LOCK_WAIT_TIMEOUT:
return true
}
}
return false
}
// withRetryTxx provides a common way to commit/rollback a txFn wrapped in a retry with exponential backoff
func (d *Datastore) withRetryTxx(fn txFn) (err error) {
operation := func() error {
tx, err := d.db.Beginx()
if err != nil {
return errors.Wrap(err, "creating transaction")
return errors.Wrap(err, "create transaction")
}
defer func() {
@@ -78,18 +95,29 @@ func (d *Datastore) withRetryTxx(fn txFn) (err error) {
}
}()
err = fn(tx)
if err != nil {
if err := fn(tx); err != nil {
rbErr := tx.Rollback()
if rbErr != nil && rbErr != sql.ErrTxDone {
return fmt.Errorf("got err '%s' rolling back after err '%s'", rbErr, err)
// Consider rollback errors to be non-retryable
return backoff.Permanent(errors.Wrapf(err, "got err '%s' rolling back after err", rbErr.Error()))
}
return err
} else {
err = tx.Commit()
if err != nil {
return errors.Wrap(err, "committing transaction")
if retryableError(err) {
return err
}
// Consider any other errors to be non-retryable
return backoff.Permanent(err)
}
if err := tx.Commit(); err != nil {
err = errors.Wrap(err, "commit transaction")
if retryableError(err) {
return err
}
return backoff.Permanent(errors.Wrap(err, "commit transaction"))
}
return nil
server/datastore/mysql/mysql_test.go
+123
View File
@@ -3,7 +3,14 @@ package mysql
import (
"testing"
"github.com/DATA-DOG/go-sqlmock"
"github.com/VividCortex/mysqlerr"
"github.com/go-kit/kit/log"
"github.com/go-sql-driver/mysql"
"github.com/jmoiron/sqlx"
"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestSanitizeColumn(t *testing.T) {
@@ -108,3 +115,119 @@ func TestSearchLike(t *testing.T) {
})
}
}
func mockDatastore(t *testing.T) (sqlmock.Sqlmock, *Datastore) {
db, mock, err := sqlmock.New()
require.NoError(t, err)
ds := &Datastore{
db: sqlx.NewDb(db, "sqlmock"),
logger: log.NewNopLogger(),
}
return mock, ds
}
func TestWithRetryTxxSuccess(t *testing.T) {
mock, ds := mockDatastore(t)
defer ds.Close()
mock.ExpectBegin()
mock.ExpectExec("SELECT 1").WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectCommit()
require.NoError(t, ds.withRetryTxx(func(tx *sqlx.Tx) error {
_, err := tx.Exec("SELECT 1")
return err
}))
require.NoError(t, mock.ExpectationsWereMet())
}
func TestWithRetryTxxRollbackSuccess(t *testing.T) {
mock, ds := mockDatastore(t)
defer ds.Close()
mock.ExpectBegin()
mock.ExpectExec("SELECT 1").WillReturnError(errors.New("fail"))
mock.ExpectRollback()
require.Error(t, ds.withRetryTxx(func(tx *sqlx.Tx) error {
_, err := tx.Exec("SELECT 1")
return err
}))
require.NoError(t, mock.ExpectationsWereMet())
}
func TestWithRetryTxxRollbackError(t *testing.T) {
mock, ds := mockDatastore(t)
defer ds.Close()
mock.ExpectBegin()
mock.ExpectExec("SELECT 1").WillReturnError(errors.New("fail"))
mock.ExpectRollback().WillReturnError(errors.New("rollback failed"))
require.Error(t, ds.withRetryTxx(func(tx *sqlx.Tx) error {
_, err := tx.Exec("SELECT 1")
return err
}))
require.NoError(t, mock.ExpectationsWereMet())
}
func TestWithRetryTxxRetrySuccess(t *testing.T) {
mock, ds := mockDatastore(t)
defer ds.Close()
mock.ExpectBegin()
// Return a retryable error
mock.ExpectExec("SELECT 1").WillReturnError(&mysql.MySQLError{Number: mysqlerr.ER_LOCK_DEADLOCK})
mock.ExpectRollback()
mock.ExpectBegin()
mock.ExpectExec("SELECT 1").WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectCommit()
assert.NoError(t, ds.withRetryTxx(func(tx *sqlx.Tx) error {
_, err := tx.Exec("SELECT 1")
return err
}))
require.NoError(t, mock.ExpectationsWereMet())
}
func TestWithRetryTxxCommitRetrySuccess(t *testing.T) {
mock, ds := mockDatastore(t)
defer ds.Close()
mock.ExpectBegin()
mock.ExpectExec("SELECT 1").WillReturnResult(sqlmock.NewResult(1, 1))
// Return a retryable error
mock.ExpectCommit().WillReturnError(&mysql.MySQLError{Number: mysqlerr.ER_LOCK_DEADLOCK})
mock.ExpectBegin()
mock.ExpectExec("SELECT 1").WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectCommit()
assert.NoError(t, ds.withRetryTxx(func(tx *sqlx.Tx) error {
_, err := tx.Exec("SELECT 1")
return err
}))
require.NoError(t, mock.ExpectationsWereMet())
}
func TestWithRetryTxxCommitError(t *testing.T) {
mock, ds := mockDatastore(t)
defer ds.Close()
mock.ExpectBegin()
mock.ExpectExec("SELECT 1").WillReturnResult(sqlmock.NewResult(1, 1))
// Return a retryable error
mock.ExpectCommit().WillReturnError(errors.New("fail"))
assert.Error(t, ds.withRetryTxx(func(tx *sqlx.Tx) error {
_, err := tx.Exec("SELECT 1")
return err
}))
require.NoError(t, mock.ExpectationsWereMet())
}