From cb0e4c4d751ebbe3a18bddc69aa2e7792b9415aa Mon Sep 17 00:00:00 2001 From: James Fargher Date: Tue, 21 Jun 2022 11:30:08 +1200 Subject: 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. --- internal/blackbox/blackbox.go | 2 +- internal/blackbox/config.go | 5 ++- internal/blackbox/config_test.go | 7 ++-- internal/gitaly/config/config.go | 33 ++++++++------- internal/gitaly/config/config_test.go | 21 +++++----- internal/gitaly/config/prometheus/config.go | 5 ++- internal/gitaly/config/ruby.go | 24 ++++++----- internal/gitaly/maintenance/daily.go | 2 +- internal/gitaly/maintenance/daily_test.go | 3 +- internal/gitaly/rubyserver/rubyserver.go | 4 +- internal/helper/duration/duration.go | 28 +++++++++++++ internal/helper/duration/duration_test.go | 15 +++++++ .../middleware/limithandler/concurrency_limiter.go | 2 +- .../middleware/limithandler/middleware_test.go | 3 +- internal/middleware/limithandler/rate_limiter.go | 2 +- internal/praefect/config/config.go | 49 +++++++++++----------- internal/praefect/config/config_test.go | 43 +++++++++---------- internal/praefect/nodes/tracker/errors.go | 2 +- internal/streamcache/cache.go | 4 +- internal/streamcache/cache_test.go | 3 +- 20 files changed, 156 insertions(+), 101 deletions(-) create mode 100644 internal/helper/duration/duration.go create mode 100644 internal/helper/duration/duration_test.go (limited to 'internal') 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: /+gitaly/PackObjectsCache - MaxAge time.Duration `toml:"max_age"` // Default: 5m + Enabled bool `toml:"enabled"` // Default: false + Dir string `toml:"dir"` // Default: /+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()) } -- cgit v1.2.3 From 72e1c78e920a917ae96a25271173740e6557888e Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Tue, 16 Aug 2022 13:47:26 +0200 Subject: go: Update module github.com/pelletier/go-toml to v2 With this change we upgrade go-toml to v2. This includes changing the import paths to include v2. --- internal/blackbox/config.go | 2 +- internal/gitaly/config/config.go | 2 +- internal/praefect/config/config.go | 2 +- internal/praefect/config/config_test.go | 4 ++-- internal/testhelper/testserver/praefect.go | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) (limited to 'internal') diff --git a/internal/blackbox/config.go b/internal/blackbox/config.go index c9e5966ed..9ea3c33a9 100644 --- a/internal/blackbox/config.go +++ b/internal/blackbox/config.go @@ -5,7 +5,7 @@ import ( "net/url" "time" - "github.com/pelletier/go-toml" + "github.com/pelletier/go-toml/v2" logconfig "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/log" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/duration" ) diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go index 8378933a6..43fae6f40 100644 --- a/internal/gitaly/config/config.go +++ b/internal/gitaly/config/config.go @@ -13,7 +13,7 @@ import ( "syscall" "time" - "github.com/pelletier/go-toml" + "github.com/pelletier/go-toml/v2" log "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/auth" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/cgroups" diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go index 6a053f0ec..5af6eccdb 100644 --- a/internal/praefect/config/config.go +++ b/internal/praefect/config/config.go @@ -6,7 +6,7 @@ import ( "os" "time" - "github.com/pelletier/go-toml" + "github.com/pelletier/go-toml/v2" promclient "github.com/prometheus/client_golang/prometheus" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/auth" diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go index 503430dda..b0e1879f6 100644 --- a/internal/praefect/config/config_test.go +++ b/internal/praefect/config/config_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/pelletier/go-toml" + "github.com/pelletier/go-toml/v2" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/log" @@ -522,6 +522,6 @@ func TestSerialization(t *testing.T) { t.Run("partially set", func(t *testing.T) { out.Reset() require.NoError(t, encoder.Encode(Config{ListenAddr: "localhost:5640"})) - require.Equal(t, "listen_addr = \"localhost:5640\"\n", out.String()) + require.Equal(t, "listen_addr = 'localhost:5640'\n", out.String()) }) } diff --git a/internal/testhelper/testserver/praefect.go b/internal/testhelper/testserver/praefect.go index ec930ffd8..8f4fc5040 100644 --- a/internal/testhelper/testserver/praefect.go +++ b/internal/testhelper/testserver/praefect.go @@ -11,7 +11,7 @@ import ( "testing" "time" - "github.com/pelletier/go-toml" + "github.com/pelletier/go-toml/v2" "github.com/stretchr/testify/require" gitalycfg "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/config" -- cgit v1.2.3