upstream/icingadb
1
0
Fork 0
mirror of https://github.com/Icinga/icingadb.git synced 2026-05-28 04:35:54 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Don't retry individual SQL transaction commands

Browse source 
If anything in a transaction fails, the transaction should be retried as
a whole, just retrying some individual command makes no sense and breaks
transaction semantics. For example, if a COMMIT fails in MySQL, it
automatically rolls back the transaction and if you issue another
COMMIT, you just commit an empty transaction.

Remove the test case for commit as is basically tested that it showed
the broken behavior described above.
This commit is contained in:
Julian Brost 2020-12-08 18:10:38 +01:00
parent 9570975494
commit 05cc307112
2 changed files with 6 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
connection/mysql.go
View file

@ -300,12 +300,6 @@ func (dbw *DBWrapper) SqlCommit(tx DbTransaction, quiet bool) error {
}).Debug("COMMIT transaction")
}
if err != nil {
if dbw.isConnectionError(err) {
continue
}
}
return err
}
}
@ -335,12 +329,6 @@ func (dbw *DBWrapper) SqlRollback(tx DbTransaction, quiet bool) error {
err = tx.Rollback()
}
if err != nil {
if dbw.isConnectionError(err) {
continue
}
}
return err
}
}
@ -382,7 +370,7 @@ func (dbw *DBWrapper) SqlFetchAllTxQuiet(tx DbTransaction, queryObserver prometh
}
// sqlExecInternal is a wrapper around sql.Exec() for auto-logging.
func (dbw *DBWrapper) sqlExecInternal(db DbClientOrTransaction, opObserver prometheus.Observer, sql string, quiet bool, args ...interface{}) (sql.Result, error) {
func (dbw *DBWrapper) sqlExecInternal(db DbClientOrTransaction, opObserver prometheus.Observer, query string, quiet bool, args ...interface{}) (sql.Result, error) {
for {
if !dbw.IsConnected() {
dbw.WaitForConnection()
@ -394,7 +382,7 @@ func (dbw *DBWrapper) sqlExecInternal(db DbClientOrTransaction, opObserver prome
benchmarc = utils.NewBenchmark()
}
res, err := db.Exec(sql, args...)
res, err := db.Exec(query, args...)
DbOperationsExec.Inc()
if !quiet {
@ -408,12 +396,12 @@ func (dbw *DBWrapper) sqlExecInternal(db DbClientOrTransaction, opObserver prome
"benchmark": benchmarc,
"affected_rows": prettyPrintedRowsAffected{res},
"args": prettyPrintedArgs{args},
"query": prettyPrintedSql{sql},
"query": prettyPrintedSql{query},
}).Debug("Finished Exec")
}
if err != nil {
if dbw.isConnectionError(err) {
if _, isTx := db.(DbTransaction); !isTx && dbw.isConnectionError(err) {
continue
}
}
@ -433,10 +421,8 @@ func (dbw *DBWrapper) sqlFetchAllInternal(db DbClientOrTransaction, queryObserve
res, err := sqlTryFetchAll(db, queryObserver, query, quiet, args...)
if err != nil {
if _, isDb := db.(*sql.DB); isDb {
if dbw.isConnectionError(err) {
continue
}
if _, isTx := db.(DbTransaction); !isTx && dbw.isConnectionError(err) {
continue
}
}

30
connection/mysql_test.go
View file

@ -107,36 +107,6 @@ func TestDBWrapper_CheckConnection(t *testing.T) {
assert.Equal(t, uint32(11), atomic.LoadUint32(dbw.ConnectionLostCounterAtomic))
}
func TestDBWrapper_SqlCommit(t *testing.T) {
mockDb := new(DbMock)
dbw := NewTestDBW(mockDb)
mockTx := new(TransactionMock)
mockTx.On("Commit").Return(errors.New("whoops")).Once()
mockTx.On("Commit").Return(nil).Once()
mockDb.On("Ping").Return(errors.New("whoops")).Once()
var err error
done := make(chan bool)
dbw.CompareAndSetConnected(true)
go func() {
err = dbw.SqlCommit(mockTx, false)
done <- true
}()
time.Sleep(time.Millisecond * 50)
dbw.CompareAndSetConnected(true)
dbw.ConnectionUpCondition.Broadcast()
<-done
assert.NoError(t, err)
mockTx.AssertExpectations(t)
mockDb.AssertExpectations(t)
}
func TestDBWrapper_SqlBegin(t *testing.T) {
mockDb := new(DbMock)
dbw := NewTestDBW(mockDb)