Skip to content

Commit

Permalink
fix(query): make config validation for query controller less strict (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
danxmoran committed Apr 28, 2021
1 parent e15b83d commit 92fe6fb
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 13 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]
----------------------
Expand Down
4 changes: 2 additions & 2 deletions cmd/influxd/launcher/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions cmd/influxd/launcher/launcher_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
21 changes: 10 additions & 11 deletions query/control/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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")
}
Expand Down

0 comments on commit 92fe6fb

Please sign in to comment.