Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(query): make config validation for query controller less strict #21324

Merged
merged 4 commits into from
Apr 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,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 @@ -129,6 +129,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 @@ -119,19 +119,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 @@ -148,10 +152,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 @@ -165,15 +169,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