From d5a7e43266c36d86b7ef478bc975983ca24317a9 Mon Sep 17 00:00:00 2001 From: Justin Fudally Date: Tue, 24 Mar 2020 14:54:54 -0500 Subject: [PATCH 01/11] update gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 63f0df9..605546d 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /bin/ /libexec/ /.vendor/ +.idea/ From 0b2702bf539aebea09948fe5e9486a30135e4241 Mon Sep 17 00:00:00 2001 From: Justin Fudally Date: Wed, 25 Mar 2020 14:12:00 -0500 Subject: [PATCH 02/11] Throttle on no metrics/error --- 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 6f9f4bb..0923c7b 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -104,7 +104,7 @@ func (this *Throttler) shouldThrottle() (result bool, reason string, reasonHint } } // Got here? No metrics indicates we need throttling. - return false, "", base.NoThrottleReasonHint + return true, "", base.NoThrottleReasonHint } // parseChangelogHeartbeat parses a string timestamp and deduces replication lag From 64083e4705042a1d6a816a5671a61735139fe347 Mon Sep 17 00:00:00 2001 From: Justin Fudally Date: Wed, 25 Mar 2020 15:40:09 -0500 Subject: [PATCH 03/11] Throttle on no metrics/error --- go/logic/throttler.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 0923c7b..25ad6cf 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -84,6 +84,7 @@ func (this *Throttler) shouldThrottle() (result bool, reason string, reasonHint if statusCode != 0 && statusCode != http.StatusOK { return true, this.throttleHttpMessage(int(statusCode)), base.NoThrottleReasonHint } + // Replication lag throttle maxLagMillisecondsThrottleThreshold := atomic.LoadInt64(&this.migrationContext.MaxLagMillisecondsThrottleThreshold) lag := atomic.LoadInt64(&this.migrationContext.CurrentLag) @@ -104,7 +105,7 @@ func (this *Throttler) shouldThrottle() (result bool, reason string, reasonHint } } // Got here? No metrics indicates we need throttling. - return true, "", base.NoThrottleReasonHint + return false, "", base.NoThrottleReasonHint } // parseChangelogHeartbeat parses a string timestamp and deduces replication lag @@ -288,7 +289,11 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- return false, nil } - collectFunc() + _, err := collectFunc() + if err != nil { + atomic.StoreInt64(&this.migrationContext.ThrottleHTTPStatusCode, int64(-1)) + } + firstThrottlingCollected <- true ticker := time.Tick(100 * time.Millisecond) From ca0b822a3de06345a36326fa10a15bead239adaf Mon Sep 17 00:00:00 2001 From: Justin Fudally Date: Wed, 25 Mar 2020 15:41:23 -0500 Subject: [PATCH 04/11] Add comment --- go/logic/throttler.go | 1 + 1 file changed, 1 insertion(+) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 25ad6cf..6113374 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -291,6 +291,7 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- _, err := collectFunc() if err != nil { + // If an error occurs during the HTTP throttle check, let's throttle to be safe atomic.StoreInt64(&this.migrationContext.ThrottleHTTPStatusCode, int64(-1)) } From d31f3aac0c09ecb258a29f1cf0f0bb3d2b4a639c Mon Sep 17 00:00:00 2001 From: Justin Fudally Date: Wed, 25 Mar 2020 15:47:56 -0500 Subject: [PATCH 05/11] Add CLI flag for ignoring http errors --- go/base/context.go | 8 ++++++++ go/cmd/gh-ost/main.go | 2 ++ 2 files changed, 10 insertions(+) diff --git a/go/base/context.go b/go/base/context.go index 5ebf092..983ae8f 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -119,6 +119,7 @@ type MigrationContext struct { ThrottleAdditionalFlagFile string throttleQuery string throttleHTTP string + ignoreHTTPErrors bool ThrottleCommandedByUser int64 HibernateUntil int64 maxLoad LoadMap @@ -574,6 +575,13 @@ func (this *MigrationContext) SetThrottleHTTP(throttleHTTP string) { this.throttleHTTP = throttleHTTP } +func (this *MigrationContext) SetIgnoreHTTPErrors(ignoreHTTPErrors bool) { + this.throttleHTTPMutex.Lock() + defer this.throttleHTTPMutex.Unlock() + + this.ignoreHTTPErrors = ignoreHTTPErrors +} + func (this *MigrationContext) GetMaxLoad() LoadMap { this.throttleMutex.Lock() defer this.throttleMutex.Unlock() diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index ce63b03..bfd852a 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -106,6 +106,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") + ignoreHTTPErrors := flag.Bool("ignore-http-error", 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") flag.StringVar(&migrationContext.ThrottleAdditionalFlagFile, "throttle-additional-flag-file", "/tmp/gh-ost.throttle", "operation pauses when this file exists; hint: keep default, use for throttling multiple gh-ost operations") @@ -259,6 +260,7 @@ func main() { migrationContext.SetMaxLagMillisecondsThrottleThreshold(*maxLagMillis) migrationContext.SetThrottleQuery(*throttleQuery) migrationContext.SetThrottleHTTP(*throttleHTTP) + migrationContext.SetIgnoreHTTPErrors(*ignoreHTTPErrors) migrationContext.SetDefaultNumRetries(*defaultRetries) migrationContext.ApplyCredentials() if err := migrationContext.SetupTLS(); err != nil { From 46dabd338b40ef73fb782046b792d3fc1f30d83d Mon Sep 17 00:00:00 2001 From: Justin Fudally Date: Wed, 25 Mar 2020 15:48:19 -0500 Subject: [PATCH 06/11] go fmt --- go/base/context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/base/context.go b/go/base/context.go index 983ae8f..9a3c577 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -119,7 +119,7 @@ type MigrationContext struct { ThrottleAdditionalFlagFile string throttleQuery string throttleHTTP string - ignoreHTTPErrors bool + ignoreHTTPErrors bool ThrottleCommandedByUser int64 HibernateUntil int64 maxLoad LoadMap From 57cf5f3c90c2e2db50863505362e484f1bb26695 Mon Sep 17 00:00:00 2001 From: Justin Fudally Date: Wed, 25 Mar 2020 15:58:32 -0500 Subject: [PATCH 07/11] add override to ignore http errors --- go/base/context.go | 4 ++-- go/cmd/gh-ost/main.go | 2 +- go/logic/throttler.go | 6 ++++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 9a3c577..52d02dd 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -119,7 +119,7 @@ type MigrationContext struct { ThrottleAdditionalFlagFile string throttleQuery string throttleHTTP string - ignoreHTTPErrors bool + IgnoreHTTPErrors bool ThrottleCommandedByUser int64 HibernateUntil int64 maxLoad LoadMap @@ -579,7 +579,7 @@ func (this *MigrationContext) SetIgnoreHTTPErrors(ignoreHTTPErrors bool) { this.throttleHTTPMutex.Lock() defer this.throttleHTTPMutex.Unlock() - this.ignoreHTTPErrors = ignoreHTTPErrors + this.IgnoreHTTPErrors = ignoreHTTPErrors } func (this *MigrationContext) GetMaxLoad() LoadMap { diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index bfd852a..33fc6f4 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -106,7 +106,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") - ignoreHTTPErrors := flag.Bool("ignore-http-error", false, "ignore HTTP connection errors during throttle check") + 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") flag.StringVar(&migrationContext.ThrottleAdditionalFlagFile, "throttle-additional-flag-file", "/tmp/gh-ost.throttle", "operation pauses when this file exists; hint: keep default, use for throttling multiple gh-ost operations") diff --git a/go/logic/throttler.go b/go/logic/throttler.go index 6113374..d3d184a 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -291,8 +291,10 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- _, err := collectFunc() if err != nil { - // If an error occurs during the HTTP throttle check, let's throttle to be safe - atomic.StoreInt64(&this.migrationContext.ThrottleHTTPStatusCode, int64(-1)) + // If not told to ignore errors, we'll throttle on HTTP connection issues + if !this.migrationContext.IgnoreHTTPErrors { + atomic.StoreInt64(&this.migrationContext.ThrottleHTTPStatusCode, int64(-1)) + } } firstThrottlingCollected <- true From df60fa4204bc7e84a02b5a9b82a19938353149e9 Mon Sep 17 00:00:00 2001 From: Justin Fudally Date: Wed, 25 Mar 2020 16:06:17 -0500 Subject: [PATCH 08/11] add error logging --- go/logic/throttler.go | 1 + 1 file changed, 1 insertion(+) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index d3d184a..dd872e7 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -293,6 +293,7 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- if err != nil { // If not told to ignore errors, we'll throttle on HTTP connection issues if !this.migrationContext.IgnoreHTTPErrors { + log.Errorf("errors occurred during HTTP throttle check: %+v", err) atomic.StoreInt64(&this.migrationContext.ThrottleHTTPStatusCode, int64(-1)) } } From 7893f8e1c3b575523176705de127e790c2a3ca40 Mon Sep 17 00:00:00 2001 From: Justin Fudally Date: Mon, 30 Mar 2020 09:53:10 -0500 Subject: [PATCH 09/11] catch error in collectFunc --- go/logic/throttler.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index dd872e7..ace1e49 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -306,7 +306,16 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- return } - if sleep, _ := collectFunc(); sleep { + sleep, err := collectFunc() + if err != nil { + // If not told to ignore errors, we'll throttle on HTTP connection issues + if !this.migrationContext.IgnoreHTTPErrors { + log.Errorf("errors occurred during HTTP throttle check: %+v", err) + atomic.StoreInt64(&this.migrationContext.ThrottleHTTPStatusCode, int64(-1)) + } + } + + if sleep { time.Sleep(1 * time.Second) } } From 5816ede7b3350d3810878d299dd8341f79b4eb2d Mon Sep 17 00:00:00 2001 From: Justin Fudally Date: Mon, 30 Mar 2020 09:55:49 -0500 Subject: [PATCH 10/11] add error message --- go/logic/throttler.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index ace1e49..be752fe 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -19,20 +19,22 @@ import ( ) var ( - httpStatusMessages map[int]string = map[int]string{ + httpStatusMessages = map[int]string{ 200: "OK", 404: "Not found", 417: "Expectation failed", 429: "Too many requests", 500: "Internal server error", + -1: "Connection error", } // See https://github.com/github/freno/blob/master/doc/http.md - httpStatusFrenoMessages map[int]string = map[int]string{ + httpStatusFrenoMessages = map[int]string{ 200: "OK", 404: "freno: unknown metric", 417: "freno: access forbidden", 429: "freno: threshold exceeded", 500: "freno: internal error", + -1: "freno: connection error", } ) From 2178b5947be80da499fef2c31ac827eaf4920d48 Mon Sep 17 00:00:00 2001 From: Justin Fudally Date: Mon, 30 Mar 2020 10:05:23 -0500 Subject: [PATCH 11/11] remove spammy error --- go/logic/throttler.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index be752fe..2cf0d97 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -25,7 +25,7 @@ var ( 417: "Expectation failed", 429: "Too many requests", 500: "Internal server error", - -1: "Connection error", + -1: "Connection error", } // See https://github.com/github/freno/blob/master/doc/http.md httpStatusFrenoMessages = map[int]string{ @@ -295,7 +295,6 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- if err != nil { // If not told to ignore errors, we'll throttle on HTTP connection issues if !this.migrationContext.IgnoreHTTPErrors { - log.Errorf("errors occurred during HTTP throttle check: %+v", err) atomic.StoreInt64(&this.migrationContext.ThrottleHTTPStatusCode, int64(-1)) } } @@ -312,7 +311,6 @@ func (this *Throttler) collectThrottleHTTPStatus(firstThrottlingCollected chan<- if err != nil { // If not told to ignore errors, we'll throttle on HTTP connection issues if !this.migrationContext.IgnoreHTTPErrors { - log.Errorf("errors occurred during HTTP throttle check: %+v", err) atomic.StoreInt64(&this.migrationContext.ThrottleHTTPStatusCode, int64(-1)) } }