Implement remaining review suggestions

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
vitessio · Dec 5, 2024 · 19d6e28 · 19d6e28
1 parent b006f4b
commit 19d6e28
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 134 deletions.
diff --git a/go/vt/mysqlctl/blackbox/backup_race_test.go b/go/vt/mysqlctl/blackbox/backup_race_test.go
@@ -75,7 +75,7 @@ func TestExecuteBackupWithFailureOnLastFile(t *testing.T) {
 	require.NoError(t, createBackupFiles(path.Join(dataDir, "test2"), 2, "ibd"))
 	defer os.RemoveAll(backupRoot)
 
-	needIt, err := needInnoDBRedoLogSubdir()
+	needIt, err := NeedInnoDBRedoLogSubdir()
 	require.NoError(t, err)
 	if needIt {
 		fpath := path.Join("log", mysql.DynamicRedoLogSubdir)

diff --git a/go/vt/mysqlctl/blackbox/backup_test.go b/go/vt/mysqlctl/blackbox/backup_test.go
@@ -65,7 +65,7 @@ func TestExecuteBackup(t *testing.T) {
 	require.NoError(t, createBackupFiles(path.Join(dataDir, "test2"), 2, "ibd"))
 	defer os.RemoveAll(backupRoot)
 
-	needIt, err := needInnoDBRedoLogSubdir()
+	needIt, err := NeedInnoDBRedoLogSubdir()
 	require.NoError(t, err)
 	if needIt {
 		fpath := path.Join("log", mysql.DynamicRedoLogSubdir)
@@ -211,7 +211,7 @@ func TestExecuteBackupWithSafeUpgrade(t *testing.T) {
 	require.NoError(t, createBackupFiles(path.Join(dataDir, "test2"), 2, "ibd"))
 	defer os.RemoveAll(backupRoot)
 
-	needIt, err := needInnoDBRedoLogSubdir()
+	needIt, err := NeedInnoDBRedoLogSubdir()
 	require.NoError(t, err)
 	if needIt {
 		fpath := path.Join("log", mysql.DynamicRedoLogSubdir)
@@ -304,7 +304,7 @@ func TestExecuteBackupWithCanceledContext(t *testing.T) {
 	require.NoError(t, createBackupFiles(path.Join(dataDir, "test2"), 2, "ibd"))
 	defer os.RemoveAll(backupRoot)
 
-	needIt, err := needInnoDBRedoLogSubdir()
+	needIt, err := NeedInnoDBRedoLogSubdir()
 	require.NoError(t, err)
 	if needIt {
 		fpath := path.Join("log", mysql.DynamicRedoLogSubdir)
@@ -393,7 +393,7 @@ func TestExecuteRestoreWithTimedOutContext(t *testing.T) {
 	require.NoError(t, createBackupFiles(path.Join(dataDir, "test2"), 2, "ibd"))
 	defer os.RemoveAll(backupRoot)
 
-	needIt, err := needInnoDBRedoLogSubdir()
+	needIt, err := NeedInnoDBRedoLogSubdir()
 	require.NoError(t, err)
 	if needIt {
 		fpath := path.Join("log", mysql.DynamicRedoLogSubdir)

diff --git a/go/vt/mysqlctl/blackbox/utils.go b/go/vt/mysqlctl/blackbox/utils.go
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"slices"
 	"strconv"
 	"testing"
 	"time"
@@ -33,6 +34,7 @@ import (
 	"vitess.io/vitess/go/vt/mysqlctl"
 	"vitess.io/vitess/go/vt/mysqlctl/backupstats"
 	"vitess.io/vitess/go/vt/mysqlctl/filebackupstorage"
+	logutilpb "vitess.io/vitess/go/vt/proto/logutil"
 	"vitess.io/vitess/go/vt/proto/topodata"
 	"vitess.io/vitess/go/vt/proto/vttime"
 	"vitess.io/vitess/go/vt/topo"
@@ -54,25 +56,17 @@ func GetStats(stats *backupstats.FakeStats) StatSummary {
 	for _, sr := range stats.ScopeReturns {
 		switch sr.ScopeV[backupstats.ScopeOperation] {
 		case "Destination:Close":
-			if len(sr.TimedIncrementCalls) > 0 {
-				ss.DestinationCloseStats++
-			}
+			ss.DestinationCloseStats += len(sr.TimedIncrementCalls)
 		case "Destination:Open":
-			if len(sr.TimedIncrementCalls) > 0 {
-				ss.DestinationOpenStats++
-			}
+			ss.DestinationOpenStats += len(sr.TimedIncrementCalls)
 		case "Destination:Write":
 			if len(sr.TimedIncrementBytesCalls) > 0 {
 				ss.DestinationWriteStats++
 			}
 		case "Source:Close":
-			if len(sr.TimedIncrementCalls) > 0 {
-				ss.SourceCloseStats++
-			}
+			ss.SourceCloseStats += len(sr.TimedIncrementCalls)
 		case "Source:Open":
-			if len(sr.TimedIncrementCalls) > 0 {
-				ss.SourceOpenStats++
-			}
+			ss.SourceOpenStats += len(sr.TimedIncrementCalls)
 		case "Source:Read":
 			if len(sr.TimedIncrementBytesCalls) > 0 {
 				ss.SourceReadStats++
@@ -84,16 +78,9 @@ func GetStats(stats *backupstats.FakeStats) StatSummary {
 
 func AssertLogs(t *testing.T, expectedLogs []string, logger *logutil.MemoryLogger) {
 	for _, log := range expectedLogs {
-		var found bool
-		for _, event := range logger.Events {
-			if log == event.GetValue() {
-				found = true
-				break
-			}
-		}
-		if !found {
-			require.Failf(t, "missing log line", "%s is missing from the logs", log)
-		}
+		require.Truef(t, slices.ContainsFunc(logger.Events, func(event *logutilpb.Event) bool {
+			return event.GetValue() == log
+		}), "%s is missing from the logs", log)
 	}
 }
 
@@ -115,7 +102,7 @@ func SetupCluster(ctx context.Context, t *testing.T, dirs, filesPerDir int) (bac
 		require.NoError(t, os.RemoveAll(backupRoot))
 	})
 
-	needIt, err := needInnoDBRedoLogSubdir()
+	needIt, err := NeedInnoDBRedoLogSubdir()
 	require.NoError(t, err)
 	if needIt {
 		fpath := path.Join("log", mysql.DynamicRedoLogSubdir)
@@ -152,12 +139,12 @@ func SetupCluster(ctx context.Context, t *testing.T, dirs, filesPerDir int) (bac
 	return backupRoot, keyspace, shard, ts
 }
 
-// needInnoDBRedoLogSubdir indicates whether we need to create a redo log subdirectory.
+// NeedInnoDBRedoLogSubdir indicates whether we need to create a redo log subdirectory.
 // Starting with MySQL 8.0.30, the InnoDB redo logs are stored in a subdirectory of the
 // <innodb_log_group_home_dir> (<datadir>/. by default) called "#innodb_redo". See:
 //
 //	https://dev.mysql.com/doc/refman/8.0/en/innodb-redo-log.html#innodb-modifying-redo-log-capacity
-func needInnoDBRedoLogSubdir() (needIt bool, err error) {
+func NeedInnoDBRedoLogSubdir() (needIt bool, err error) {
 	mysqldVersionStr, err := mysqlctl.GetVersionString()
 	if err != nil {
 		return needIt, err