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
Changes from 1 commit
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
Next Next commit
fix(query): accept queue-size > 0 when concurrency = 0
danxmoran committed Apr 28, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 387a8f33ca3d6dc7eb80d3aa2690bbf2e969c832
21 changes: 10 additions & 11 deletions query/control/controller.go
Original file line number Diff line number Diff line change
@@ -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 {
@@ -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 {
@@ -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")
}