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

[dbnode] Fix AggregateQuery limits #3112

Merged
merged 22 commits into from
Jan 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
476dbd4
[dbnode] AggregateQuery limit fix
arnikola Jan 22, 2021
5a99682
Cleanup
arnikola Jan 22, 2021
a0ca9db
[integration test] Add label query limits integration test
wesleyk Jan 22, 2021
5d2173d
Add paren
wesleyk Jan 22, 2021
c4ee6c6
Fixed exhaustive case, remove dead code
arnikola Jan 22, 2021
522543c
Merge branch 'arnikola/fix-limits' of github.com:m3db/m3 into arnikol…
arnikola Jan 22, 2021
ebfaa35
Aggregate results changes
arnikola Jan 22, 2021
849c92b
Add proper require exhaustive conversion + integration test for agg q…
wesleyk Jan 22, 2021
bd35ca9
Merge branch 'arnikola/fix-limits' of github.com:m3db/m3 into arnikol…
wesleyk Jan 22, 2021
cad0e06
Merge branch 'master' into arnikola/fix-limits
wesleyk Jan 22, 2021
829e6b3
Avoid flakiness with high limits
wesleyk Jan 22, 2021
31f001b
Limit on docs or inserts
arnikola Jan 22, 2021
172100f
Fixup integration test
wesleyk Jan 22, 2021
2695b90
Merge branch 'arnikola/fix-limits' of github.com:m3db/m3 into arnikol…
wesleyk Jan 22, 2021
84d5a86
Add more precise assertions to label query limits integration test
wesleyk Jan 22, 2021
dd5ae2e
Finish test fixes and refactor
arnikola Jan 22, 2021
b18484e
Response + lint
arnikola Jan 22, 2021
ab538cd
Improve IT comments
wesleyk Jan 22, 2021
505319c
Merge branch 'master' into arnikola/fix-limits
wesleyk Jan 22, 2021
b3fdbba
Response + lint
arnikola Jan 22, 2021
4221a99
Fix integrations
arnikola Jan 22, 2021
704b172
Merge branch 'arnikola/fix-limits' of github.com:m3db/m3 into arnikol…
arnikola Jan 22, 2021
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
43 changes: 42 additions & 1 deletion scripts/docker-integration-tests/prometheus/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ function test_query_limits_applied {
ATTEMPTS=50 TIMEOUT=2 MAX_TIMEOUT=4 retry_with_backoff \
'[[ $(curl -s -o /dev/null -w "%{http_code}" -H "M3-Limit-Max-Series: 3" -H "M3-Limit-Require-Exhaustive: true" 0.0.0.0:7201/api/v1/query?query=database_write_tagged_success) = "400" ]]'

# Test the default docs limit applied when directly querying
# Test the docs limit applied when directly querying
# coordinator (docs limit set by header)
echo "Test query docs limit with coordinator limit header"
ATTEMPTS=50 TIMEOUT=2 MAX_TIMEOUT=4 retry_with_backoff \
Expand Down Expand Up @@ -391,6 +391,46 @@ function test_series {
'[[ $(curl -s "0.0.0.0:7201/api/v1/series?match[]=prometheus_remote_storage_samples_total&start=-292273086-05-16T16:47:06Z&end=292277025-08-18T07:12:54.999999999Z" | jq -r ".data | length") -eq 1 ]]'
}

function test_label_query_limits_applied {
# Test that require exhaustive does nothing if limits are not hit
echo "Test label limits with require-exhaustive headers true (below limit therefore no error)"
ATTEMPTS=50 TIMEOUT=2 MAX_TIMEOUT=4 retry_with_backoff \
'[[ $(curl -s -o /dev/null -w "%{http_code}" -H "M3-Limit-Max-Series: 10000" -H "M3-Limit-Max-Series: 10000" -H "M3-Limit-Require-Exhaustive: true" 0.0.0.0:7201/api/v1/label/__name__/values) = "200" ]]'

# the header takes precedence over the configured default series limit
echo "Test label series limit with coordinator limit header (default requires exhaustive so error)"
ATTEMPTS=50 TIMEOUT=2 MAX_TIMEOUT=4 retry_with_backoff \
'[[ -n $(curl -s -H "M3-Limit-Max-Series: 1" 0.0.0.0:7201/api/v1/label/__name__/values | jq ."error" | grep "query exceeded limit") ]]'

echo "Test label series limit with require-exhaustive headers false"
ATTEMPTS=50 TIMEOUT=2 MAX_TIMEOUT=4 retry_with_backoff \
'[[ $(curl -s -H "M3-Limit-Max-Series: 2" -H "M3-Limit-Require-Exhaustive: false" 0.0.0.0:7201/api/v1/label/__name__/values | jq -r ".data | length") -eq 1 ]]'

echo "Test label series limit with require-exhaustive headers true (above limit therefore error)"
# Test that require exhaustive error is returned
ATTEMPTS=50 TIMEOUT=2 MAX_TIMEOUT=4 retry_with_backoff \
'[[ -n $(curl -s -H "M3-Limit-Max-Series: 2" -H "M3-Limit-Require-Exhaustive: true" 0.0.0.0:7201/api/v1/label/__name__/values | jq ."error" | grep "query exceeded limit") ]]'
# Test that require exhaustive error is 4xx
ATTEMPTS=50 TIMEOUT=2 MAX_TIMEOUT=4 retry_with_backoff \
'[[ $(curl -s -o /dev/null -w "%{http_code}" -H "M3-Limit-Max-Series: 2" -H "M3-Limit-Require-Exhaustive: true" 0.0.0.0:7201/api/v1/label/__name__/values) = "400" ]]'

echo "Test label docs limit with coordinator limit header (default requires exhaustive so error)"
ATTEMPTS=50 TIMEOUT=2 MAX_TIMEOUT=4 retry_with_backoff \
'[[ -n $(curl -s -H "M3-Limit-Max-Docs: 1" 0.0.0.0:7201/api/v1/label/__name__/values | jq ."error" | grep "query exceeded limit") ]]'

echo "Test label docs limit with require-exhaustive headers false"
ATTEMPTS=50 TIMEOUT=2 MAX_TIMEOUT=4 retry_with_backoff \
'[[ $(curl -s -H "M3-Limit-Max-Docs: 2" -H "M3-Limit-Require-Exhaustive: false" 0.0.0.0:7201/api/v1/label/__name__/values | jq -r ".data | length") -eq 1 ]]'

echo "Test label docs limit with require-exhaustive headers true (above limit therefore error)"
# Test that require exhaustive error is returned
ATTEMPTS=50 TIMEOUT=2 MAX_TIMEOUT=4 retry_with_backoff \
'[[ -n $(curl -s -H "M3-Limit-Max-Docs: 1" -H "M3-Limit-Require-Exhaustive: true" 0.0.0.0:7201/api/v1/label/__name__/values | jq ."error" | grep "query exceeded limit") ]]'
# Test that require exhaustive error is 4xx
ATTEMPTS=50 TIMEOUT=2 MAX_TIMEOUT=4 retry_with_backoff \
'[[ $(curl -s -o /dev/null -w "%{http_code}" -H "M3-Limit-Max-Docs: 1" -H "M3-Limit-Require-Exhaustive: true" 0.0.0.0:7201/api/v1/label/__name__/values) = "400" ]]'
}

echo "Running readiness test"
test_readiness

Expand All @@ -409,6 +449,7 @@ test_prometheus_query_native_timeout
test_query_restrict_tags
test_prometheus_remote_write_map_tags
test_series
test_label_query_limits_applied

echo "Running function correctness tests"
test_correctness
Expand Down
23 changes: 21 additions & 2 deletions src/dbnode/storage/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -1421,6 +1421,7 @@ func (i *nsIndex) AggregateQuery(
results := i.aggregateResultsPool.Get()
aopts := index.AggregateResultsOptions{
SizeLimit: opts.SeriesLimit,
DocsLimit: opts.DocsLimit,
FieldFilter: opts.FieldFilter,
Type: opts.Type,
}
Expand Down Expand Up @@ -1687,7 +1688,16 @@ func (i *nsIndex) execBlockQueryFn(
sp.LogFields(logFields...)
defer sp.Finish()

blockExhaustive, err := block.Query(ctx, cancellable, query, opts, results, logFields)
docResults, ok := results.(index.DocumentResults)
if !ok { // should never happen
state.Lock()
err := fmt.Errorf("unknown results type [%T] received during query", results)
state.multiErr = state.multiErr.Add(err)
state.Unlock()
return
}

blockExhaustive, err := block.Query(ctx, cancellable, query, opts, docResults, logFields)
if err == index.ErrUnableToQueryBlockClosed {
// NB(r): Because we query this block outside of the results lock, it's
// possible this block may get closed if it slides out of retention, in
Expand Down Expand Up @@ -1725,7 +1735,16 @@ func (i *nsIndex) execBlockWideQueryFn(
sp.LogFields(logFields...)
defer sp.Finish()

_, err := block.Query(ctx, cancellable, query, opts, results, logFields)
docResults, ok := results.(index.DocumentResults)
if !ok { // should never happen
state.Lock()
err := fmt.Errorf("unknown results type [%T] received during wide query", results)
state.multiErr = state.multiErr.Add(err)
state.Unlock()
return
}

_, err := block.Query(ctx, cancellable, query, opts, docResults, logFields)
if err == index.ErrUnableToQueryBlockClosed {
// NB(r): Because we query this block outside of the results lock, it's
// possible this block may get closed if it slides out of retention, in
Expand Down
Loading