PR feedback

Signed-off-by: Renan Rangel <[email protected]>

go/ioutil/writer.go

@@ -89,15 +89,15 @@ func (tw *meteredWriter) Write(p []byte) (int, error) {
 	return tw.meter.measure(tw.Writer.Write, p)
 }
 
-// MemoryBuffer implements io.WriteCloser using an in-memory buffer.
-type MemoryBuffer struct {
+// BytesBufferWriter implements io.WriteCloser using an in-memory buffer.
+type BytesBufferWriter struct {
 	*bytes.Buffer
 }
 
-func (m MemoryBuffer) Close() error {
+func (m BytesBufferWriter) Close() error {
 	return nil
 }
 
-func NewMemoryBuffer() MemoryBuffer {
-	return MemoryBuffer{bytes.NewBuffer(nil)}
+func NewBytesBufferWriter() BytesBufferWriter {
+	return BytesBufferWriter{bytes.NewBuffer(nil)}
 }

go/vt/mysqlctl/mysqlshellbackupengine.go

@@ -155,8 +155,9 @@ func (be *MySQLShellBackupEngine) ExecuteBackup(ctx context.Context, params Back
 	lockAcquired := time.Now() // we will report how long we hold the lock for
 
 	// we need to release the global read lock in case the backup fails to start and
-	// the lock wasn't released by releaseReadLock() yet
-	defer func() { _ = params.Mysqld.ReleaseGlobalReadLock(ctx) }()
+	// the lock wasn't released by releaseReadLock() yet. context might be expired,
+	// so we pass a new one.
+	defer func() { _ = params.Mysqld.ReleaseGlobalReadLock(context.Background()) }()
 
 	posBeforeBackup, err := params.Mysqld.PrimaryPosition(ctx)
 	if err != nil {

go/vt/mysqlctl/mysqlshellbackupengine_test.go

@@ -22,7 +22,6 @@ import (
 	"fmt"
 	"os"
 	"path"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -335,6 +334,7 @@ func TestMySQLShellBackupEngine_ExecuteBackup_ReleaseLock(t *testing.T) {
 		mysqlShellBackupBinaryName = originalBinary
 	}()
 
+	logger := logutil.NewMemoryLogger()
 	fakedb := fakesqldb.New(t)
 	defer fakedb.Close()
 	mysql := NewFakeMysqlDaemon(fakedb)
@@ -343,16 +343,16 @@ func TestMySQLShellBackupEngine_ExecuteBackup_ReleaseLock(t *testing.T) {
 	be := &MySQLShellBackupEngine{}
 	params := BackupParams{
 		TabletAlias: "test",
+		Logger:      logger,
 		Mysqld:      mysql,
 	}
 	bs := FakeBackupStorage{
 		StartBackupReturn: FakeBackupStorageStartBackupReturn{},
 	}
 
 	t.Run("lock released if we see the mysqlsh lock being acquired", func(t *testing.T) {
-		logger := logutil.NewMemoryLogger()
-		params.Logger = logger
-		manifestBuffer := ioutil.NewMemoryBuffer()
+		logger.Clear()
+		manifestBuffer := ioutil.NewBytesBufferWriter()
 		bs.StartBackupReturn.BackupHandle = &FakeBackupHandle{
 			Dir:           t.TempDir(),
 			AddFileReturn: FakeBackupHandleAddFileReturn{WriteCloser: manifestBuffer},
@@ -376,21 +376,14 @@ func TestMySQLShellBackupEngine_ExecuteBackup_ReleaseLock(t *testing.T) {
 		require.Equal(t, mysqlShellBackupEngineName, manifest.BackupMethod)
 
 		// did we notice the lock was release and did we release it ours as well?
-		errorLogged := false
-		for _, event := range logger.Events {
-			if strings.Contains(event.Value, "global read lock released after") {
-				errorLogged = true
-			}
-		}
-
-		assert.True(t, errorLogged, "failed to release the global lock after mysqlsh")
+		require.Contains(t, logger.String(), "global read lock released after",
+			"failed to release the global lock after mysqlsh")
 	})
 
 	t.Run("lock released if when we don't see mysqlsh released it", func(t *testing.T) {
 		mysql.GlobalReadLock = false // clear lock status.
-		logger := logutil.NewMemoryLogger()
-		params.Logger = logger
-		manifestBuffer := ioutil.NewMemoryBuffer()
+		logger.Clear()
+		manifestBuffer := ioutil.NewBytesBufferWriter()
 		bs.StartBackupReturn.BackupHandle = &FakeBackupHandle{
 			Dir:           t.TempDir(),
 			AddFileReturn: FakeBackupHandleAddFileReturn{WriteCloser: manifestBuffer},
@@ -409,21 +402,14 @@ func TestMySQLShellBackupEngine_ExecuteBackup_ReleaseLock(t *testing.T) {
 		require.False(t, mysql.GlobalReadLock) // lock must be released.
 
 		// make sure we are at least logging the lock wasn't able to be released earlier.
-		errorLogged := false
-		for _, event := range logger.Events {
-			if strings.Contains(event.Value, "could not release global lock earlier") {
-				errorLogged = true
-			}
-		}
-
-		assert.True(t, errorLogged, "failed to log error message when unable to release lock during backup")
+		require.Contains(t, logger.String(), "could not release global lock earlier",
+			"failed to log error message when unable to release lock during backup")
 	})
 
 	t.Run("lock released when backup fails", func(t *testing.T) {
 		mysql.GlobalReadLock = false // clear lock status.
-		logger := logutil.NewMemoryLogger()
-		params.Logger = logger
-		manifestBuffer := ioutil.NewMemoryBuffer()
+		logger.Clear()
+		manifestBuffer := ioutil.NewBytesBufferWriter()
 		bs.StartBackupReturn.BackupHandle = &FakeBackupHandle{
 			Dir:           t.TempDir(),
 			AddFileReturn: FakeBackupHandleAddFileReturn{WriteCloser: manifestBuffer},