From d212d0284559db3d0e6e16de902937e9fa6768d8 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Tue, 31 May 2022 23:30:37 +0200 Subject: [PATCH 01/19] Add context/timeout to HTTP throttle check --- .golangci.yml | 1 + go/base/context.go | 15 +++++++++++++++ go/cmd/gh-ost/main.go | 2 ++ go/logic/throttler.go | 17 +++++++++++++++-- 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 4a487bd97..6a0faf699 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,4 +9,5 @@ linters: enable: - gosimple - govet + - noctx - unused diff --git a/go/base/context.go b/go/base/context.go index 93f84ce65..3376c36b5 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -184,6 +184,7 @@ type MigrationContext struct { CurrentLag int64 currentProgress uint64 etaNanoseonds int64 + ThrottleHTTPInterval time.Duration ThrottleHTTPStatusCode int64 controlReplicasLagResult mysql.ReplicationLagResult TotalRowsCopied int64 @@ -639,6 +640,20 @@ func (this *MigrationContext) SetThrottleHTTP(throttleHTTP string) { this.throttleHTTP = throttleHTTP } +func (this *MigrationContext) SetThrottleHTTPInterval(timeoutMillis int64) { + this.throttleHTTPMutex.Lock() + defer this.throttleHTTPMutex.Unlock() + + this.ThrottleHTTPInterval = time.Duration(timeoutMillis) * time.Millisecond +} + +func (this *MigrationContext) GetThrottleHTTPInterval() time.Duration { + this.throttleHTTPMutex.Lock() + defer this.throttleHTTPMutex.Unlock() + + return this.ThrottleHTTPInterval +} + func (this *MigrationContext) SetIgnoreHTTPErrors(ignoreHTTPErrors bool) { this.throttleHTTPMutex.Lock() defer this.throttleHTTPMutex.Unlock() diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 93d8fb94e..bdbccc767 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -110,6 +110,7 @@ func main() { throttleControlReplicas := flag.String("throttle-control-replicas", "", "List of replicas on which to check for lag; comma delimited. Example: myhost1.com:3306,myhost2.com,myhost3.com:3307") throttleQuery := flag.String("throttle-query", "", "when given, issued (every second) to check if operation should throttle. Expecting to return zero for no-throttle, >0 for throttle. Query is issued on the migrated server. Make sure this query is lightweight") throttleHTTP := flag.String("throttle-http", "", "when given, gh-ost checks given URL via HEAD request; any response code other than 200 (OK) causes throttling; make sure it has low latency response") + throttleHTTPIntervalMillis := flag.Int64("throttle-http-interval-millis", 100, "Number of milliseconds to wait between HTTP throttle checks") ignoreHTTPErrors := flag.Bool("ignore-http-errors", false, "ignore HTTP connection errors during throttle check") heartbeatIntervalMillis := flag.Int64("heartbeat-interval-millis", 100, "how frequently would gh-ost inject a heartbeat value") flag.StringVar(&migrationContext.ThrottleFlagFile, "throttle-flag-file", "", "operation pauses when this file exists; hint: use a file that is specific to the table being altered") @@ -281,6 +282,7 @@ func main() { migrationContext.SetMaxLagMillisecondsThrottleThreshold(*maxLagMillis) migrationContext.SetThrottleQuery(*throttleQuery) migrationContext.SetThrottleHTTP(*throttleHTTP) + migrationContext.SetThrottleHTTPInterval(*throttleHTTPIntervalMillis) migrationContext.SetIgnoreHTTPErrors(*ignoreHTTPErrors) migrationContext.SetDefaultNumRetries(*defaultRetries) migrationContext.ApplyCredentials() diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 9c120b3e5..050b1e3fb 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -6,6 +6,7 @@ package logic import ( + "context" "fmt" "net/http" "strings" @@ -44,6 +45,7 @@ const frenoMagicHint = "freno" type Throttler struct { migrationContext *base.MigrationContext applier *Applier + httpClient *http.Client inspector *Inspector finishedMigrating int64 } @@ -52,6 +54,7 @@ func NewThrottler(migrationContext *base.MigrationContext, applier *Applier, ins return &Throttler{ migrationContext: migrationContext, applier: applier, + httpClient: &http.Client{}, inspector: inspector, finishedMigrating: 0, } @@ -285,7 +288,17 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- if url == "" { return true, nil } - resp, err := http.Head(url) + + // make the HTTP context timeout equal to the ticker interval + ctx, cancel := context.WithTimeout(context.Background(), this.migrationContext.GetThrottleHTTPInterval()) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil) + if err != nil { + return false, err + } + + resp, err := this.httpClient.Do(req) if err != nil { return false, err } @@ -303,7 +316,7 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- firstThrottlingCollected <- true - ticker := time.Tick(100 * time.Millisecond) + ticker := time.Tick(this.migrationContext.GetThrottleHTTPInterval()) for range ticker { if atomic.LoadInt64(&this.finishedMigrating) > 0 { return From cd72b33bd88bc7233eb986aea395e9f4a76af902 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Tue, 31 May 2022 23:53:48 +0200 Subject: [PATCH 02/19] Dont run `.GetThrottleHTTPInterval()` on every loop --- go/logic/throttler.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 050b1e3fb..47cc75c89 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -280,6 +280,7 @@ func (this *Throttler) criticalLoadIsMet() (met bool, variableName string, value // collectReplicationLag reads the latest changelog heartbeat value func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- bool) { + collectInterval := this.migrationContext.GetThrottleHTTPInterval() collectFunc := func() (sleep bool, err error) { if atomic.LoadInt64(&this.migrationContext.HibernateUntil) > 0 { return true, nil @@ -290,7 +291,7 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- } // make the HTTP context timeout equal to the ticker interval - ctx, cancel := context.WithTimeout(context.Background(), this.migrationContext.GetThrottleHTTPInterval()) + ctx, cancel := context.WithTimeout(context.Background(), collectInterval) defer cancel() req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil) @@ -316,7 +317,7 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- firstThrottlingCollected <- true - ticker := time.Tick(this.migrationContext.GetThrottleHTTPInterval()) + ticker := time.Tick(collectInterval) for range ticker { if atomic.LoadInt64(&this.finishedMigrating) > 0 { return From a5dbba9e8cc381a00a5c88671aba45b766e1119c Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Tue, 31 May 2022 23:59:45 +0200 Subject: [PATCH 03/19] Update help message --- go/cmd/gh-ost/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index bdbccc767..9f588928c 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -110,7 +110,7 @@ func main() { throttleControlReplicas := flag.String("throttle-control-replicas", "", "List of replicas on which to check for lag; comma delimited. Example: myhost1.com:3306,myhost2.com,myhost3.com:3307") throttleQuery := flag.String("throttle-query", "", "when given, issued (every second) to check if operation should throttle. Expecting to return zero for no-throttle, >0 for throttle. Query is issued on the migrated server. Make sure this query is lightweight") throttleHTTP := flag.String("throttle-http", "", "when given, gh-ost checks given URL via HEAD request; any response code other than 200 (OK) causes throttling; make sure it has low latency response") - throttleHTTPIntervalMillis := flag.Int64("throttle-http-interval-millis", 100, "Number of milliseconds to wait between HTTP throttle checks") + throttleHTTPIntervalMillis := flag.Int64("throttle-http-interval-millis", 100, "Number of milliseconds to wait between HTTP throttle checks. This is also used as the HTTP request timeout") ignoreHTTPErrors := flag.Bool("ignore-http-errors", false, "ignore HTTP connection errors during throttle check") heartbeatIntervalMillis := flag.Int64("heartbeat-interval-millis", 100, "how frequently would gh-ost inject a heartbeat value") flag.StringVar(&migrationContext.ThrottleFlagFile, "throttle-flag-file", "", "operation pauses when this file exists; hint: use a file that is specific to the table being altered") From 140736147e0d91dcefeb1b3c5911ff5eb57b6038 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Wed, 1 Jun 2022 01:53:21 +0200 Subject: [PATCH 04/19] Var rename --- go/base/context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 3376c36b5..7661525d6 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -640,11 +640,11 @@ func (this *MigrationContext) SetThrottleHTTP(throttleHTTP string) { this.throttleHTTP = throttleHTTP } -func (this *MigrationContext) SetThrottleHTTPInterval(timeoutMillis int64) { +func (this *MigrationContext) SetThrottleHTTPInterval(intervalMillis int64) { this.throttleHTTPMutex.Lock() defer this.throttleHTTPMutex.Unlock() - this.ThrottleHTTPInterval = time.Duration(timeoutMillis) * time.Millisecond + this.ThrottleHTTPInterval = time.Duration(intervalMillis) * time.Millisecond } func (this *MigrationContext) GetThrottleHTTPInterval() time.Duration { From 604deb6b8c359928a03e51f2cc9b99b3c2f1a703 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Wed, 1 Jun 2022 23:28:16 +0200 Subject: [PATCH 05/19] 2022 --- go/cmd/gh-ost/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 9f588928c..81c2e7289 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -1,5 +1,5 @@ /* - Copyright 2016 GitHub Inc. + Copyright 2022 GitHub Inc. See https://github.com/github/gh-ost/blob/master/LICENSE */ From 3586db0a8d89861f53f96ac7bb77ddd69d33d5a5 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Tue, 7 Jun 2022 01:48:24 +0200 Subject: [PATCH 06/19] Add timeout flag --- go/base/context.go | 15 +++++++++++++++ go/cmd/gh-ost/main.go | 4 +++- go/logic/throttler.go | 6 +++--- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 7661525d6..8093f8811 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -186,6 +186,7 @@ type MigrationContext struct { etaNanoseonds int64 ThrottleHTTPInterval time.Duration ThrottleHTTPStatusCode int64 + ThrottleHTTPTimeout time.Duration controlReplicasLagResult mysql.ReplicationLagResult TotalRowsCopied int64 TotalDMLEventsApplied int64 @@ -654,6 +655,20 @@ func (this *MigrationContext) GetThrottleHTTPInterval() time.Duration { return this.ThrottleHTTPInterval } +func (this *MigrationContext) SetThrottleHTTPTimeout(timeoutMillis int64) { + this.throttleHTTPMutex.Lock() + defer this.throttleHTTPMutex.Unlock() + + this.ThrottleHTTPTimeout = time.Duration(timeoutMillis) * time.Millisecond +} + +func (this *MigrationContext) GetThrottleHTTPTimeout() time.Duration { + this.throttleHTTPMutex.Lock() + defer this.throttleHTTPMutex.Unlock() + + return this.ThrottleHTTPTimeout +} + func (this *MigrationContext) SetIgnoreHTTPErrors(ignoreHTTPErrors bool) { this.throttleHTTPMutex.Lock() defer this.throttleHTTPMutex.Unlock() diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 81c2e7289..28c34c34b 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -110,7 +110,8 @@ func main() { throttleControlReplicas := flag.String("throttle-control-replicas", "", "List of replicas on which to check for lag; comma delimited. Example: myhost1.com:3306,myhost2.com,myhost3.com:3307") throttleQuery := flag.String("throttle-query", "", "when given, issued (every second) to check if operation should throttle. Expecting to return zero for no-throttle, >0 for throttle. Query is issued on the migrated server. Make sure this query is lightweight") throttleHTTP := flag.String("throttle-http", "", "when given, gh-ost checks given URL via HEAD request; any response code other than 200 (OK) causes throttling; make sure it has low latency response") - throttleHTTPIntervalMillis := flag.Int64("throttle-http-interval-millis", 100, "Number of milliseconds to wait between HTTP throttle checks. This is also used as the HTTP request timeout") + throttleHTTPIntervalMillis := flag.Int64("throttle-http-interval-millis", 100, "Number of milliseconds to wait between HTTP throttle checks") + throttleHTTPTimeoutMillis := flag.Int64("throttle-http-timeout-millis", 1000, "Number of milliseconds to use as a HTTP throttle check timeout") ignoreHTTPErrors := flag.Bool("ignore-http-errors", false, "ignore HTTP connection errors during throttle check") heartbeatIntervalMillis := flag.Int64("heartbeat-interval-millis", 100, "how frequently would gh-ost inject a heartbeat value") flag.StringVar(&migrationContext.ThrottleFlagFile, "throttle-flag-file", "", "operation pauses when this file exists; hint: use a file that is specific to the table being altered") @@ -283,6 +284,7 @@ func main() { migrationContext.SetThrottleQuery(*throttleQuery) migrationContext.SetThrottleHTTP(*throttleHTTP) migrationContext.SetThrottleHTTPInterval(*throttleHTTPIntervalMillis) + migrationContext.SetThrottleHTTPTimeout(*throttleHTTPTimeoutMillis) migrationContext.SetIgnoreHTTPErrors(*ignoreHTTPErrors) migrationContext.SetDefaultNumRetries(*defaultRetries) migrationContext.ApplyCredentials() diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 47cc75c89..2358685e3 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -280,7 +280,7 @@ func (this *Throttler) criticalLoadIsMet() (met bool, variableName string, value // collectReplicationLag reads the latest changelog heartbeat value func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- bool) { - collectInterval := this.migrationContext.GetThrottleHTTPInterval() + collectTimeout := this.migrationContext.GetThrottleHTTPTimeout() collectFunc := func() (sleep bool, err error) { if atomic.LoadInt64(&this.migrationContext.HibernateUntil) > 0 { return true, nil @@ -291,7 +291,7 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- } // make the HTTP context timeout equal to the ticker interval - ctx, cancel := context.WithTimeout(context.Background(), collectInterval) + ctx, cancel := context.WithTimeout(context.Background(), collectTimeout) defer cancel() req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil) @@ -317,7 +317,7 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- firstThrottlingCollected <- true - ticker := time.Tick(collectInterval) + ticker := time.Tick(this.migrationContext.GetThrottleHTTPInterval()) for range ticker { if atomic.LoadInt64(&this.finishedMigrating) > 0 { return From 6c40b6f8c1f783950221f8ee707cc55dd2d8783e Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 18 Jun 2022 02:04:09 +0200 Subject: [PATCH 07/19] Add unix/tcp server commands, use ParseInt() for string->int64 --- go/logic/server.go | 80 +++++++++++++++++++++++++++++++--------------- 1 file changed, 54 insertions(+), 26 deletions(-) diff --git a/go/logic/server.go b/go/logic/server.go index 3d128b146..aa00316d8 100644 --- a/go/logic/server.go +++ b/go/logic/server.go @@ -144,26 +144,28 @@ func (this *Server) applyServerCommand(command string, writer *bufio.Writer) (pr case "help": { fmt.Fprint(writer, `available commands: -status # Print a detailed status message -sup # Print a short status message -coordinates # Print the currently inspected coordinates -applier # Print the hostname of the applier -inspector # Print the hostname of the inspector -chunk-size= # Set a new chunk-size -dml-batch-size= # Set a new dml-batch-size -nice-ratio= # Set a new nice-ratio, immediate sleep after each row-copy operation, float (examples: 0 is aggressive, 0.7 adds 70% runtime, 1.0 doubles runtime, 2.0 triples runtime, ...) -critical-load= # Set a new set of max-load thresholds -max-lag-millis= # Set a new replication lag threshold -replication-lag-query= # Set a new query that determines replication lag (no quotes) -max-load= # Set a new set of max-load thresholds -throttle-query= # Set a new throttle-query (no quotes) -throttle-http= # Set a new throttle URL -throttle-control-replicas= # Set a new comma delimited list of throttle control replicas -throttle # Force throttling -no-throttle # End forced throttling (other throttling may still apply) -unpostpone # Bail out a cut-over postpone; proceed to cut-over -panic # panic and quit without cleanup -help # This message +status # Print a detailed status message +sup # Print a short status message +coordinates # Print the currently inspected coordinates +applier # Print the hostname of the applier +inspector # Print the hostname of the inspector +chunk-size= # Set a new chunk-size +dml-batch-size= # Set a new dml-batch-size +nice-ratio= # Set a new nice-ratio, immediate sleep after each row-copy operation, float (examples: 0 is aggressive, 0.7 adds 70% runtime, 1.0 doubles runtime, 2.0 triples runtime, ...) +critical-load= # Set a new set of max-load thresholds +max-lag-millis= # Set a new replication lag threshold +replication-lag-query= # Set a new query that determines replication lag (no quotes) +max-load= # Set a new set of max-load thresholds +throttle-query= # Set a new throttle-query (no quotes) +throttle-http= # Set a new throttle URL +throttle-http-interval-millis= # Set throttler HTTP polling interval in milliseconds +throttle-http-timeout-millis= # Set throttler HTTP timeout in milliseconds +throttle-control-replicas= # Set a new comma delimited list of throttle control replicas +throttle # Force throttling +no-throttle # End forced throttling (other throttling may still apply) +unpostpone # Bail out a cut-over postpone; proceed to cut-over +panic # panic and quit without cleanup +help # This message - use '?' (question mark) as argument to get info rather than set. e.g. "max-load=?" will just print out current max-load. `) } @@ -201,10 +203,10 @@ help # This message fmt.Fprintf(writer, "%+v\n", atomic.LoadInt64(&this.migrationContext.ChunkSize)) return NoPrintStatusRule, nil } - if chunkSize, err := strconv.Atoi(arg); err != nil { + if chunkSize, err := strconv.ParseInt(arg, 10, 64); err != nil { return NoPrintStatusRule, err } else { - this.migrationContext.SetChunkSize(int64(chunkSize)) + this.migrationContext.SetChunkSize(chunkSize) return ForcePrintStatusAndHintRule, nil } } @@ -214,10 +216,10 @@ help # This message fmt.Fprintf(writer, "%+v\n", atomic.LoadInt64(&this.migrationContext.DMLBatchSize)) return NoPrintStatusRule, nil } - if dmlBatchSize, err := strconv.Atoi(arg); err != nil { + if dmlBatchSize, err := strconv.ParseInt(arg, 10, 64); err != nil { return NoPrintStatusRule, err } else { - this.migrationContext.SetDMLBatchSize(int64(dmlBatchSize)) + this.migrationContext.SetDMLBatchSize(dmlBatchSize) return ForcePrintStatusAndHintRule, nil } } @@ -227,10 +229,10 @@ help # This message fmt.Fprintf(writer, "%+v\n", atomic.LoadInt64(&this.migrationContext.MaxLagMillisecondsThrottleThreshold)) return NoPrintStatusRule, nil } - if maxLagMillis, err := strconv.Atoi(arg); err != nil { + if maxLagMillis, err := strconv.ParseInt(arg, 10, 64); err != nil { return NoPrintStatusRule, err } else { - this.migrationContext.SetMaxLagMillisecondsThrottleThreshold(int64(maxLagMillis)) + this.migrationContext.SetMaxLagMillisecondsThrottleThreshold(maxLagMillis) return ForcePrintStatusAndHintRule, nil } } @@ -295,6 +297,32 @@ help # This message fmt.Fprintf(writer, throttleHint) return ForcePrintStatusAndHintRule, nil } + case "throttle-http-interval-millis": + { + if argIsQuestion { + fmt.Fprintf(writer, "%+v\n", this.migrationContext.GetThrottleHTTPInterval()) + return NoPrintStatusRule, nil + } + if timeoutMillis, err := strconv.ParseInt(arg, 10, 64); err != nil { + return NoPrintStatusRule, err + } else { + this.migrationContext.SetThrottleHTTPInterval(timeoutMillis) + return ForcePrintStatusAndHintRule, nil + } + } + case "throttle-http-timeout-millis": + { + if argIsQuestion { + fmt.Fprintf(writer, "%+v\n", this.migrationContext.GetThrottleHTTPTimeout()) + return NoPrintStatusRule, nil + } + if timeout, err := strconv.ParseInt(arg, 10, 64); err != nil { + return NoPrintStatusRule, err + } else { + this.migrationContext.SetThrottleHTTPTimeout(timeout) + return ForcePrintStatusAndHintRule, nil + } + } case "throttle-control-replicas": { if argIsQuestion { From 6f21747a3cc18c191494685a90b9a66869e1466f Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 18 Jun 2022 02:05:24 +0200 Subject: [PATCH 08/19] Var rename --- go/logic/server.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go/logic/server.go b/go/logic/server.go index aa00316d8..d37442d14 100644 --- a/go/logic/server.go +++ b/go/logic/server.go @@ -303,10 +303,10 @@ help # This message fmt.Fprintf(writer, "%+v\n", this.migrationContext.GetThrottleHTTPInterval()) return NoPrintStatusRule, nil } - if timeoutMillis, err := strconv.ParseInt(arg, 10, 64); err != nil { + if intervalMillis, err := strconv.ParseInt(arg, 10, 64); err != nil { return NoPrintStatusRule, err } else { - this.migrationContext.SetThrottleHTTPInterval(timeoutMillis) + this.migrationContext.SetThrottleHTTPInterval(intervalMillis) return ForcePrintStatusAndHintRule, nil } } @@ -316,10 +316,10 @@ help # This message fmt.Fprintf(writer, "%+v\n", this.migrationContext.GetThrottleHTTPTimeout()) return NoPrintStatusRule, nil } - if timeout, err := strconv.ParseInt(arg, 10, 64); err != nil { + if timeoutMillis, err := strconv.ParseInt(arg, 10, 64); err != nil { return NoPrintStatusRule, err } else { - this.migrationContext.SetThrottleHTTPTimeout(timeout) + this.migrationContext.SetThrottleHTTPTimeout(timeoutMillis) return ForcePrintStatusAndHintRule, nil } } From ab49c9e3a4ce110aa1929d6ad28e903a54c977ac Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 18 Jun 2022 02:07:03 +0200 Subject: [PATCH 09/19] Re-check http timeout on every loop iteration --- go/logic/throttler.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 2358685e3..51351026a 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -280,7 +280,6 @@ func (this *Throttler) criticalLoadIsMet() (met bool, variableName string, value // collectReplicationLag reads the latest changelog heartbeat value func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- bool) { - collectTimeout := this.migrationContext.GetThrottleHTTPTimeout() collectFunc := func() (sleep bool, err error) { if atomic.LoadInt64(&this.migrationContext.HibernateUntil) > 0 { return true, nil @@ -291,7 +290,7 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- } // make the HTTP context timeout equal to the ticker interval - ctx, cancel := context.WithTimeout(context.Background(), collectTimeout) + ctx, cancel := context.WithTimeout(context.Background(), this.migrationContext.GetThrottleHTTPTimeout()) defer cancel() req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil) From c7e44b0298c42b35914fac315516f7afef51c83b Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 18 Jun 2022 02:07:53 +0200 Subject: [PATCH 10/19] Remove stale comment --- go/logic/throttler.go | 1 - 1 file changed, 1 deletion(-) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 51351026a..9c367196d 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -289,7 +289,6 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- return true, nil } - // make the HTTP context timeout equal to the ticker interval ctx, cancel := context.WithTimeout(context.Background(), this.migrationContext.GetThrottleHTTPTimeout()) defer cancel() From 16f38faa8bcd236f72d1b4f71dc0c0cd300a53bc Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 18 Jun 2022 02:14:54 +0200 Subject: [PATCH 11/19] Make throttle interval idempotent --- go/base/context.go | 16 +------------ go/cmd/gh-ost/main.go | 3 +-- go/logic/server.go | 56 ++++++++++++++++--------------------------- go/logic/throttler.go | 3 ++- 4 files changed, 25 insertions(+), 53 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 8093f8811..d18973786 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -184,7 +184,7 @@ type MigrationContext struct { CurrentLag int64 currentProgress uint64 etaNanoseonds int64 - ThrottleHTTPInterval time.Duration + ThrottleHTTPIntervalMillis int64 ThrottleHTTPStatusCode int64 ThrottleHTTPTimeout time.Duration controlReplicasLagResult mysql.ReplicationLagResult @@ -641,20 +641,6 @@ func (this *MigrationContext) SetThrottleHTTP(throttleHTTP string) { this.throttleHTTP = throttleHTTP } -func (this *MigrationContext) SetThrottleHTTPInterval(intervalMillis int64) { - this.throttleHTTPMutex.Lock() - defer this.throttleHTTPMutex.Unlock() - - this.ThrottleHTTPInterval = time.Duration(intervalMillis) * time.Millisecond -} - -func (this *MigrationContext) GetThrottleHTTPInterval() time.Duration { - this.throttleHTTPMutex.Lock() - defer this.throttleHTTPMutex.Unlock() - - return this.ThrottleHTTPInterval -} - func (this *MigrationContext) SetThrottleHTTPTimeout(timeoutMillis int64) { this.throttleHTTPMutex.Lock() defer this.throttleHTTPMutex.Unlock() diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 28c34c34b..fc31b234d 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -110,7 +110,7 @@ func main() { throttleControlReplicas := flag.String("throttle-control-replicas", "", "List of replicas on which to check for lag; comma delimited. Example: myhost1.com:3306,myhost2.com,myhost3.com:3307") throttleQuery := flag.String("throttle-query", "", "when given, issued (every second) to check if operation should throttle. Expecting to return zero for no-throttle, >0 for throttle. Query is issued on the migrated server. Make sure this query is lightweight") throttleHTTP := flag.String("throttle-http", "", "when given, gh-ost checks given URL via HEAD request; any response code other than 200 (OK) causes throttling; make sure it has low latency response") - throttleHTTPIntervalMillis := flag.Int64("throttle-http-interval-millis", 100, "Number of milliseconds to wait between HTTP throttle checks") + flag.Int64Var(&migrationContext.ThrottleHTTPIntervalMillis, "throttle-http-interval-millis", 100, "Number of milliseconds to wait between HTTP throttle checks") throttleHTTPTimeoutMillis := flag.Int64("throttle-http-timeout-millis", 1000, "Number of milliseconds to use as a HTTP throttle check timeout") ignoreHTTPErrors := flag.Bool("ignore-http-errors", false, "ignore HTTP connection errors during throttle check") heartbeatIntervalMillis := flag.Int64("heartbeat-interval-millis", 100, "how frequently would gh-ost inject a heartbeat value") @@ -283,7 +283,6 @@ func main() { migrationContext.SetMaxLagMillisecondsThrottleThreshold(*maxLagMillis) migrationContext.SetThrottleQuery(*throttleQuery) migrationContext.SetThrottleHTTP(*throttleHTTP) - migrationContext.SetThrottleHTTPInterval(*throttleHTTPIntervalMillis) migrationContext.SetThrottleHTTPTimeout(*throttleHTTPTimeoutMillis) migrationContext.SetIgnoreHTTPErrors(*ignoreHTTPErrors) migrationContext.SetDefaultNumRetries(*defaultRetries) diff --git a/go/logic/server.go b/go/logic/server.go index d37442d14..d5c99a504 100644 --- a/go/logic/server.go +++ b/go/logic/server.go @@ -144,28 +144,27 @@ func (this *Server) applyServerCommand(command string, writer *bufio.Writer) (pr case "help": { fmt.Fprint(writer, `available commands: -status # Print a detailed status message -sup # Print a short status message -coordinates # Print the currently inspected coordinates -applier # Print the hostname of the applier -inspector # Print the hostname of the inspector -chunk-size= # Set a new chunk-size -dml-batch-size= # Set a new dml-batch-size -nice-ratio= # Set a new nice-ratio, immediate sleep after each row-copy operation, float (examples: 0 is aggressive, 0.7 adds 70% runtime, 1.0 doubles runtime, 2.0 triples runtime, ...) -critical-load= # Set a new set of max-load thresholds -max-lag-millis= # Set a new replication lag threshold -replication-lag-query= # Set a new query that determines replication lag (no quotes) -max-load= # Set a new set of max-load thresholds -throttle-query= # Set a new throttle-query (no quotes) -throttle-http= # Set a new throttle URL -throttle-http-interval-millis= # Set throttler HTTP polling interval in milliseconds -throttle-http-timeout-millis= # Set throttler HTTP timeout in milliseconds -throttle-control-replicas= # Set a new comma delimited list of throttle control replicas -throttle # Force throttling -no-throttle # End forced throttling (other throttling may still apply) -unpostpone # Bail out a cut-over postpone; proceed to cut-over -panic # panic and quit without cleanup -help # This message +status # Print a detailed status message +sup # Print a short status message +coordinates # Print the currently inspected coordinates +applier # Print the hostname of the applier +inspector # Print the hostname of the inspector +chunk-size= # Set a new chunk-size +dml-batch-size= # Set a new dml-batch-size +nice-ratio= # Set a new nice-ratio, immediate sleep after each row-copy operation, float (examples: 0 is aggressive, 0.7 adds 70% runtime, 1.0 doubles runtime, 2.0 triples runtime, ...) +critical-load= # Set a new set of max-load thresholds +max-lag-millis= # Set a new replication lag threshold +replication-lag-query= # Set a new query that determines replication lag (no quotes) +max-load= # Set a new set of max-load thresholds +throttle-query= # Set a new throttle-query (no quotes) +throttle-http= # Set a new throttle URL +throttle-http-timeout-millis= # Set throttler HTTP timeout in milliseconds +throttle-control-replicas= # Set a new comma delimited list of throttle control replicas +throttle # Force throttling +no-throttle # End forced throttling (other throttling may still apply) +unpostpone # Bail out a cut-over postpone; proceed to cut-over +panic # panic and quit without cleanup +help # This message - use '?' (question mark) as argument to get info rather than set. e.g. "max-load=?" will just print out current max-load. `) } @@ -297,19 +296,6 @@ help # This message fmt.Fprintf(writer, throttleHint) return ForcePrintStatusAndHintRule, nil } - case "throttle-http-interval-millis": - { - if argIsQuestion { - fmt.Fprintf(writer, "%+v\n", this.migrationContext.GetThrottleHTTPInterval()) - return NoPrintStatusRule, nil - } - if intervalMillis, err := strconv.ParseInt(arg, 10, 64); err != nil { - return NoPrintStatusRule, err - } else { - this.migrationContext.SetThrottleHTTPInterval(intervalMillis) - return ForcePrintStatusAndHintRule, nil - } - } case "throttle-http-timeout-millis": { if argIsQuestion { diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 9c367196d..c54da8154 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -315,7 +315,8 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- firstThrottlingCollected <- true - ticker := time.Tick(this.migrationContext.GetThrottleHTTPInterval()) + interval := time.Duration(this.migrationContext.ThrottleHTTPIntervalMillis) * time.Millisecond + ticker := time.Tick(interval) for range ticker { if atomic.LoadInt64(&this.finishedMigrating) > 0 { return From 6a38297d4031472a69b2280f388fd6fb28c9989a Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 18 Jun 2022 02:17:20 +0200 Subject: [PATCH 12/19] var rename --- go/logic/throttler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index c54da8154..8a8d6cf68 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -315,8 +315,8 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- firstThrottlingCollected <- true - interval := time.Duration(this.migrationContext.ThrottleHTTPIntervalMillis) * time.Millisecond - ticker := time.Tick(interval) + collectInterval := time.Duration(this.migrationContext.ThrottleHTTPIntervalMillis) * time.Millisecond + ticker := time.Tick(collectInterval) for range ticker { if atomic.LoadInt64(&this.finishedMigrating) > 0 { return From ecd1fe3b7fb8bb9c1adbdf4d635981b84c537b99 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 18 Jun 2022 02:18:17 +0200 Subject: [PATCH 13/19] Usage grammar --- go/cmd/gh-ost/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index fc31b234d..a55305e53 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -111,7 +111,7 @@ func main() { throttleQuery := flag.String("throttle-query", "", "when given, issued (every second) to check if operation should throttle. Expecting to return zero for no-throttle, >0 for throttle. Query is issued on the migrated server. Make sure this query is lightweight") throttleHTTP := flag.String("throttle-http", "", "when given, gh-ost checks given URL via HEAD request; any response code other than 200 (OK) causes throttling; make sure it has low latency response") flag.Int64Var(&migrationContext.ThrottleHTTPIntervalMillis, "throttle-http-interval-millis", 100, "Number of milliseconds to wait between HTTP throttle checks") - throttleHTTPTimeoutMillis := flag.Int64("throttle-http-timeout-millis", 1000, "Number of milliseconds to use as a HTTP throttle check timeout") + throttleHTTPTimeoutMillis := flag.Int64("throttle-http-timeout-millis", 1000, "Number of milliseconds to use as an HTTP throttle check timeout") ignoreHTTPErrors := flag.Bool("ignore-http-errors", false, "ignore HTTP connection errors during throttle check") heartbeatIntervalMillis := flag.Int64("heartbeat-interval-millis", 100, "how frequently would gh-ost inject a heartbeat value") flag.StringVar(&migrationContext.ThrottleFlagFile, "throttle-flag-file", "", "operation pauses when this file exists; hint: use a file that is specific to the table being altered") From 629063c6304cf885b6d3af6a9d13a825816e6238 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 18 Jun 2022 02:25:49 +0200 Subject: [PATCH 14/19] Make http timeout idempotent too --- go/base/context.go | 16 +---------- go/cmd/gh-ost/main.go | 3 +- go/logic/server.go | 66 +++++++++++++++++-------------------------- go/logic/throttler.go | 3 +- 4 files changed, 30 insertions(+), 58 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index d18973786..1c8b17a38 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -186,7 +186,7 @@ type MigrationContext struct { etaNanoseonds int64 ThrottleHTTPIntervalMillis int64 ThrottleHTTPStatusCode int64 - ThrottleHTTPTimeout time.Duration + ThrottleHTTPTimeoutMillis int64 controlReplicasLagResult mysql.ReplicationLagResult TotalRowsCopied int64 TotalDMLEventsApplied int64 @@ -641,20 +641,6 @@ func (this *MigrationContext) SetThrottleHTTP(throttleHTTP string) { this.throttleHTTP = throttleHTTP } -func (this *MigrationContext) SetThrottleHTTPTimeout(timeoutMillis int64) { - this.throttleHTTPMutex.Lock() - defer this.throttleHTTPMutex.Unlock() - - this.ThrottleHTTPTimeout = time.Duration(timeoutMillis) * time.Millisecond -} - -func (this *MigrationContext) GetThrottleHTTPTimeout() time.Duration { - this.throttleHTTPMutex.Lock() - defer this.throttleHTTPMutex.Unlock() - - return this.ThrottleHTTPTimeout -} - func (this *MigrationContext) SetIgnoreHTTPErrors(ignoreHTTPErrors bool) { this.throttleHTTPMutex.Lock() defer this.throttleHTTPMutex.Unlock() diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index a55305e53..8f5822729 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -111,7 +111,7 @@ func main() { throttleQuery := flag.String("throttle-query", "", "when given, issued (every second) to check if operation should throttle. Expecting to return zero for no-throttle, >0 for throttle. Query is issued on the migrated server. Make sure this query is lightweight") throttleHTTP := flag.String("throttle-http", "", "when given, gh-ost checks given URL via HEAD request; any response code other than 200 (OK) causes throttling; make sure it has low latency response") flag.Int64Var(&migrationContext.ThrottleHTTPIntervalMillis, "throttle-http-interval-millis", 100, "Number of milliseconds to wait between HTTP throttle checks") - throttleHTTPTimeoutMillis := flag.Int64("throttle-http-timeout-millis", 1000, "Number of milliseconds to use as an HTTP throttle check timeout") + flag.Int64Var(&migrationContext.ThrottleHTTPTimeoutMillis, "throttle-http-timeout-millis", 1000, "Number of milliseconds to use as an HTTP throttle check timeout") ignoreHTTPErrors := flag.Bool("ignore-http-errors", false, "ignore HTTP connection errors during throttle check") heartbeatIntervalMillis := flag.Int64("heartbeat-interval-millis", 100, "how frequently would gh-ost inject a heartbeat value") flag.StringVar(&migrationContext.ThrottleFlagFile, "throttle-flag-file", "", "operation pauses when this file exists; hint: use a file that is specific to the table being altered") @@ -283,7 +283,6 @@ func main() { migrationContext.SetMaxLagMillisecondsThrottleThreshold(*maxLagMillis) migrationContext.SetThrottleQuery(*throttleQuery) migrationContext.SetThrottleHTTP(*throttleHTTP) - migrationContext.SetThrottleHTTPTimeout(*throttleHTTPTimeoutMillis) migrationContext.SetIgnoreHTTPErrors(*ignoreHTTPErrors) migrationContext.SetDefaultNumRetries(*defaultRetries) migrationContext.ApplyCredentials() diff --git a/go/logic/server.go b/go/logic/server.go index d5c99a504..3d128b146 100644 --- a/go/logic/server.go +++ b/go/logic/server.go @@ -144,27 +144,26 @@ func (this *Server) applyServerCommand(command string, writer *bufio.Writer) (pr case "help": { fmt.Fprint(writer, `available commands: -status # Print a detailed status message -sup # Print a short status message -coordinates # Print the currently inspected coordinates -applier # Print the hostname of the applier -inspector # Print the hostname of the inspector -chunk-size= # Set a new chunk-size -dml-batch-size= # Set a new dml-batch-size -nice-ratio= # Set a new nice-ratio, immediate sleep after each row-copy operation, float (examples: 0 is aggressive, 0.7 adds 70% runtime, 1.0 doubles runtime, 2.0 triples runtime, ...) -critical-load= # Set a new set of max-load thresholds -max-lag-millis= # Set a new replication lag threshold -replication-lag-query= # Set a new query that determines replication lag (no quotes) -max-load= # Set a new set of max-load thresholds -throttle-query= # Set a new throttle-query (no quotes) -throttle-http= # Set a new throttle URL -throttle-http-timeout-millis= # Set throttler HTTP timeout in milliseconds -throttle-control-replicas= # Set a new comma delimited list of throttle control replicas -throttle # Force throttling -no-throttle # End forced throttling (other throttling may still apply) -unpostpone # Bail out a cut-over postpone; proceed to cut-over -panic # panic and quit without cleanup -help # This message +status # Print a detailed status message +sup # Print a short status message +coordinates # Print the currently inspected coordinates +applier # Print the hostname of the applier +inspector # Print the hostname of the inspector +chunk-size= # Set a new chunk-size +dml-batch-size= # Set a new dml-batch-size +nice-ratio= # Set a new nice-ratio, immediate sleep after each row-copy operation, float (examples: 0 is aggressive, 0.7 adds 70% runtime, 1.0 doubles runtime, 2.0 triples runtime, ...) +critical-load= # Set a new set of max-load thresholds +max-lag-millis= # Set a new replication lag threshold +replication-lag-query= # Set a new query that determines replication lag (no quotes) +max-load= # Set a new set of max-load thresholds +throttle-query= # Set a new throttle-query (no quotes) +throttle-http= # Set a new throttle URL +throttle-control-replicas= # Set a new comma delimited list of throttle control replicas +throttle # Force throttling +no-throttle # End forced throttling (other throttling may still apply) +unpostpone # Bail out a cut-over postpone; proceed to cut-over +panic # panic and quit without cleanup +help # This message - use '?' (question mark) as argument to get info rather than set. e.g. "max-load=?" will just print out current max-load. `) } @@ -202,10 +201,10 @@ help # This message fmt.Fprintf(writer, "%+v\n", atomic.LoadInt64(&this.migrationContext.ChunkSize)) return NoPrintStatusRule, nil } - if chunkSize, err := strconv.ParseInt(arg, 10, 64); err != nil { + if chunkSize, err := strconv.Atoi(arg); err != nil { return NoPrintStatusRule, err } else { - this.migrationContext.SetChunkSize(chunkSize) + this.migrationContext.SetChunkSize(int64(chunkSize)) return ForcePrintStatusAndHintRule, nil } } @@ -215,10 +214,10 @@ help # This message fmt.Fprintf(writer, "%+v\n", atomic.LoadInt64(&this.migrationContext.DMLBatchSize)) return NoPrintStatusRule, nil } - if dmlBatchSize, err := strconv.ParseInt(arg, 10, 64); err != nil { + if dmlBatchSize, err := strconv.Atoi(arg); err != nil { return NoPrintStatusRule, err } else { - this.migrationContext.SetDMLBatchSize(dmlBatchSize) + this.migrationContext.SetDMLBatchSize(int64(dmlBatchSize)) return ForcePrintStatusAndHintRule, nil } } @@ -228,10 +227,10 @@ help # This message fmt.Fprintf(writer, "%+v\n", atomic.LoadInt64(&this.migrationContext.MaxLagMillisecondsThrottleThreshold)) return NoPrintStatusRule, nil } - if maxLagMillis, err := strconv.ParseInt(arg, 10, 64); err != nil { + if maxLagMillis, err := strconv.Atoi(arg); err != nil { return NoPrintStatusRule, err } else { - this.migrationContext.SetMaxLagMillisecondsThrottleThreshold(maxLagMillis) + this.migrationContext.SetMaxLagMillisecondsThrottleThreshold(int64(maxLagMillis)) return ForcePrintStatusAndHintRule, nil } } @@ -296,19 +295,6 @@ help # This message fmt.Fprintf(writer, throttleHint) return ForcePrintStatusAndHintRule, nil } - case "throttle-http-timeout-millis": - { - if argIsQuestion { - fmt.Fprintf(writer, "%+v\n", this.migrationContext.GetThrottleHTTPTimeout()) - return NoPrintStatusRule, nil - } - if timeoutMillis, err := strconv.ParseInt(arg, 10, 64); err != nil { - return NoPrintStatusRule, err - } else { - this.migrationContext.SetThrottleHTTPTimeout(timeoutMillis) - return ForcePrintStatusAndHintRule, nil - } - } case "throttle-control-replicas": { if argIsQuestion { diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 8a8d6cf68..a25e09482 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -289,7 +289,8 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- return true, nil } - ctx, cancel := context.WithTimeout(context.Background(), this.migrationContext.GetThrottleHTTPTimeout()) + httpTimeout := time.Duration(this.migrationContext.ThrottleHTTPTimeoutMillis) * time.Millisecond + ctx, cancel := context.WithTimeout(context.Background(), httpTimeout) defer cancel() req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil) From 6a0eb54da88d6dd99cba0ece9bcff5bb38791d73 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 18 Jun 2022 02:27:23 +0200 Subject: [PATCH 15/19] Parse time.Duration once --- go/logic/throttler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index a25e09482..32f71497d 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -280,6 +280,7 @@ func (this *Throttler) criticalLoadIsMet() (met bool, variableName string, value // collectReplicationLag reads the latest changelog heartbeat value func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- bool) { + httpTimeout := time.Duration(this.migrationContext.ThrottleHTTPTimeoutMillis) * time.Millisecond collectFunc := func() (sleep bool, err error) { if atomic.LoadInt64(&this.migrationContext.HibernateUntil) > 0 { return true, nil @@ -289,7 +290,6 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- return true, nil } - httpTimeout := time.Duration(this.migrationContext.ThrottleHTTPTimeoutMillis) * time.Millisecond ctx, cancel := context.WithTimeout(context.Background(), httpTimeout) defer cancel() From 8ee88d91bf373503ec32e795ca5e4b80b09090cd Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 18 Jun 2022 02:29:52 +0200 Subject: [PATCH 16/19] Move timeout to NewThrottler --- go/logic/throttler.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 32f71497d..0adb2bb94 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -46,6 +46,7 @@ type Throttler struct { migrationContext *base.MigrationContext applier *Applier httpClient *http.Client + httpClientTimeout time.Duration inspector *Inspector finishedMigrating int64 } @@ -55,6 +56,7 @@ func NewThrottler(migrationContext *base.MigrationContext, applier *Applier, ins migrationContext: migrationContext, applier: applier, httpClient: &http.Client{}, + httpClientTimeout: time.Duration(migrationContext.ThrottleHTTPTimeoutMillis) * time.Millisecond, inspector: inspector, finishedMigrating: 0, } @@ -280,7 +282,6 @@ func (this *Throttler) criticalLoadIsMet() (met bool, variableName string, value // collectReplicationLag reads the latest changelog heartbeat value func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- bool) { - httpTimeout := time.Duration(this.migrationContext.ThrottleHTTPTimeoutMillis) * time.Millisecond collectFunc := func() (sleep bool, err error) { if atomic.LoadInt64(&this.migrationContext.HibernateUntil) > 0 { return true, nil @@ -290,7 +291,7 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- return true, nil } - ctx, cancel := context.WithTimeout(context.Background(), httpTimeout) + ctx, cancel := context.WithTimeout(context.Background(), this.httpClientTimeout) defer cancel() req, err := http.NewRequestWithContext(ctx, http.MethodHead, url, nil) From 3e9e9d23c057d2c0cf07fa5a5d1d4693cc268f65 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 18 Jun 2022 06:21:57 +0200 Subject: [PATCH 17/19] Help update --- go/cmd/gh-ost/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 8f5822729..7c9ca690b 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -110,7 +110,7 @@ func main() { throttleControlReplicas := flag.String("throttle-control-replicas", "", "List of replicas on which to check for lag; comma delimited. Example: myhost1.com:3306,myhost2.com,myhost3.com:3307") throttleQuery := flag.String("throttle-query", "", "when given, issued (every second) to check if operation should throttle. Expecting to return zero for no-throttle, >0 for throttle. Query is issued on the migrated server. Make sure this query is lightweight") throttleHTTP := flag.String("throttle-http", "", "when given, gh-ost checks given URL via HEAD request; any response code other than 200 (OK) causes throttling; make sure it has low latency response") - flag.Int64Var(&migrationContext.ThrottleHTTPIntervalMillis, "throttle-http-interval-millis", 100, "Number of milliseconds to wait between HTTP throttle checks") + flag.Int64Var(&migrationContext.ThrottleHTTPIntervalMillis, "throttle-http-interval-millis", 100, "Number of milliseconds to wait before triggering another HTTP throttle check") flag.Int64Var(&migrationContext.ThrottleHTTPTimeoutMillis, "throttle-http-timeout-millis", 1000, "Number of milliseconds to use as an HTTP throttle check timeout") ignoreHTTPErrors := flag.Bool("ignore-http-errors", false, "ignore HTTP connection errors during throttle check") heartbeatIntervalMillis := flag.Int64("heartbeat-interval-millis", 100, "how frequently would gh-ost inject a heartbeat value") From 8980673ca20d9fc73b454a48df4c4a515ef87ed7 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 18 Jun 2022 07:49:32 +0200 Subject: [PATCH 18/19] Set User-Agent header --- go/cmd/gh-ost/main.go | 2 +- go/logic/migrator.go | 6 ++++-- go/logic/throttler.go | 5 ++++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index 7c9ca690b..d0ae4c3d3 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -299,7 +299,7 @@ func main() { log.Infof("starting gh-ost %+v", AppVersion) acceptSignals(migrationContext) - migrator := logic.NewMigrator(migrationContext) + migrator := logic.NewMigrator(migrationContext, AppVersion) err := migrator.Migrate() if err != nil { migrator.ExecOnFailureHook() diff --git a/go/logic/migrator.go b/go/logic/migrator.go index c5cb24432..4426b4711 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -61,6 +61,7 @@ const ( // Migrator is the main schema migration flow manager. type Migrator struct { + appVersion string parser *sql.AlterTableParser inspector *Inspector applier *Applier @@ -86,8 +87,9 @@ type Migrator struct { finishedMigrating int64 } -func NewMigrator(context *base.MigrationContext) *Migrator { +func NewMigrator(context *base.MigrationContext, appVersion string) *Migrator { migrator := &Migrator{ + appVersion: appVersion, migrationContext: context, parser: sql.NewAlterTableParser(), ghostTableMigrated: make(chan bool), @@ -1064,7 +1066,7 @@ func (this *Migrator) addDMLEventsListener() error { // initiateThrottler kicks in the throttling collection and the throttling checks. func (this *Migrator) initiateThrottler() error { - this.throttler = NewThrottler(this.migrationContext, this.applier, this.inspector) + this.throttler = NewThrottler(this.migrationContext, this.applier, this.inspector, this.appVersion) go this.throttler.initiateThrottlerCollection(this.firstThrottlingCollected) this.migrationContext.Log.Infof("Waiting for first throttle metrics to be collected") diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 0adb2bb94..8f3d0db10 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -43,6 +43,7 @@ const frenoMagicHint = "freno" // Throttler collects metrics related to throttling and makes informed decision // whether throttling should take place. type Throttler struct { + appVersion string migrationContext *base.MigrationContext applier *Applier httpClient *http.Client @@ -51,8 +52,9 @@ type Throttler struct { finishedMigrating int64 } -func NewThrottler(migrationContext *base.MigrationContext, applier *Applier, inspector *Inspector) *Throttler { +func NewThrottler(migrationContext *base.MigrationContext, applier *Applier, inspector *Inspector, appVersion string) *Throttler { return &Throttler{ + appVersion: appVersion, migrationContext: migrationContext, applier: applier, httpClient: &http.Client{}, @@ -298,6 +300,7 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- if err != nil { return false, err } + req.Header.Set("User-Agent", fmt.Sprintf("gh-ost/%s", this.appVersion)) resp, err := this.httpClient.Do(req) if err != nil { From ecba185b95d8eac3a7e338021ff426c7609d44c8 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Wed, 6 Jul 2022 23:46:37 +0200 Subject: [PATCH 19/19] Re-add newline --- .golangci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 2f6fc1f88..a71dcdd31 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -12,4 +12,5 @@ linters: - noctx - rowserrcheck - sqlclosecheck - - unused \ No newline at end of file + - unused +