Skip to content

Commit

Permalink
fix: limit query queue size to an int32 to avoid panic
Browse files Browse the repository at this point in the history
  • Loading branch information
danxmoran committed Nov 24, 2020
1 parent d026281 commit 295d89b
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -25,6 +25,7 @@ want to use the default.
1. [20151](https://github.com/influxdata/influxdb/pull/20151): Don't log bodies of V1 write requests.
1. [20097](https://github.com/influxdata/influxdb/pull/20097): Ensure Index.Walk fetches matching foreign keys only.
1. [20149](https://github.com/influxdata/influxdb/pull/20149): Enforce max value of 2147483647 on query concurrency to avoid startup panic.
1. [20149](https://github.com/influxdata/influxdb/pull/20149): Enforce max value of 2147483647 on query queue size to avoid startup panic.

## v2.0.2 [2020-11-19]

Expand Down
2 changes: 1 addition & 1 deletion cmd/influxd/launcher/launcher.go
Expand Up @@ -508,7 +508,7 @@ type Launcher struct {
initialMemoryBytesQuotaPerQuery int
memoryBytesQuotaPerQuery int
maxMemoryBytes int
queueSize int
queueSize int32

boltClient *bolt.Client
kvStore kv.SchemaStore
Expand Down
20 changes: 15 additions & 5 deletions query/control/controller.go
Expand Up @@ -95,10 +95,20 @@ type Config struct {
// This number may be less than the ConcurrencyQuota * MemoryBytesQuotaPerQuery.
MaxMemoryBytes int64

// QueueSize is the number of queries that are allowed to be awaiting execution before new queries are
// rejected.
QueueSize int
Logger *zap.Logger
// QueueSize is the number of queries that are allowed to be awaiting execution before new queries are rejected.
//
// This value is limited to an int32 because it's used to make(chan *Query, QueueSize) on controller startup.
// Through trial-and-error I found that make(chan *Query, N) starts to panic for N > 1<<45 - 12, so not all
// ints or int64s are safe to pass here. Using that max value still immediately crashes the program with an OOM,
// because it tries to allocate TBs of memory for the channel.
// I was able to boot influxd locally using math.MaxInt32 for this parameter.
//
// Less-scientifically, this was the only Config parameter other than ConcurrencyQuota to be typed as an int
// instead of an explicit int64. When ConcurrencyQuota changed to an int32, it felt like a decent idea for
// this to follow suit.
QueueSize int32

Logger *zap.Logger
// MetricLabelKeys is a list of labels to add to the metrics produced by the controller.
// The value for a given key will be read off the context.
// The context value must be a string or an implementation of the Stringer interface.
Expand Down Expand Up @@ -167,7 +177,7 @@ func New(config Config) (*Controller, error) {
zap.Int64("initial_memory_bytes_quota_per_query", c.InitialMemoryBytesQuotaPerQuery),
zap.Int64("memory_bytes_quota_per_query", c.MemoryBytesQuotaPerQuery),
zap.Int64("max_memory_bytes", c.MaxMemoryBytes),
zap.Int("queue_size", c.QueueSize))
zap.Int32("queue_size", c.QueueSize))

mm := &memoryManager{
initialBytesQuotaPerQuery: c.InitialMemoryBytesQuotaPerQuery,
Expand Down

0 comments on commit 295d89b

Please sign in to comment.