From ab6070a6ce6c8060b9b4bf7ade9cfa93319fec6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Poniedzia=C5=82ek?= Date: Fri, 25 Oct 2024 13:39:45 +0200 Subject: [PATCH] More retrying changes --- .../docs/configuration/overview-full-example.hcl | 4 ++-- assets/docs/configuration/retry-example.hcl | 4 ++-- cmd/cli/cli.go | 16 ++++++++++------ config/config.go | 8 ++++---- config/config_test.go | 8 ++++---- docs/configuration_retry_docs_test.go | 4 ++-- 6 files changed, 24 insertions(+), 20 deletions(-) diff --git a/assets/docs/configuration/overview-full-example.hcl b/assets/docs/configuration/overview-full-example.hcl index 184f7d6b..a8f28749 100644 --- a/assets/docs/configuration/overview-full-example.hcl +++ b/assets/docs/configuration/overview-full-example.hcl @@ -96,11 +96,11 @@ log_level = "info" // Specifies how failed writes to the target should be retried, depending on an error type retry { transient { - delay_sec = 1 + delay_ms = 1000 max_attempts = 5 } setup { - delay_sec = 20 + delay_ms = 20000 } } diff --git a/assets/docs/configuration/retry-example.hcl b/assets/docs/configuration/retry-example.hcl index 34e17e52..2f0d5299 100644 --- a/assets/docs/configuration/retry-example.hcl +++ b/assets/docs/configuration/retry-example.hcl @@ -1,9 +1,9 @@ retry { transient { - delay_sec = 5 + delay_ms = 5000 max_attempts = 10 } setup { - delay_sec = 30 + delay_ms = 30000 } } diff --git a/cmd/cli/cli.go b/cmd/cli/cli.go index be44a5df..6a3d121e 100644 --- a/cmd/cli/cli.go +++ b/cmd/cli/cli.go @@ -290,7 +290,7 @@ func handleWrite(cfg *config.Config, write func() error) error { }) onSetupError := retry.OnRetry(func(attempt uint, err error) { - log.Infof("Setup target write error. Attempt: %d, error: %s\n", attempt+1, err) + log.Warnf("Setup target write error. Attempt: %d, error: %s\n", attempt+1, err) // Here we can set unhealthy status + send monitoring alerts in the future. Nothing happens here now. }) @@ -299,7 +299,7 @@ func handleWrite(cfg *config.Config, write func() error) error { write, retryOnlySetupErrors, onSetupError, - retry.Delay(time.Duration(cfg.Data.Retry.Setup.Delay)*time.Second), + retry.Delay(time.Duration(cfg.Data.Retry.Setup.Delay) * time.Millisecond), // for now let's limit attempts to 5 for setup errors, because we don't have health check which would allow app to be killed externally. Unlimited attempts don't make sense right now. retry.Attempts(5), retry.LastErrorOnly(true), @@ -311,17 +311,21 @@ func handleWrite(cfg *config.Config, write func() error) error { return err } - //If no setup, then handle as transient. We already had at least 1 attempt from above 'setup' retrying section. - log.Infof("Transient target write error. Starting retrying. error: %s\n", err) + // If no setup, then handle as transient. + log.Warnf("Transient target write error. Starting retrying. error: %s\n", err) + + // We already had at least 1 attempt from above 'setup' retrying section, so before we start transient retrying we need add 'manual' initial delay. + time.Sleep(time.Duration(cfg.Data.Retry.Transient.Delay) * time.Millisecond) onTransientError := retry.OnRetry(func(retry uint, err error) { - log.Infof("Retry failed with transient error. Retry counter: %d, error: %s\n", retry+1, err) + log.Warnf("Retry failed with transient error. Retry counter: %d, error: %s\n", retry+1, err) }) err = retry.Do( write, onTransientError, - retry.Delay(time.Duration(cfg.Data.Retry.Transient.Delay)*time.Second), + // * 2 because we have initial sleep above + retry.Delay(time.Duration(cfg.Data.Retry.Transient.Delay*2) * time.Millisecond), retry.Attempts(uint(cfg.Data.Retry.Transient.MaxAttempts)), retry.LastErrorOnly(true), ) diff --git a/config/config.go b/config/config.go index 558ccbfc..d4cd33c3 100644 --- a/config/config.go +++ b/config/config.go @@ -101,12 +101,12 @@ type retryConfig struct { } type transientRetryConfig struct { - Delay int `hcl:"delay_sec,optional"` + Delay int `hcl:"delay_ms,optional"` MaxAttempts int `hcl:"max_attempts,optional"` } type setupRetryConfig struct { - Delay int `hcl:"delay_sec,optional"` + Delay int `hcl:"delay_ms,optional"` } // defaultConfigData returns the initial main configuration target. @@ -135,11 +135,11 @@ func defaultConfigData() *configurationData { }, Retry: &retryConfig{ Transient: &transientRetryConfig{ - Delay: 2, + Delay: 1000, MaxAttempts: 5, }, Setup: &setupRetryConfig{ - Delay: 20, + Delay: 20000, }, }, } diff --git a/config/config_test.go b/config/config_test.go index 22e6e08e..63fccaf3 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -44,9 +44,9 @@ func TestNewConfig_NoConfig(t *testing.T) { assert.Equal(c.Data.LogLevel, "info") assert.Equal(c.Data.DisableTelemetry, false) assert.Equal(c.Data.License.Accept, false) - assert.Equal(2, c.Data.Retry.Transient.Delay) + assert.Equal(1000, c.Data.Retry.Transient.Delay) assert.Equal(5, c.Data.Retry.Transient.MaxAttempts) - assert.Equal(20, c.Data.Retry.Setup.Delay) + assert.Equal(20000, c.Data.Retry.Setup.Delay) } func TestNewConfig_InvalidFailureFormat(t *testing.T) { @@ -176,9 +176,9 @@ func TestNewConfig_Hcl_defaults(t *testing.T) { assert.Equal(1, c.Data.StatsReceiver.TimeoutSec) assert.Equal(15, c.Data.StatsReceiver.BufferSec) assert.Equal("info", c.Data.LogLevel) - assert.Equal(2, c.Data.Retry.Transient.Delay) + assert.Equal(1000, c.Data.Retry.Transient.Delay) assert.Equal(5, c.Data.Retry.Transient.MaxAttempts) - assert.Equal(20, c.Data.Retry.Setup.Delay) + assert.Equal(20000, c.Data.Retry.Setup.Delay) } func TestNewConfig_Hcl_sentry(t *testing.T) { diff --git a/docs/configuration_retry_docs_test.go b/docs/configuration_retry_docs_test.go index a6c8561f..e67ed369 100644 --- a/docs/configuration_retry_docs_test.go +++ b/docs/configuration_retry_docs_test.go @@ -26,7 +26,7 @@ func TestRetryConfigDocumentation(t *testing.T) { retryConfig := c.Data.Retry assert.NotNil(retryConfig) - assert.Equal(5, retryConfig.Transient.Delay) + assert.Equal(5000, retryConfig.Transient.Delay) assert.Equal(10, retryConfig.Transient.MaxAttempts) - assert.Equal(30, retryConfig.Setup.Delay) + assert.Equal(30000, retryConfig.Setup.Delay) }