From b4d77def258e259ea3eb3761d4c5415bcc9d032f Mon Sep 17 00:00:00 2001 From: Leszek Zalewski Date: Wed, 14 Oct 2026 22:27:08 +0200 Subject: [PATCH 1/2] Add repro for unset or tiny report frequencies With a progress or state callback configured and the frequency left at 0 (or set negative), the reporting loop waits on time.After(0) and posts to the callback as fast as it can. --- test/go/config_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/go/config_test.go b/test/go/config_test.go index 50b8d0f1..89a54888 100644 --- a/test/go/config_test.go +++ b/test/go/config_test.go @@ -220,6 +220,28 @@ func (this *ConfigTestSuite) TestBinlogCoordinateModeInvalidIsRejected() { this.Require().EqualError(err, `invalid BinlogCoordinateMode "banana", must be "file_position" or "gtid"`) } +func (this *ConfigTestSuite) TestReportFrequenciesDefaultWhenCallbacksAreSet() { + this.config.ProgressCallback.URI = "http://example.com/progress" + this.config.StateCallback.URI = "http://example.com/state" + err := this.config.ValidateConfig() + this.Require().Nil(err) + this.Require().Equal(60000, this.config.ProgressReportFrequency) + this.Require().Equal(60000, this.config.StateReportFrequency) +} + +func (this *ConfigTestSuite) TestReportFrequenciesBelowMinimumAreRejected() { + this.config.ProgressCallback.URI = "http://example.com/progress" + this.config.ProgressReportFrequency = -1 + err := this.config.ValidateConfig() + this.Require().EqualError(err, "ProgressReportFrequency must be at least 100 milliseconds, got -1") + + this.SetupTest() + this.config.StateCallback.URI = "http://example.com/state" + this.config.StateReportFrequency = 50 + err = this.config.ValidateConfig() + this.Require().EqualError(err, "StateReportFrequency must be at least 100 milliseconds, got 50") +} + func TestConfig(t *testing.T) { testhelpers.SetupTest() suite.Run(t, new(ConfigTestSuite)) From ecbe8a5955bffefd6326d011912e70d37c9d72f7 Mon Sep 17 00:00:00 2001 From: Leszek Zalewski Date: Wed, 14 Oct 2026 22:29:54 +0200 Subject: [PATCH 2/2] Default report frequencies and reject values below 100ms ProgressReportFrequency and StateReportFrequency now default to 60000ms like other optional settings, and ValidateConfig rejects values below 100ms when the matching callback is configured. Fixes #214. --- CHANGELOG.md | 1 + config.go | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a05bf0a4..cdad7282 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ All notable changes to this project will be documented in this file. - Finish replayed GTID transactions before stopping at cutover, even when their GTIDs were already streamed before a reconnect. - Exclude the current GTID from serialized resume checkpoints during reconnect replay, preserving other committed GTIDs. Stop streaming on invalid GTID identities before delivering further rows. - Publish binlog stop requests atomically after recording the stop coordinate, including reads from the control server. +- Default `ProgressReportFrequency` and `StateReportFrequency` to 60000 ms, and reject values below 100 ms when the matching callback is set. An unset, zero or negative frequency used to make Ghostferry post to the callback in a tight loop ([#214](https://github.com/Shopify/ghostferry/issues/214)). ## [1.3.1 - 2026-04-15] diff --git a/config.go b/config.go index 4cf51d0e..db469d42 100644 --- a/config.go +++ b/config.go @@ -23,6 +23,9 @@ const ( DefaultNet = "tcp" DefaultMarginalia = "application:ghostferry" MySQLNumParamsLimit = 1<<16 - 1 // see num_params https://dev.mysql.com/doc/internals/en/com-stmt-prepare-response.html + + defaultReportFrequencyMs = 60000 + minReportFrequencyMs = 100 ) type TLSConfig struct { @@ -634,12 +637,18 @@ type Config struct { // Report progress via an HTTP callback. The Payload field of the callback // will be sent to the server as the CustomPayload field in the Progress // struct. The unit of ProgressReportFrequency is in milliseconds. + // + // Optional: ProgressReportFrequency defaults to 60000 and must be at least + // 100 when ProgressCallback is set. ProgressCallback HTTPCallback ProgressReportFrequency int // Report state via an HTTP callback. The serialized SerializableState // (the same JSON as the state dump) will be sent as the Payload parameter. // The unit of StateReportFrequency is in milliseconds. + // + // Optional: StateReportFrequency defaults to 60000 and must be at least + // 100 when StateCallback is set. StateCallback HTTPCallback StateReportFrequency int @@ -943,6 +952,20 @@ func (c *Config) ValidateConfig() error { c.CutoverRetryWaitSeconds = 1 } + if c.ProgressReportFrequency == 0 { + c.ProgressReportFrequency = defaultReportFrequencyMs + } + if c.ProgressCallback.URI != "" && c.ProgressReportFrequency < minReportFrequencyMs { + return fmt.Errorf("ProgressReportFrequency must be at least %d milliseconds, got %d", minReportFrequencyMs, c.ProgressReportFrequency) + } + + if c.StateReportFrequency == 0 { + c.StateReportFrequency = defaultReportFrequencyMs + } + if c.StateCallback.URI != "" && c.StateReportFrequency < minReportFrequencyMs { + return fmt.Errorf("StateReportFrequency must be at least %d milliseconds, got %d", minReportFrequencyMs, c.StateReportFrequency) + } + return nil }