From 92fe6fbeca49ab44ef4aac8de9c224786421a4d6 Mon Sep 17 00:00:00 2001 From: Daniel Moran Date: Wed, 28 Apr 2021 17:27:37 -0400 Subject: [PATCH] fix(query): make config validation for query controller less strict (#21324) * fix(query): accept queue-size > 0 when concurrency = 0 * fix(influxd): revert defaults for query settings to avoid validation err * test: lower the default query concurrency used by test launchers --- CHANGELOG.md | 2 ++ cmd/influxd/launcher/cmd.go | 4 ++-- cmd/influxd/launcher/launcher_helpers.go | 2 ++ query/control/controller.go | 21 ++++++++++----------- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f78606ef14..df0f0326643 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Bug Fixes 1. [21321](https://github.com/influxdata/influxdb/pull/21321): Ensure query config written by influxd upgrade is valid. +1. [21324](https://github.com/influxdata/influxdb/pull/21324): Revert to nonzero defaults for `query-concurrency` and `query-queue-size` to avoid validation failures for upgrading users. +1. [21324](https://github.com/influxdata/influxdb/pull/21324): Don't fail validation when `query-concurrency` is 0 and `query-queue-size` is > 0. ## v2.0.5 [2021-04-27] ---------------------- diff --git a/cmd/influxd/launcher/cmd.go b/cmd/influxd/launcher/cmd.go index 892e2c8d7c7..fac62ed3e62 100644 --- a/cmd/influxd/launcher/cmd.go +++ b/cmd/influxd/launcher/cmd.go @@ -205,11 +205,11 @@ func NewOpts(viper *viper.Viper) *InfluxdOpts { NoTasks: false, - ConcurrencyQuota: 0, + ConcurrencyQuota: 1024, InitialMemoryBytesQuotaPerQuery: 0, MemoryBytesQuotaPerQuery: MaxInt, MaxMemoryBytes: 0, - QueueSize: 0, + QueueSize: 1024, Testing: false, TestingAlwaysAllowSetup: false, diff --git a/cmd/influxd/launcher/launcher_helpers.go b/cmd/influxd/launcher/launcher_helpers.go index b7fd4fad817..c0e92133c72 100644 --- a/cmd/influxd/launcher/launcher_helpers.go +++ b/cmd/influxd/launcher/launcher_helpers.go @@ -128,6 +128,8 @@ func (tl *TestLauncher) Run(tb zaptest.TestingT, ctx context.Context, setters .. opts.HttpBindAddress = "127.0.0.1:0" opts.LogLevel = zap.DebugLevel opts.ReportingDisabled = true + opts.ConcurrencyQuota = 32 + opts.QueueSize = 16 for _, setter := range setters { setter(opts) diff --git a/query/control/controller.go b/query/control/controller.go index 95f6bfaea6a..b6abd1c5c45 100644 --- a/query/control/controller.go +++ b/query/control/controller.go @@ -118,19 +118,23 @@ type Config struct { // complete will fill in the defaults, validate the configuration, and // return the new Config. -func (c *Config) complete() (Config, error) { +func (c *Config) complete(log *zap.Logger) (Config, error) { config := *c if config.InitialMemoryBytesQuotaPerQuery == 0 { config.InitialMemoryBytesQuotaPerQuery = config.MemoryBytesQuotaPerQuery } + if config.ConcurrencyQuota == 0 && config.QueueSize > 0 { + log.Warn("Ignoring query QueueSize > 0 when ConcurrencyQuota is 0") + config.QueueSize = 0 + } - if err := config.validate(true); err != nil { + if err := config.validate(); err != nil { return Config{}, err } return config, nil } -func (c *Config) validate(isComplete bool) error { +func (c *Config) validate() error { if c.ConcurrencyQuota < 0 { return errors.New("ConcurrencyQuota must not be negative") } else if c.ConcurrencyQuota == 0 { @@ -147,10 +151,10 @@ func (c *Config) validate(isComplete bool) error { return errors.New("QueueSize must be positive when ConcurrencyQuota is limited") } } - if c.MemoryBytesQuotaPerQuery < 0 || (isComplete && c.MemoryBytesQuotaPerQuery == 0) { + if c.MemoryBytesQuotaPerQuery < 0 { return errors.New("MemoryBytesQuotaPerQuery must be positive") } - if c.InitialMemoryBytesQuotaPerQuery < 0 || (isComplete && c.InitialMemoryBytesQuotaPerQuery == 0) { + if c.InitialMemoryBytesQuotaPerQuery < 0 { return errors.New("InitialMemoryBytesQuotaPerQuery must be positive") } if c.MaxMemoryBytes < 0 { @@ -164,15 +168,10 @@ func (c *Config) validate(isComplete bool) error { return nil } -// Validate will validate that the controller configuration is valid. -func (c *Config) Validate() error { - return c.validate(false) -} - type QueryID uint64 func New(config Config, logger *zap.Logger) (*Controller, error) { - c, err := config.complete() + c, err := config.complete(logger) if err != nil { return nil, errors.Wrap(err, "invalid controller config") }