From 2a74952cf8e2cd4bd51755f0b3396120d1451dcb Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sun, 29 May 2022 02:13:46 +0200 Subject: [PATCH 1/3] Add `golangci-lint`, fix `gosimple`, `govet` and `unused` linter complaints --- .github/workflows/golangci-lint.yml | 21 +++++++++++ .golangci.yml | 12 ++++++ go/base/context_test.go | 5 +-- go/base/default_logger.go | 6 --- go/base/utils.go | 4 +- go/binlog/gomysql_reader.go | 5 ++- go/logic/hooks.go | 4 +- go/logic/inspect.go | 1 - go/logic/migrator.go | 58 ++++++++++++----------------- go/logic/streamer.go | 1 - go/mysql/instance_key.go | 7 +++- go/sql/builder.go | 16 ++++---- 12 files changed, 77 insertions(+), 63 deletions(-) create mode 100644 .github/workflows/golangci-lint.yml create mode 100644 .golangci.yml diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 000000000..caa1f5714 --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,21 @@ +name: golangci-lint +on: + push: + branches: + - master + pull_request: +permissions: + contents: read + # Optional: allow read access to pull request. Use with `only-new-issues` option. + # pull-requests: read +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/setup-go@v3 + with: + go-version: 1.17 + - uses: actions/checkout@v3 + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..4a487bd97 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,12 @@ +run: + timeout: 5m + modules-download-mode: readonly + +linters: + disable: + - errcheck + - staticcheck + enable: + - gosimple + - govet + - unused diff --git a/go/base/context_test.go b/go/base/context_test.go index 07e00ce9f..8b0c2badb 100644 --- a/go/base/context_test.go +++ b/go/base/context_test.go @@ -6,7 +6,6 @@ package base import ( - "fmt" "io/ioutil" "os" "testing" @@ -89,7 +88,7 @@ func TestReadConfigFile(t *testing.T) { } defer os.Remove(f.Name()) - f.Write([]byte(fmt.Sprintf("[client]\nuser=test\npassword=123456"))) + f.Write([]byte("[client]\nuser=test\npassword=123456")) context := NewMigrationContext() context.ConfigFile = f.Name() if err := context.ReadConfigFile(); err != nil { @@ -109,7 +108,7 @@ func TestReadConfigFile(t *testing.T) { } defer os.Remove(f.Name()) - f.Write([]byte(fmt.Sprintf("[osc]\nmax_load=10"))) + f.Write([]byte("[osc]\nmax_load=10")) context := NewMigrationContext() context.ConfigFile = f.Name() if err := context.ReadConfigFile(); err != nil { diff --git a/go/base/default_logger.go b/go/base/default_logger.go index 0bea41924..7c7011aae 100644 --- a/go/base/default_logger.go +++ b/go/base/default_logger.go @@ -12,22 +12,18 @@ func NewDefaultLogger() *simpleLogger { func (*simpleLogger) Debug(args ...interface{}) { log.Debug(args[0].(string), args[1:]) - return } func (*simpleLogger) Debugf(format string, args ...interface{}) { log.Debugf(format, args...) - return } func (*simpleLogger) Info(args ...interface{}) { log.Info(args[0].(string), args[1:]) - return } func (*simpleLogger) Infof(format string, args ...interface{}) { log.Infof(format, args...) - return } func (*simpleLogger) Warning(args ...interface{}) error { @@ -64,10 +60,8 @@ func (*simpleLogger) Fatale(err error) error { func (*simpleLogger) SetLevel(level log.LogLevel) { log.SetLevel(level) - return } func (*simpleLogger) SetPrintStackTrace(printStackTraceFlag bool) { log.SetPrintStackTrace(printStackTraceFlag) - return } diff --git a/go/base/utils.go b/go/base/utils.go index ed1451402..0fd581b3e 100644 --- a/go/base/utils.go +++ b/go/base/utils.go @@ -25,9 +25,7 @@ func PrettifyDurationOutput(d time.Duration) string { if d < time.Second { return "0s" } - result := fmt.Sprintf("%s", d) - result = prettifyDurationRegexp.ReplaceAllString(result, "") - return result + return prettifyDurationRegexp.ReplaceAllString(d.String(), "") } func FileExists(fileName string) bool { diff --git a/go/binlog/gomysql_reader.go b/go/binlog/gomysql_reader.go index 454b27abc..892b9fe67 100644 --- a/go/binlog/gomysql_reader.go +++ b/go/binlog/gomysql_reader.go @@ -64,7 +64,10 @@ func (this *GoMySQLReader) ConnectBinlogStreamer(coordinates mysql.BinlogCoordin this.currentCoordinates = coordinates this.migrationContext.Log.Infof("Connecting binlog streamer at %+v", this.currentCoordinates) // Start sync with specified binlog file and position - this.binlogStreamer, err = this.binlogSyncer.StartSync(gomysql.Position{this.currentCoordinates.LogFile, uint32(this.currentCoordinates.LogPos)}) + this.binlogStreamer, err = this.binlogSyncer.StartSync(gomysql.Position{ + Name: this.currentCoordinates.LogFile, + Pos: uint32(this.currentCoordinates.LogPos), + }) return err } diff --git a/go/logic/hooks.go b/go/logic/hooks.go index 00672bab4..03fec9583 100644 --- a/go/logic/hooks.go +++ b/go/logic/hooks.go @@ -72,9 +72,7 @@ func (this *HooksExecutor) applyEnvironmentVariables(extraVariables ...string) [ env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT_TOKEN=%s", this.migrationContext.HooksHintToken)) env = append(env, fmt.Sprintf("GH_OST_DRY_RUN=%t", this.migrationContext.Noop)) - for _, variable := range extraVariables { - env = append(env, variable) - } + env = append(env, extraVariables...) return env } diff --git a/go/logic/inspect.go b/go/logic/inspect.go index 9f82d7490..e66d673c4 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -804,5 +804,4 @@ func (this *Inspector) getReplicationLag() (replicationLag time.Duration, err er func (this *Inspector) Teardown() { this.db.Close() this.informationSchemaDb.Close() - return } diff --git a/go/logic/migrator.go b/go/logic/migrator.go index ceb6e6704..f539d1322 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -176,16 +176,6 @@ func (this *Migrator) retryOperationWithExponentialBackoff(operation func() erro return err } -// executeAndThrottleOnError executes a given function. If it errors, it -// throttles. -func (this *Migrator) executeAndThrottleOnError(operation func() error) (err error) { - if err := operation(); err != nil { - this.throttler.throttle(nil) - return err - } - return nil -} - // consumeRowCopyComplete blocks on the rowCopyComplete channel once, and then // consumes and drops any further incoming events that may be left hanging. func (this *Migrator) consumeRowCopyComplete() { @@ -823,57 +813,57 @@ func (this *Migrator) initiateStatus() error { // migration, and as response to the "status" interactive command. func (this *Migrator) printMigrationStatusHint(writers ...io.Writer) { w := io.MultiWriter(writers...) - fmt.Fprintln(w, fmt.Sprintf("# Migrating %s.%s; Ghost table is %s.%s", + fmt.Fprintf(w, "# Migrating %s.%s; Ghost table is %s.%s\n", sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName), sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.GetGhostTableName()), - )) - fmt.Fprintln(w, fmt.Sprintf("# Migrating %+v; inspecting %+v; executing on %+v", + ) + fmt.Fprintf(w, "# Migrating %+v; inspecting %+v; executing on %+v\n", *this.applier.connectionConfig.ImpliedKey, *this.inspector.connectionConfig.ImpliedKey, this.migrationContext.Hostname, - )) - fmt.Fprintln(w, fmt.Sprintf("# Migration started at %+v", + ) + fmt.Fprintf(w, "# Migration started at %+v\n", this.migrationContext.StartTime.Format(time.RubyDate), - )) + ) maxLoad := this.migrationContext.GetMaxLoad() criticalLoad := this.migrationContext.GetCriticalLoad() - fmt.Fprintln(w, fmt.Sprintf("# chunk-size: %+v; max-lag-millis: %+vms; dml-batch-size: %+v; max-load: %s; critical-load: %s; nice-ratio: %f", + fmt.Fprintf(w, "# chunk-size: %+v; max-lag-millis: %+vms; dml-batch-size: %+v; max-load: %s; critical-load: %s; nice-ratio: %f\n", atomic.LoadInt64(&this.migrationContext.ChunkSize), atomic.LoadInt64(&this.migrationContext.MaxLagMillisecondsThrottleThreshold), atomic.LoadInt64(&this.migrationContext.DMLBatchSize), maxLoad.String(), criticalLoad.String(), this.migrationContext.GetNiceRatio(), - )) + ) if this.migrationContext.ThrottleFlagFile != "" { setIndicator := "" if base.FileExists(this.migrationContext.ThrottleFlagFile) { setIndicator = "[set]" } - fmt.Fprintln(w, fmt.Sprintf("# throttle-flag-file: %+v %+v", + fmt.Fprintf(w, "# throttle-flag-file: %+v %+v\n", this.migrationContext.ThrottleFlagFile, setIndicator, - )) + ) } if this.migrationContext.ThrottleAdditionalFlagFile != "" { setIndicator := "" if base.FileExists(this.migrationContext.ThrottleAdditionalFlagFile) { setIndicator = "[set]" } - fmt.Fprintln(w, fmt.Sprintf("# throttle-additional-flag-file: %+v %+v", + fmt.Fprintf(w, "# throttle-additional-flag-file: %+v %+v\n", this.migrationContext.ThrottleAdditionalFlagFile, setIndicator, - )) + ) } if throttleQuery := this.migrationContext.GetThrottleQuery(); throttleQuery != "" { - fmt.Fprintln(w, fmt.Sprintf("# throttle-query: %+v", + fmt.Fprintf(w, "# throttle-query: %+v\n", throttleQuery, - )) + ) } if throttleControlReplicaKeys := this.migrationContext.GetThrottleControlReplicaKeys(); throttleControlReplicaKeys.Len() > 0 { - fmt.Fprintln(w, fmt.Sprintf("# throttle-control-replicas count: %+v", + fmt.Fprintf(w, "# throttle-control-replicas count: %+v\n", throttleControlReplicaKeys.Len(), - )) + ) } if this.migrationContext.PostponeCutOverFlagFile != "" { @@ -881,20 +871,20 @@ func (this *Migrator) printMigrationStatusHint(writers ...io.Writer) { if base.FileExists(this.migrationContext.PostponeCutOverFlagFile) { setIndicator = "[set]" } - fmt.Fprintln(w, fmt.Sprintf("# postpone-cut-over-flag-file: %+v %+v", + fmt.Fprintf(w, "# postpone-cut-over-flag-file: %+v %+v\n", this.migrationContext.PostponeCutOverFlagFile, setIndicator, - )) + ) } if this.migrationContext.PanicFlagFile != "" { - fmt.Fprintln(w, fmt.Sprintf("# panic-flag-file: %+v", + fmt.Fprintf(w, "# panic-flag-file: %+v\n", this.migrationContext.PanicFlagFile, - )) + ) } - fmt.Fprintln(w, fmt.Sprintf("# Serving on unix socket: %+v", + fmt.Fprintf(w, "# Serving on unix socket: %+v\n", this.migrationContext.ServeSocketFile, - )) + ) if this.migrationContext.ServeTCPPort != 0 { - fmt.Fprintln(w, fmt.Sprintf("# Serving on TCP port: %+v", this.migrationContext.ServeTCPPort)) + fmt.Fprintf(w, "# Serving on TCP port: %+v\n", this.migrationContext.ServeTCPPort) } } @@ -1195,7 +1185,6 @@ func (this *Migrator) iterateChunks() error { // Enqueue copy operation; to be executed by executeWriteFuncs() this.copyRowsQueue <- copyRowsFunc } - return nil } func (this *Migrator) onApplyEventStruct(eventStruct *applyEventStruct) error { @@ -1301,7 +1290,6 @@ func (this *Migrator) executeWriteFuncs() error { } } } - return nil } // finalCleanup takes actions at very end of migration, dropping tables etc. diff --git a/go/logic/streamer.go b/go/logic/streamer.go index 22158b97d..673d0d647 100644 --- a/go/logic/streamer.go +++ b/go/logic/streamer.go @@ -220,5 +220,4 @@ func (this *EventsStreamer) Close() (err error) { func (this *EventsStreamer) Teardown() { this.db.Close() - return } diff --git a/go/mysql/instance_key.go b/go/mysql/instance_key.go index eb108d84a..26034bd07 100644 --- a/go/mysql/instance_key.go +++ b/go/mysql/instance_key.go @@ -19,8 +19,11 @@ const ( var ( ipv4HostPortRegexp = regexp.MustCompile("^([^:]+):([0-9]+)$") ipv4HostRegexp = regexp.MustCompile("^([^:]+)$") - ipv6HostPortRegexp = regexp.MustCompile("^\\[([:0-9a-fA-F]+)\\]:([0-9]+)$") // e.g. [2001:db8:1f70::999:de8:7648:6e8]:3308 - ipv6HostRegexp = regexp.MustCompile("^([:0-9a-fA-F]+)$") // e.g. 2001:db8:1f70::999:de8:7648:6e8 + + // e.g. [2001:db8:1f70::999:de8:7648:6e8]:3308 + ipv6HostPortRegexp = regexp.MustCompile("^\\[([:0-9a-fA-F]+)\\]:([0-9]+)$") //nolint:gosimple + // e.g. 2001:db8:1f70::999:de8:7648:6e8 + ipv6HostRegexp = regexp.MustCompile("^([:0-9a-fA-F]+)$") ) // InstanceKey is an instance indicator, identified by hostname and port diff --git a/go/sql/builder.go b/go/sql/builder.go index 7fe366c6f..d373e5af8 100644 --- a/go/sql/builder.go +++ b/go/sql/builder.go @@ -33,7 +33,7 @@ func EscapeName(name string) string { } func buildColumnsPreparedValues(columns *ColumnList) []string { - values := make([]string, columns.Len(), columns.Len()) + values := make([]string, columns.Len()) for i, column := range columns.Columns() { var token string if column.timezoneConversion != nil { @@ -51,7 +51,7 @@ func buildColumnsPreparedValues(columns *ColumnList) []string { } func buildPreparedValues(length int) []string { - values := make([]string, length, length) + values := make([]string, length) for i := 0; i < length; i++ { values[i] = "?" } @@ -59,7 +59,7 @@ func buildPreparedValues(length int) []string { } func duplicateNames(names []string) []string { - duplicate := make([]string, len(names), len(names)) + duplicate := make([]string, len(names)) copy(duplicate, names) return duplicate } @@ -261,8 +261,8 @@ func BuildUniqueKeyRangeEndPreparedQueryViaOffset(databaseName, tableName string explodedArgs = append(explodedArgs, rangeExplodedArgs...) uniqueKeyColumnNames := duplicateNames(uniqueKeyColumns.Names()) - uniqueKeyColumnAscending := make([]string, len(uniqueKeyColumnNames), len(uniqueKeyColumnNames)) - uniqueKeyColumnDescending := make([]string, len(uniqueKeyColumnNames), len(uniqueKeyColumnNames)) + uniqueKeyColumnAscending := make([]string, len(uniqueKeyColumnNames)) + uniqueKeyColumnDescending := make([]string, len(uniqueKeyColumnNames)) for i, column := range uniqueKeyColumns.Columns() { uniqueKeyColumnNames[i] = EscapeName(uniqueKeyColumnNames[i]) if column.Type == EnumColumnType { @@ -316,8 +316,8 @@ func BuildUniqueKeyRangeEndPreparedQueryViaTemptable(databaseName, tableName str explodedArgs = append(explodedArgs, rangeExplodedArgs...) uniqueKeyColumnNames := duplicateNames(uniqueKeyColumns.Names()) - uniqueKeyColumnAscending := make([]string, len(uniqueKeyColumnNames), len(uniqueKeyColumnNames)) - uniqueKeyColumnDescending := make([]string, len(uniqueKeyColumnNames), len(uniqueKeyColumnNames)) + uniqueKeyColumnAscending := make([]string, len(uniqueKeyColumnNames)) + uniqueKeyColumnDescending := make([]string, len(uniqueKeyColumnNames)) for i, column := range uniqueKeyColumns.Columns() { uniqueKeyColumnNames[i] = EscapeName(uniqueKeyColumnNames[i]) if column.Type == EnumColumnType { @@ -368,7 +368,7 @@ func buildUniqueKeyMinMaxValuesPreparedQuery(databaseName, tableName string, uni tableName = EscapeName(tableName) uniqueKeyColumnNames := duplicateNames(uniqueKeyColumns.Names()) - uniqueKeyColumnOrder := make([]string, len(uniqueKeyColumnNames), len(uniqueKeyColumnNames)) + uniqueKeyColumnOrder := make([]string, len(uniqueKeyColumnNames)) for i, column := range uniqueKeyColumns.Columns() { uniqueKeyColumnNames[i] = EscapeName(uniqueKeyColumnNames[i]) if column.Type == EnumColumnType { From d1f730e81a0d69a1d86df3021d76f03341d57b83 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sun, 29 May 2022 05:07:23 +0200 Subject: [PATCH 2/3] Go 1.16 --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index caa1f5714..685afbeda 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -15,7 +15,7 @@ jobs: steps: - uses: actions/setup-go@v3 with: - go-version: 1.17 + go-version: 1.16 - uses: actions/checkout@v3 - name: golangci-lint uses: golangci/golangci-lint-action@v3 From c8f626558c647b59ded433242970b4701e5f2d85 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Tue, 31 May 2022 00:39:57 +0200 Subject: [PATCH 3/3] Update copyright dates --- go/base/context.go | 4 ++-- go/base/context_test.go | 2 +- go/base/default_logger.go | 5 +++++ go/base/utils.go | 2 +- go/binlog/gomysql_reader.go | 2 +- go/logic/hooks.go | 3 +-- go/logic/migrator.go | 2 +- go/logic/streamer.go | 2 +- go/logic/throttler.go | 2 +- go/mysql/instance_key.go | 5 ++--- go/sql/builder.go | 2 +- 11 files changed, 17 insertions(+), 14 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 4378e6b6a..93f84ce65 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -1,5 +1,5 @@ /* - Copyright 2016 GitHub Inc. + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */ @@ -15,7 +15,7 @@ import ( "sync/atomic" "time" - "github.com/satori/go.uuid" + uuid "github.com/satori/go.uuid" "github.com/github/gh-ost/go/mysql" "github.com/github/gh-ost/go/sql" diff --git a/go/base/context_test.go b/go/base/context_test.go index 8b0c2badb..de208bae4 100644 --- a/go/base/context_test.go +++ b/go/base/context_test.go @@ -1,5 +1,5 @@ /* - Copyright 2021 GitHub Inc. + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */ diff --git a/go/base/default_logger.go b/go/base/default_logger.go index 7c7011aae..59563ff4c 100644 --- a/go/base/default_logger.go +++ b/go/base/default_logger.go @@ -1,3 +1,8 @@ +/* + Copyright 2022 GitHub Inc. + See https://github.com/github/gh-ost/blob/master/LICENSE +*/ + package base import ( diff --git a/go/base/utils.go b/go/base/utils.go index 0fd581b3e..c0e32933a 100644 --- a/go/base/utils.go +++ b/go/base/utils.go @@ -1,5 +1,5 @@ /* - Copyright 2016 GitHub Inc. + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */ diff --git a/go/binlog/gomysql_reader.go b/go/binlog/gomysql_reader.go index 892b9fe67..1e936d251 100644 --- a/go/binlog/gomysql_reader.go +++ b/go/binlog/gomysql_reader.go @@ -1,5 +1,5 @@ /* - Copyright 2021 GitHub Inc. + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */ diff --git a/go/logic/hooks.go b/go/logic/hooks.go index 03fec9583..0ff296d82 100644 --- a/go/logic/hooks.go +++ b/go/logic/hooks.go @@ -1,6 +1,5 @@ /* -/* - Copyright 2016 GitHub Inc. + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */ diff --git a/go/logic/migrator.go b/go/logic/migrator.go index f539d1322..c5cb24432 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -1,5 +1,5 @@ /* - Copyright 2016 GitHub Inc. + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */ diff --git a/go/logic/streamer.go b/go/logic/streamer.go index 673d0d647..604f28910 100644 --- a/go/logic/streamer.go +++ b/go/logic/streamer.go @@ -1,5 +1,5 @@ /* - Copyright 2016 GitHub Inc. + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */ diff --git a/go/logic/throttler.go b/go/logic/throttler.go index abe8669e1..9c120b3e5 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -1,5 +1,5 @@ /* - Copyright 2016 GitHub Inc. + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */ diff --git a/go/mysql/instance_key.go b/go/mysql/instance_key.go index 26034bd07..679bdc9f0 100644 --- a/go/mysql/instance_key.go +++ b/go/mysql/instance_key.go @@ -1,5 +1,6 @@ /* Copyright 2015 Shlomi Noach, courtesy Booking.com + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */ @@ -12,9 +13,7 @@ import ( "strings" ) -const ( - DefaultInstancePort = 3306 -) +const DefaultInstancePort = 3306 var ( ipv4HostPortRegexp = regexp.MustCompile("^([^:]+):([0-9]+)$") diff --git a/go/sql/builder.go b/go/sql/builder.go index d373e5af8..15199ffca 100644 --- a/go/sql/builder.go +++ b/go/sql/builder.go @@ -1,5 +1,5 @@ /* - Copyright 2016 GitHub Inc. + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */