diff options
author | James Fargher <jfargher@gitlab.com> | 2022-06-21 02:30:08 +0300 |
---|---|---|
committer | Will Chandler <wchandler@gitlab.com> | 2022-08-20 06:42:45 +0300 |
commit | cb0e4c4d751ebbe3a18bddc69aa2e7792b9415aa (patch) | |
tree | 8145062e1e118ded7d11119b8550d731015ef962 /internal | |
parent | 109499970cae4fa648d9c0119bb708f0dd65f1d4 (diff) |
Extract Duration into its own package
go-toml v2 forces us to use `TextMarshaller` and this means we now need
the Duration wrapper in several packages. To prevent import loops, this
requires moving the type to its own package.
Diffstat (limited to 'internal')
20 files changed, 156 insertions, 101 deletions
diff --git a/internal/blackbox/blackbox.go b/internal/blackbox/blackbox.go index f793fdb0a..38cfa8b6d 100644 --- a/internal/blackbox/blackbox.go +++ b/internal/blackbox/blackbox.go @@ -138,7 +138,7 @@ func (b Blackbox) Run() error { } func (b Blackbox) runProbes() { - for ; ; time.Sleep(b.cfg.sleepDuration) { + for ; ; time.Sleep(b.cfg.sleepDuration.Duration()) { for _, probe := range b.cfg.Probes { entry := log.Default().WithFields(map[string]interface{}{ "probe": probe.Name, diff --git a/internal/blackbox/config.go b/internal/blackbox/config.go index 4d8d373a5..c9e5966ed 100644 --- a/internal/blackbox/config.go +++ b/internal/blackbox/config.go @@ -7,6 +7,7 @@ import ( "github.com/pelletier/go-toml" logconfig "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/log" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/duration" ) // Config is the configuration for gitaly-blackbox. @@ -17,7 +18,7 @@ type Config struct { // Sleep is the number of seconds between probe runs. Sleep int `toml:"sleep"` // sleepDuration is the same as Sleep but converted to a proper duration. - sleepDuration time.Duration + sleepDuration duration.Duration // Logging configures logging. Logging logconfig.Config `toml:"logging"` // Probes defines endpoints to probe. At least one probe must be defined. @@ -92,7 +93,7 @@ func ParseConfig(raw string) (Config, error) { if config.Sleep == 0 { config.Sleep = 15 * 60 } - config.sleepDuration = time.Duration(config.Sleep) * time.Second + config.sleepDuration = duration.Duration(config.Sleep) * duration.Duration(time.Second) if len(config.Probes) == 0 { return Config{}, fmt.Errorf("must define at least one probe") diff --git a/internal/blackbox/config_test.go b/internal/blackbox/config_test.go index bdf80e165..6be4c80cc 100644 --- a/internal/blackbox/config_test.go +++ b/internal/blackbox/config_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/duration" ) func TestConfigParseFailures(t *testing.T) { @@ -77,16 +78,16 @@ func TestConfigSleep(t *testing.T) { testCases := []struct { desc string in string - out time.Duration + out duration.Duration }{ { desc: "default sleep time", - out: 15 * time.Minute, + out: duration.Duration(15 * time.Minute), }, { desc: "1 second", in: "sleep = 1\n", - out: time.Second, + out: duration.Duration(time.Second), }, } diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go index c8988f77e..8378933a6 100644 --- a/internal/gitaly/config/config.go +++ b/internal/gitaly/config/config.go @@ -20,6 +20,7 @@ import ( internallog "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/log" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/prometheus" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/sentry" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/duration" ) const ( @@ -32,10 +33,10 @@ const ( // DailyJob enables a daily task to be scheduled for specific storages type DailyJob struct { - Hour uint `toml:"start_hour"` - Minute uint `toml:"start_minute"` - Duration time.Duration `toml:"duration"` - Storages []string `toml:"storages"` + Hour uint `toml:"start_hour"` + Minute uint `toml:"start_minute"` + Duration duration.Duration `toml:"duration"` + Storages []string `toml:"storages"` // Disabled will completely disable a daily job, even in cases where a // default schedule is implied @@ -62,7 +63,7 @@ type Cfg struct { Hooks Hooks `toml:"hooks"` Concurrency []Concurrency `toml:"concurrency"` RateLimiting []RateLimiting `toml:"rate_limiting"` - GracefulRestartTimeout time.Duration `toml:"graceful_restart_timeout"` + GracefulRestartTimeout duration.Duration `toml:"graceful_restart_timeout"` DailyMaintenance DailyJob `toml:"daily_maintenance"` Cgroups cgroups.Config `toml:"cgroups"` PackObjectsCache StreamCacheConfig `toml:"pack_objects_cache"` @@ -147,7 +148,7 @@ type Concurrency struct { MaxQueueSize int `toml:"max_queue_size"` // MaxQueueWait is the maximum time a request can remain in the concurrency queue // waiting to be picked up by Gitaly - MaxQueueWait time.Duration `toml:"max_queue_wait"` + MaxQueueWait duration.Duration `toml:"max_queue_wait"` } // RateLimiting allows endpoints to be limited to a maximum request rate per @@ -161,16 +162,16 @@ type RateLimiting struct { RPC string `toml:"rpc"` // Interval sets the interval with which the token bucket will // be refilled to what is configured in Burst. - Interval time.Duration `toml:"interval"` + Interval duration.Duration `toml:"interval"` // Burst sets the capacity of the token bucket (see above). Burst int `toml:"burst"` } // StreamCacheConfig contains settings for a streamcache instance. type StreamCacheConfig struct { - Enabled bool `toml:"enabled"` // Default: false - Dir string `toml:"dir"` // Default: <FIRST STORAGE PATH>/+gitaly/PackObjectsCache - MaxAge time.Duration `toml:"max_age"` // Default: 5m + Enabled bool `toml:"enabled"` // Default: false + Dir string `toml:"dir"` // Default: <FIRST STORAGE PATH>/+gitaly/PackObjectsCache + MaxAge duration.Duration `toml:"max_age"` // Default: 5m } // Load initializes the Config variable from file and the environment. @@ -219,8 +220,8 @@ func (cfg *Cfg) Validate() error { } func (cfg *Cfg) setDefaults() error { - if cfg.GracefulRestartTimeout == 0 { - cfg.GracefulRestartTimeout = time.Minute + if cfg.GracefulRestartTimeout.Duration() == 0 { + cfg.GracefulRestartTimeout = duration.Duration(time.Minute) } if cfg.Gitlab.SecretFile == "" { @@ -462,7 +463,7 @@ func defaultMaintenanceWindow(storages []Storage) DailyJob { return DailyJob{ Hour: 12, Minute: 0, - Duration: 10 * time.Minute, + Duration: duration.Duration(10 * time.Minute), Storages: storageNames, } } @@ -486,8 +487,8 @@ func (cfg *Cfg) validateMaintenance() error { if dm.Minute > 59 { return fmt.Errorf("daily maintenance specified minute '%d' outside range (0-59)", dm.Minute) } - if dm.Duration > 24*time.Hour { - return fmt.Errorf("daily maintenance specified duration %s must be less than 24 hours", dm.Duration) + if dm.Duration.Duration() > 24*time.Hour { + return fmt.Errorf("daily maintenance specified duration %s must be less than 24 hours", dm.Duration.Duration()) } return nil @@ -524,7 +525,7 @@ func (cfg *Cfg) configurePackObjectsCache() error { } if poc.MaxAge == 0 { - poc.MaxAge = 5 * time.Minute + poc.MaxAge = duration.Duration(5 * time.Minute) } if poc.Dir == "" { diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go index 0bc386636..fb93676f8 100644 --- a/internal/gitaly/config/config_test.go +++ b/internal/gitaly/config/config_test.go @@ -19,6 +19,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/cgroups" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/prometheus" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/sentry" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/duration" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" ) @@ -158,7 +159,7 @@ func TestLoadPrometheus(t *testing.T) { require.Equal(t, ":9236", cfg.PrometheusListenAddr) require.Equal(t, prometheus.Config{ - ScrapeTimeout: time.Second, + ScrapeTimeout: duration.Duration(time.Second), GRPCLatencyBuckets: []float64{0, 1, 2}, }, cfg.Prometheus) } @@ -576,16 +577,16 @@ func TestLoadGracefulRestartTimeout(t *testing.T) { tests := []struct { name string config string - expected time.Duration + expected duration.Duration }{ { name: "default value", - expected: 1 * time.Minute, + expected: duration.Duration(1 * time.Minute), }, { name: "8m03s", config: `graceful_restart_timeout = "8m03s"`, - expected: 8*time.Minute + 3*time.Second, + expected: duration.Duration(8*time.Minute + 3*time.Second), }, } for _, test := range tests { @@ -715,7 +716,7 @@ func TestLoadDailyMaintenance(t *testing.T) { expect: DailyJob{ Hour: 11, Minute: 23, - Duration: 45 * time.Minute, + Duration: duration.Duration(45 * time.Minute), Storages: []string{"default"}, }, }, @@ -753,7 +754,7 @@ func TestLoadDailyMaintenance(t *testing.T) { expect: DailyJob{ Hour: 0, Minute: 59, - Duration: 24*time.Hour + time.Second, + Duration: duration.Duration(24*time.Hour + time.Second), }, validateErr: errors.New("daily maintenance specified duration 24h0m1s must be less than 24 hours"), }, @@ -761,7 +762,7 @@ func TestLoadDailyMaintenance(t *testing.T) { rawCfg: `[daily_maintenance] duration = "meow"`, expect: DailyJob{}, - loadErr: errors.New(`load toml: (2, 4): Can't convert meow(string) to time.Duration. time: invalid duration "meow"`), + loadErr: errors.New("load toml: toml: time: invalid duration \"meow\""), }, { rawCfg: `[daily_maintenance] @@ -780,7 +781,7 @@ func TestLoadDailyMaintenance(t *testing.T) { expect: DailyJob{ Hour: 12, Minute: 0, - Duration: 10 * time.Minute, + Duration: duration.Duration(10 * time.Minute), Storages: []string{"default"}, }, }, @@ -1127,7 +1128,7 @@ path="/foobar" in: storageConfig + `[pack_objects_cache] enabled = true `, - out: StreamCacheConfig{Enabled: true, MaxAge: 5 * time.Minute, Dir: "/foobar/+gitaly/PackObjectsCache"}, + out: StreamCacheConfig{Enabled: true, MaxAge: duration.Duration(5 * time.Minute), Dir: "/foobar/+gitaly/PackObjectsCache"}, }, { desc: "enabled with custom values", @@ -1136,7 +1137,7 @@ enabled = true dir = "/bazqux" max_age = "10m" `, - out: StreamCacheConfig{Enabled: true, MaxAge: 10 * time.Minute, Dir: "/bazqux"}, + out: StreamCacheConfig{Enabled: true, MaxAge: duration.Duration(10 * time.Minute), Dir: "/bazqux"}, }, { desc: "enabled with 0 storages", diff --git a/internal/gitaly/config/prometheus/config.go b/internal/gitaly/config/prometheus/config.go index 4873eec1d..12fbeec09 100644 --- a/internal/gitaly/config/prometheus/config.go +++ b/internal/gitaly/config/prometheus/config.go @@ -6,12 +6,13 @@ import ( grpcprometheus "github.com/grpc-ecosystem/go-grpc-prometheus" "github.com/prometheus/client_golang/prometheus" log "github.com/sirupsen/logrus" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/duration" ) // Config contains additional configuration data for prometheus type Config struct { // ScrapeTimeout is the allowed duration of a Prometheus scrape before timing out. - ScrapeTimeout time.Duration `toml:"scrape_timeout,omitempty"` + ScrapeTimeout duration.Duration `toml:"scrape_timeout,omitempty"` // GRPCLatencyBuckets configures the histogram buckets used for gRPC // latency measurements. GRPCLatencyBuckets []float64 `toml:"grpc_latency_buckets,omitempty"` @@ -20,7 +21,7 @@ type Config struct { // DefaultConfig returns a new config with default values set. func DefaultConfig() Config { return Config{ - ScrapeTimeout: 10 * time.Second, + ScrapeTimeout: duration.Duration(10 * time.Second), GRPCLatencyBuckets: []float64{0.001, 0.005, 0.025, 0.1, 0.5, 1.0, 10.0, 30.0, 60.0, 300.0, 1500.0}, } } diff --git a/internal/gitaly/config/ruby.go b/internal/gitaly/config/ruby.go index 0392a31d7..92fa70c60 100644 --- a/internal/gitaly/config/ruby.go +++ b/internal/gitaly/config/ruby.go @@ -4,31 +4,33 @@ import ( "fmt" "path/filepath" "time" + + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/duration" ) // Ruby contains setting for Ruby worker processes type Ruby struct { - Dir string `toml:"dir"` - MaxRSS int `toml:"max_rss"` - GracefulRestartTimeout time.Duration `toml:"graceful_restart_timeout"` - RestartDelay time.Duration `toml:"restart_delay"` - NumWorkers int `toml:"num_workers"` - LinguistLanguagesPath string `toml:"linguist_languages_path"` - RuggedGitConfigSearchPath string `toml:"rugged_git_config_search_path"` + Dir string `toml:"dir"` + MaxRSS int `toml:"max_rss"` + GracefulRestartTimeout duration.Duration `toml:"graceful_restart_timeout"` + RestartDelay duration.Duration `toml:"restart_delay"` + NumWorkers int `toml:"num_workers"` + LinguistLanguagesPath string `toml:"linguist_languages_path"` + RuggedGitConfigSearchPath string `toml:"rugged_git_config_search_path"` } // ConfigureRuby validates the gitaly-ruby configuration and sets default values. func (cfg *Cfg) ConfigureRuby() error { - if cfg.Ruby.GracefulRestartTimeout == 0 { - cfg.Ruby.GracefulRestartTimeout = 10 * time.Minute + if cfg.Ruby.GracefulRestartTimeout.Duration() == 0 { + cfg.Ruby.GracefulRestartTimeout = duration.Duration(10 * time.Minute) } if cfg.Ruby.MaxRSS == 0 { cfg.Ruby.MaxRSS = 200 * 1024 * 1024 } - if cfg.Ruby.RestartDelay == 0 { - cfg.Ruby.RestartDelay = 5 * time.Minute + if cfg.Ruby.RestartDelay.Duration() == 0 { + cfg.Ruby.RestartDelay = duration.Duration(5 * time.Minute) } if len(cfg.Ruby.Dir) == 0 { diff --git a/internal/gitaly/maintenance/daily.go b/internal/gitaly/maintenance/daily.go index 93f00df2f..c99eeb547 100644 --- a/internal/gitaly/maintenance/daily.go +++ b/internal/gitaly/maintenance/daily.go @@ -61,7 +61,7 @@ func (dw DailyWorker) StartDaily(ctx context.Context, l logrus.FieldLogger, sche var jobErr error dontpanic.Try(func() { - ctx, cancel := context.WithTimeout(ctx, schedule.Duration) + ctx, cancel := context.WithTimeout(ctx, schedule.Duration.Duration()) defer cancel() jobErr = job(ctx, l, schedule.Storages) diff --git a/internal/gitaly/maintenance/daily_test.go b/internal/gitaly/maintenance/daily_test.go index ef43fbd54..689052e67 100644 --- a/internal/gitaly/maintenance/daily_test.go +++ b/internal/gitaly/maintenance/daily_test.go @@ -10,6 +10,7 @@ import ( "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/duration" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" ) @@ -37,7 +38,7 @@ func TestStartDaily(t *testing.T) { errQ := make(chan error) s := config.DailyJob{ Hour: 1, - Duration: time.Hour, + Duration: duration.Duration(time.Hour), Storages: []string{"meow"}, } ctx, cancel := context.WithCancel(testhelper.Context(t)) diff --git a/internal/gitaly/rubyserver/rubyserver.go b/internal/gitaly/rubyserver/rubyserver.go index 6bda8bdae..76c901a84 100644 --- a/internal/gitaly/rubyserver/rubyserver.go +++ b/internal/gitaly/rubyserver/rubyserver.go @@ -200,8 +200,8 @@ func (s *Server) start() error { return err } - restartDelay := cfg.Ruby.RestartDelay - gracefulRestartTimeout := cfg.Ruby.GracefulRestartTimeout + restartDelay := cfg.Ruby.RestartDelay.Duration() + gracefulRestartTimeout := cfg.Ruby.GracefulRestartTimeout.Duration() s.workers = append(s.workers, newWorker(p, socketPath, restartDelay, gracefulRestartTimeout, events, false)) } diff --git a/internal/helper/duration/duration.go b/internal/helper/duration/duration.go new file mode 100644 index 000000000..5d1e5b87d --- /dev/null +++ b/internal/helper/duration/duration.go @@ -0,0 +1,28 @@ +package duration + +import "time" + +// Duration is a trick to let our TOML library parse durations from strings. +type Duration time.Duration + +// Duration converts the duration.Duration to a time.Duration +func (d *Duration) Duration() time.Duration { + if d != nil { + return time.Duration(*d) + } + return 0 +} + +// UnmarshalText implements the encoding.TextUnmarshaler interface +func (d *Duration) UnmarshalText(text []byte) error { + td, err := time.ParseDuration(string(text)) + if err == nil { + *d = Duration(td) + } + return err +} + +// MarshalText implements the encoding.TextMarshaler interface +func (d Duration) MarshalText() ([]byte, error) { + return []byte(time.Duration(d).String()), nil +} diff --git a/internal/helper/duration/duration_test.go b/internal/helper/duration/duration_test.go new file mode 100644 index 000000000..9ca1830dc --- /dev/null +++ b/internal/helper/duration/duration_test.go @@ -0,0 +1,15 @@ +package duration + +import ( + "encoding" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDuration(t *testing.T) { + t.Parallel() + + require.Implements(t, (*encoding.TextMarshaler)(nil), new(Duration)) + require.Implements(t, (*encoding.TextUnmarshaler)(nil), new(Duration)) +} diff --git a/internal/middleware/limithandler/concurrency_limiter.go b/internal/middleware/limithandler/concurrency_limiter.go index cf552917d..e2595571b 100644 --- a/internal/middleware/limithandler/concurrency_limiter.go +++ b/internal/middleware/limithandler/concurrency_limiter.go @@ -276,7 +276,7 @@ func WithConcurrencyLimiters(cfg config.Cfg, middleware *LimiterMiddleware) { if limit.MaxQueueWait > 0 { limit := limit newTickerFunc = func() helper.Ticker { - return helper.NewTimerTicker(limit.MaxQueueWait) + return helper.NewTimerTicker(limit.MaxQueueWait.Duration()) } } diff --git a/internal/middleware/limithandler/middleware_test.go b/internal/middleware/limithandler/middleware_test.go index 9a2fb9870..d5aa4ee57 100644 --- a/internal/middleware/limithandler/middleware_test.go +++ b/internal/middleware/limithandler/middleware_test.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/duration" "gitlab.com/gitlab-org/gitaly/v15/internal/middleware/limithandler" pb "gitlab.com/gitlab-org/gitaly/v15/internal/middleware/limithandler/testdata" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" @@ -487,7 +488,7 @@ func TestRateLimitHandler(t *testing.T) { methodName := "/test.limithandler.Test/Unary" cfg := config.Cfg{ RateLimiting: []config.RateLimiting{ - {RPC: methodName, Interval: 1 * time.Hour, Burst: 1}, + {RPC: methodName, Interval: duration.Duration(1 * time.Hour), Burst: 1}, }, } diff --git a/internal/middleware/limithandler/rate_limiter.go b/internal/middleware/limithandler/rate_limiter.go index 89f6deeab..5f2f2d9f6 100644 --- a/internal/middleware/limithandler/rate_limiter.go +++ b/internal/middleware/limithandler/rate_limiter.go @@ -119,7 +119,7 @@ func WithRateLimiters(ctx context.Context) SetupFunc { if limitCfg.Burst > 0 && limitCfg.Interval > 0 { serviceName, methodName := splitMethodName(limitCfg.RPC) rateLimiter := NewRateLimiter( - limitCfg.Interval, + limitCfg.Interval.Duration(), limitCfg.Burst, helper.NewTimerTicker(5*time.Minute), middleware.requestsDroppedMetric.With(prometheus.Labels{ diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go index 59a0f2be2..6a053f0ec 100644 --- a/internal/praefect/config/config.go +++ b/internal/praefect/config/config.go @@ -13,6 +13,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/log" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/prometheus" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/sentry" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/duration" ) // ElectionStrategy is a Praefect primary election strategy. @@ -44,16 +45,16 @@ const ( type Failover struct { Enabled bool `toml:"enabled,omitempty"` // ElectionStrategy is the strategy to use for electing primaries nodes. - ElectionStrategy ElectionStrategy `toml:"election_strategy,omitempty"` - ErrorThresholdWindow time.Duration `toml:"error_threshold_window,omitempty"` - WriteErrorThresholdCount uint32 `toml:"write_error_threshold_count,omitempty"` - ReadErrorThresholdCount uint32 `toml:"read_error_threshold_count,omitempty"` + ElectionStrategy ElectionStrategy `toml:"election_strategy,omitempty"` + ErrorThresholdWindow duration.Duration `toml:"error_threshold_window,omitempty"` + WriteErrorThresholdCount uint32 `toml:"write_error_threshold_count,omitempty"` + ReadErrorThresholdCount uint32 `toml:"read_error_threshold_count,omitempty"` // BootstrapInterval allows set a time duration that would be used on startup to make initial health check. // The default value is 1s. - BootstrapInterval time.Duration `toml:"bootstrap_interval,omitempty"` + BootstrapInterval duration.Duration `toml:"bootstrap_interval,omitempty"` // MonitorInterval allows set a time duration that would be used after bootstrap is completed to execute health checks. // The default value is 3s. - MonitorInterval time.Duration `toml:"monitor_interval,omitempty"` + MonitorInterval duration.Duration `toml:"monitor_interval,omitempty"` } // ErrorThresholdsConfigured checks whether returns whether the errors thresholds are configured. If they @@ -82,7 +83,7 @@ func (f Failover) ErrorThresholdsConfigured() (bool, error) { type BackgroundVerification struct { // VerificationInterval determines the duration after a replica due for reverification. // The feature is disabled if verification interval is 0 or below. - VerificationInterval time.Duration `toml:"verification_interval,omitempty"` + VerificationInterval duration.Duration `toml:"verification_interval,omitempty"` // DeleteInvalidRecords controls whether the background verifier will actually delete the metadata // records that point to non-existent replicas. DeleteInvalidRecords bool `toml:"delete_invalid_records"` @@ -90,14 +91,14 @@ type BackgroundVerification struct { // DefaultBackgroundVerificationConfig returns the default background verification configuration. func DefaultBackgroundVerificationConfig() BackgroundVerification { - return BackgroundVerification{VerificationInterval: 7 * 24 * time.Hour} + return BackgroundVerification{VerificationInterval: duration.Duration(7 * 24 * time.Hour)} } // Reconciliation contains reconciliation specific configuration options. type Reconciliation struct { // SchedulingInterval the interval between each automatic reconciliation run. If set to 0, // automatic reconciliation is disabled. - SchedulingInterval time.Duration `toml:"scheduling_interval,omitempty"` + SchedulingInterval duration.Duration `toml:"scheduling_interval,omitempty"` // HistogramBuckets configures the reconciliation scheduling duration histogram's buckets. HistogramBuckets []float64 `toml:"histogram_buckets,omitempty"` } @@ -105,7 +106,7 @@ type Reconciliation struct { // DefaultReconciliationConfig returns the default values for reconciliation configuration. func DefaultReconciliationConfig() Reconciliation { return Reconciliation{ - SchedulingInterval: 5 * time.Minute, + SchedulingInterval: 5 * duration.Duration(time.Minute), HistogramBuckets: promclient.DefBuckets, } } @@ -152,7 +153,7 @@ type Config struct { // Keep for legacy reasons: remove after Omnibus has switched FailoverEnabled bool `toml:"failover_enabled,omitempty"` MemoryQueueEnabled bool `toml:"memory_queue_enabled,omitempty"` - GracefulStopTimeout time.Duration `toml:"graceful_stop_timeout,omitempty"` + GracefulStopTimeout duration.Duration `toml:"graceful_stop_timeout,omitempty"` RepositoriesCleanup RepositoriesCleanup `toml:"repositories_cleanup,omitempty"` } @@ -276,11 +277,11 @@ func (c *Config) Validate() error { } } - if c.RepositoriesCleanup.RunInterval > 0 { - if c.RepositoriesCleanup.CheckInterval < minimalSyncCheckInterval { + if c.RepositoriesCleanup.RunInterval.Duration() > 0 { + if c.RepositoriesCleanup.CheckInterval.Duration() < minimalSyncCheckInterval { return fmt.Errorf("repositories_cleanup.check_interval is less then %s, which could lead to a database performance problem", minimalSyncCheckInterval.String()) } - if c.RepositoriesCleanup.RunInterval < minimalSyncRunInterval { + if c.RepositoriesCleanup.RunInterval.Duration() < minimalSyncRunInterval { return fmt.Errorf("repositories_cleanup.run_interval is less then %s, which could lead to a database performance problem", minimalSyncRunInterval.String()) } } @@ -294,17 +295,17 @@ func (c *Config) NeedsSQL() bool { } func (c *Config) setDefaults() { - if c.GracefulStopTimeout == 0 { - c.GracefulStopTimeout = time.Minute + if c.GracefulStopTimeout.Duration() == 0 { + c.GracefulStopTimeout = duration.Duration(time.Minute) } if c.Failover.Enabled { - if c.Failover.BootstrapInterval == 0 { - c.Failover.BootstrapInterval = time.Second + if c.Failover.BootstrapInterval.Duration() == 0 { + c.Failover.BootstrapInterval = duration.Duration(time.Second) } - if c.Failover.MonitorInterval == 0 { - c.Failover.MonitorInterval = 3 * time.Second + if c.Failover.MonitorInterval.Duration() == 0 { + c.Failover.MonitorInterval = duration.Duration(3 * time.Second) } } } @@ -383,9 +384,9 @@ type RepositoriesCleanup struct { // CheckInterval is a time period used to check if operation should be executed. // It is recommended to keep it less than run_interval configuration as some // nodes may be out of service, so they can be stale for too long. - CheckInterval time.Duration `toml:"check_interval,omitempty"` + CheckInterval duration.Duration `toml:"check_interval,omitempty"` // RunInterval: the check runs if the previous operation was done at least RunInterval before. - RunInterval time.Duration `toml:"run_interval,omitempty"` + RunInterval duration.Duration `toml:"run_interval,omitempty"` // RepositoriesInBatch is the number of repositories to pass as a batch for processing. RepositoriesInBatch int `toml:"repositories_in_batch,omitempty"` } @@ -393,8 +394,8 @@ type RepositoriesCleanup struct { // DefaultRepositoriesCleanup contains default configuration values for the RepositoriesCleanup. func DefaultRepositoriesCleanup() RepositoriesCleanup { return RepositoriesCleanup{ - CheckInterval: 30 * time.Minute, - RunInterval: 24 * time.Hour, + CheckInterval: duration.Duration(30 * time.Minute), + RunInterval: duration.Duration(24 * time.Hour), RepositoriesInBatch: 16, } } diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go index 410599d33..503430dda 100644 --- a/internal/praefect/config/config_test.go +++ b/internal/praefect/config/config_test.go @@ -15,6 +15,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/log" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/prometheus" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/sentry" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/duration" ) func TestConfigValidation(t *testing.T) { @@ -205,14 +206,14 @@ func TestConfigValidation(t *testing.T) { { desc: "repositories_cleanup minimal duration is too low", changeConfig: func(cfg *Config) { - cfg.RepositoriesCleanup.CheckInterval = minimalSyncCheckInterval - time.Nanosecond + cfg.RepositoriesCleanup.CheckInterval = duration.Duration(minimalSyncCheckInterval - time.Nanosecond) }, errMsg: `repositories_cleanup.check_interval is less then 1m0s, which could lead to a database performance problem`, }, { desc: "repositories_cleanup minimal duration is too low", changeConfig: func(cfg *Config) { - cfg.RepositoriesCleanup.RunInterval = minimalSyncRunInterval - time.Nanosecond + cfg.RepositoriesCleanup.RunInterval = duration.Duration(minimalSyncRunInterval - time.Nanosecond) }, errMsg: `repositories_cleanup.run_interval is less then 1m0s, which could lead to a database performance problem`, }, @@ -290,7 +291,7 @@ func TestConfigParsing(t *testing.T) { }, }, Prometheus: prometheus.Config{ - ScrapeTimeout: time.Second, + ScrapeTimeout: duration.Duration(time.Second), GRPCLatencyBuckets: []float64{0.1, 0.2, 0.3}, }, PrometheusExcludeDatabaseFromDefaultMetrics: true, @@ -317,28 +318,28 @@ func TestConfigParsing(t *testing.T) { }, }, MemoryQueueEnabled: true, - GracefulStopTimeout: 30 * time.Second, + GracefulStopTimeout: duration.Duration(30 * time.Second), Reconciliation: Reconciliation{ - SchedulingInterval: time.Minute, + SchedulingInterval: duration.Duration(time.Minute), HistogramBuckets: []float64{1, 2, 3, 4, 5}, }, Replication: Replication{BatchSize: 1, ParallelStorageProcessingWorkers: 2}, Failover: Failover{ Enabled: true, ElectionStrategy: ElectionStrategyPerRepository, - ErrorThresholdWindow: 20 * time.Second, + ErrorThresholdWindow: duration.Duration(20 * time.Second), WriteErrorThresholdCount: 1500, ReadErrorThresholdCount: 100, - BootstrapInterval: 1 * time.Second, - MonitorInterval: 3 * time.Second, + BootstrapInterval: duration.Duration(1 * time.Second), + MonitorInterval: duration.Duration(3 * time.Second), }, RepositoriesCleanup: RepositoriesCleanup{ - CheckInterval: time.Second, - RunInterval: 3 * time.Second, + CheckInterval: duration.Duration(time.Second), + RunInterval: duration.Duration(3 * time.Second), RepositoriesInBatch: 10, }, BackgroundVerification: BackgroundVerification{ - VerificationInterval: 24 * time.Hour, + VerificationInterval: duration.Duration(24 * time.Hour), DeleteInvalidRecords: true, }, }, @@ -347,7 +348,7 @@ func TestConfigParsing(t *testing.T) { desc: "overwriting default values in the config", filePath: "testdata/config.overwritedefaults.toml", expected: Config{ - GracefulStopTimeout: time.Minute, + GracefulStopTimeout: duration.Duration(time.Minute), Reconciliation: Reconciliation{ SchedulingInterval: 0, HistogramBuckets: []float64{1, 2, 3, 4, 5}, @@ -358,12 +359,12 @@ func TestConfigParsing(t *testing.T) { Failover: Failover{ Enabled: false, ElectionStrategy: "local", - BootstrapInterval: 5 * time.Second, - MonitorInterval: 10 * time.Second, + BootstrapInterval: duration.Duration(5 * time.Second), + MonitorInterval: duration.Duration(10 * time.Second), }, RepositoriesCleanup: RepositoriesCleanup{ - CheckInterval: time.Second, - RunInterval: 4 * time.Second, + CheckInterval: duration.Duration(time.Second), + RunInterval: duration.Duration(4 * time.Second), RepositoriesInBatch: 11, }, BackgroundVerification: DefaultBackgroundVerificationConfig(), @@ -373,7 +374,7 @@ func TestConfigParsing(t *testing.T) { desc: "empty config yields default values", filePath: "testdata/config.empty.toml", expected: Config{ - GracefulStopTimeout: time.Minute, + GracefulStopTimeout: duration.Duration(time.Minute), Prometheus: prometheus.DefaultConfig(), PrometheusExcludeDatabaseFromDefaultMetrics: true, Reconciliation: DefaultReconciliationConfig(), @@ -381,12 +382,12 @@ func TestConfigParsing(t *testing.T) { Failover: Failover{ Enabled: true, ElectionStrategy: ElectionStrategyPerRepository, - BootstrapInterval: time.Second, - MonitorInterval: 3 * time.Second, + BootstrapInterval: duration.Duration(time.Second), + MonitorInterval: duration.Duration(3 * time.Second), }, RepositoriesCleanup: RepositoriesCleanup{ - CheckInterval: 30 * time.Minute, - RunInterval: 24 * time.Hour, + CheckInterval: duration.Duration(30 * time.Minute), + RunInterval: duration.Duration(24 * time.Hour), RepositoriesInBatch: 16, }, BackgroundVerification: DefaultBackgroundVerificationConfig(), diff --git a/internal/praefect/nodes/tracker/errors.go b/internal/praefect/nodes/tracker/errors.go index 1fce839bf..8a87d30cc 100644 --- a/internal/praefect/nodes/tracker/errors.go +++ b/internal/praefect/nodes/tracker/errors.go @@ -32,7 +32,7 @@ func NewErrorWindowFunction(cfg config.Failover) (ErrorWindowFunction, error) { return nil, errors.New("errorWindow must be non zero") } - errorWindow := cfg.ErrorThresholdWindow + errorWindow := cfg.ErrorThresholdWindow.Duration() return func(now time.Time, errorTime time.Time) bool { return errorTime.After(now.Add(-errorWindow)) }, nil diff --git a/internal/streamcache/cache.go b/internal/streamcache/cache.go index 53c3c6c44..333be63c7 100644 --- a/internal/streamcache/cache.go +++ b/internal/streamcache/cache.go @@ -145,10 +145,10 @@ func New(cfg config.StreamCacheConfig, logger logrus.FieldLogger) Cache { if cfg.Enabled { packObjectsCacheEnabled.WithLabelValues( cfg.Dir, - strconv.Itoa(int(cfg.MaxAge.Seconds())), + strconv.Itoa(int(cfg.MaxAge.Duration().Seconds())), ).Set(1) - return newCacheWithSleep(cfg.Dir, cfg.MaxAge, time.After, time.After, logger) + return newCacheWithSleep(cfg.Dir, cfg.MaxAge.Duration(), time.After, time.After, logger) } return NullCache{} diff --git a/internal/streamcache/cache_test.go b/internal/streamcache/cache_test.go index 01c71f6a6..4010f95fe 100644 --- a/internal/streamcache/cache_test.go +++ b/internal/streamcache/cache_test.go @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper/duration" "gitlab.com/gitlab-org/gitaly/v15/internal/log" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" ) @@ -26,7 +27,7 @@ func newCache(dir string) Cache { return New(config.StreamCacheConfig{ Enabled: true, Dir: dir, - MaxAge: time.Hour, + MaxAge: duration.Duration(time.Hour), }, log.Default()) } |