Enforce bounds on MaxQueryTime (#9064)

The MaxQueryTime value used in QueryOptions.HasTimedOut() can be set to
an invalid value that would throw off how RPC requests are retried.

This fix uses the same logic that enforces the MaxQueryTime bounds in the
blockingRPC() call.
This commit is contained in:
Pierre Cauchois
2020-10-15 05:43:06 -07:00
committed by GitHub
parent 9aa24f382c
commit 2b5647503c
2 changed files with 19 additions and 10 deletions

View File

@@ -31,13 +31,6 @@ import (
)
const (
// maxQueryTime is used to bound the limit of a blocking query
maxQueryTime = 300 * time.Second
// defaultQueryTime is the amount of time we block waiting for a change
// if no time is specified. Previously we would wait the maxQueryTime.
defaultQueryTime = 300 * time.Second
// Warn if the Raft command is larger than this.
// If it's over 1MB something is probably being abusive.
raftWarnSize = 1024 * 1024
@@ -788,10 +781,10 @@ func (r *rpcHandler) blockingRPC(opts *blockingOptions) error {
}
// Restrict the max query time, and ensure there is always one
if opts.queryOpts.MaxQueryTime > maxQueryTime {
opts.queryOpts.MaxQueryTime = maxQueryTime
if opts.queryOpts.MaxQueryTime > structs.MaxBlockingRPCQueryTime {
opts.queryOpts.MaxQueryTime = structs.MaxBlockingRPCQueryTime
} else if opts.queryOpts.MaxQueryTime <= 0 {
opts.queryOpts.MaxQueryTime = defaultQueryTime
opts.queryOpts.MaxQueryTime = structs.DefaultBlockingRPCQueryTime
}
// Apply a small amount of jitter to the request

View File

@@ -24,6 +24,7 @@ import (
"strings"
"time"
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/cronexpr"
"github.com/hashicorp/go-msgpack/codec"
"github.com/hashicorp/go-multierror"
@@ -163,6 +164,13 @@ const (
// Normalized scorer name
NormScorerName = "normalized-score"
// MaxBlockingRPCQueryTime is used to bound the limit of a blocking query
MaxBlockingRPCQueryTime = 300 * time.Second
// DefaultBlockingRPCQueryTime is the amount of time we block waiting for a change
// if no time is specified. Previously we would wait the MaxBlockingRPCQueryTime.
DefaultBlockingRPCQueryTime = 300 * time.Second
)
// Context defines the scope in which a search for Nomad object operates, and
@@ -289,6 +297,14 @@ func (q QueryOptions) AllowStaleRead() bool {
func (q QueryOptions) HasTimedOut(start time.Time, rpcHoldTimeout time.Duration) bool {
if q.MinQueryIndex > 0 {
// Restrict the max query time, and ensure there is always one
if q.MaxQueryTime > MaxBlockingRPCQueryTime {
q.MaxQueryTime = MaxBlockingRPCQueryTime
} else if q.MaxQueryTime <= 0 {
q.MaxQueryTime = DefaultBlockingRPCQueryTime
}
q.MaxQueryTime += lib.RandomStagger(q.MaxQueryTime / JitterFraction)
return time.Since(start) > (q.MaxQueryTime + rpcHoldTimeout)
}
return time.Since(start) > rpcHoldTimeout