From de86ba7efd01084276e320ade5813b26e7d7851b Mon Sep 17 00:00:00 2001 From: Linas Medziunas Date: Thu, 26 Nov 2020 09:29:47 +0200 Subject: [PATCH 1/5] [dbnode] Integration test for error message --- src/dbnode/integration/error_msg_test.go | 58 ++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 src/dbnode/integration/error_msg_test.go diff --git a/src/dbnode/integration/error_msg_test.go b/src/dbnode/integration/error_msg_test.go new file mode 100644 index 0000000000..94e722b6bc --- /dev/null +++ b/src/dbnode/integration/error_msg_test.go @@ -0,0 +1,58 @@ +// +build integration + +// Copyright (c) 2020 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package integration + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/m3db/m3/src/dbnode/storage/index" + "github.com/m3db/m3/src/m3ninx/idx" + "github.com/m3db/m3/src/x/ident" +) + +func TestErrorMsg(t *testing.T) { + testOpts := NewTestOptions(t) + testSetup := newTestSetupWithCommitLogAndFilesystemBootstrapper(t, testOpts) + defer testSetup.Close() + + require.NoError(t, testSetup.StartServer()) + defer require.NoError(t, testSetup.StopServer()) + + var ( + storageOpts = testSetup.StorageOpts() + nowFn = storageOpts.ClockOptions().NowFn() + end = nowFn() + start = end.Add(-time.Hour) + query = index.Query{Query: idx.NewTermQuery([]byte("tag"), []byte("value"))} + ) + + session, err := testSetup.M3DBClient().DefaultSession() + require.NoError(t, err) + + _, _, err = session.FetchTagged( + ident.BytesID("???"), query, index.QueryOptions{StartInclusive: start, EndExclusive: end}) + require.NoError(t, err) +} From 46518fd60fc915458007dab0b490b24ce59932a3 Mon Sep 17 00:00:00 2001 From: Linas Medziunas Date: Thu, 26 Nov 2020 09:37:50 +0200 Subject: [PATCH 2/5] Do not stop test server prematurely --- src/dbnode/integration/error_msg_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/dbnode/integration/error_msg_test.go b/src/dbnode/integration/error_msg_test.go index 94e722b6bc..e0d8a469e4 100644 --- a/src/dbnode/integration/error_msg_test.go +++ b/src/dbnode/integration/error_msg_test.go @@ -39,7 +39,9 @@ func TestErrorMsg(t *testing.T) { defer testSetup.Close() require.NoError(t, testSetup.StartServer()) - defer require.NoError(t, testSetup.StopServer()) + defer func() { + require.NoError(t, testSetup.StopServer()) + }() var ( storageOpts = testSetup.StorageOpts() From 954b9ce58798ec60d4573b5d42521476aa75b7ea Mon Sep 17 00:00:00 2001 From: Vilius Pranckaitis Date: Fri, 18 Dec 2020 14:17:27 +0200 Subject: [PATCH 3/5] add a few tests for query limit exceeded error --- src/dbnode/integration/error_msg_test.go | 60 ------------ src/dbnode/integration/query_limit_test.go | 107 +++++++++++++++++++++ src/dbnode/storage/limits/errors.go | 10 +- src/dbnode/storage/limits/errors_test.go | 96 ++++++++++++++++++ 4 files changed, 212 insertions(+), 61 deletions(-) delete mode 100644 src/dbnode/integration/error_msg_test.go create mode 100644 src/dbnode/integration/query_limit_test.go create mode 100644 src/dbnode/storage/limits/errors_test.go diff --git a/src/dbnode/integration/error_msg_test.go b/src/dbnode/integration/error_msg_test.go deleted file mode 100644 index e0d8a469e4..0000000000 --- a/src/dbnode/integration/error_msg_test.go +++ /dev/null @@ -1,60 +0,0 @@ -// +build integration - -// Copyright (c) 2020 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - -package integration - -import ( - "testing" - "time" - - "github.com/stretchr/testify/require" - - "github.com/m3db/m3/src/dbnode/storage/index" - "github.com/m3db/m3/src/m3ninx/idx" - "github.com/m3db/m3/src/x/ident" -) - -func TestErrorMsg(t *testing.T) { - testOpts := NewTestOptions(t) - testSetup := newTestSetupWithCommitLogAndFilesystemBootstrapper(t, testOpts) - defer testSetup.Close() - - require.NoError(t, testSetup.StartServer()) - defer func() { - require.NoError(t, testSetup.StopServer()) - }() - - var ( - storageOpts = testSetup.StorageOpts() - nowFn = storageOpts.ClockOptions().NowFn() - end = nowFn() - start = end.Add(-time.Hour) - query = index.Query{Query: idx.NewTermQuery([]byte("tag"), []byte("value"))} - ) - - session, err := testSetup.M3DBClient().DefaultSession() - require.NoError(t, err) - - _, _, err = session.FetchTagged( - ident.BytesID("???"), query, index.QueryOptions{StartInclusive: start, EndExclusive: end}) - require.NoError(t, err) -} diff --git a/src/dbnode/integration/query_limit_test.go b/src/dbnode/integration/query_limit_test.go new file mode 100644 index 0000000000..adc49ddbfc --- /dev/null +++ b/src/dbnode/integration/query_limit_test.go @@ -0,0 +1,107 @@ +// +build integration + +// Copyright (c) 2020 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package integration + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/m3db/m3/src/dbnode/client" + "github.com/m3db/m3/src/dbnode/namespace" + "github.com/m3db/m3/src/dbnode/storage" + "github.com/m3db/m3/src/dbnode/storage/index" + "github.com/m3db/m3/src/dbnode/storage/limits" + "github.com/m3db/m3/src/m3ninx/idx" + "github.com/m3db/m3/src/x/ident" + xtime "github.com/m3db/m3/src/x/time" +) + +func TestErrorMsg(t *testing.T) { + testOpts, ns := newTestOptionsWithIndexedNamespace(t) + testSetup := newTestSetupWithQueryLimits(t, testOpts) + defer testSetup.Close() + + require.NoError(t, testSetup.StartServer()) + defer func() { + require.NoError(t, testSetup.StopServer()) + }() + + var ( + nowFn = testSetup.StorageOpts().ClockOptions().NowFn() + end = nowFn() + start = end.Add(-time.Hour) + query = index.Query{Query: idx.NewTermQuery([]byte("tag"), []byte("value"))} + queryOpts = index.QueryOptions{StartInclusive: start, EndExclusive: end} + ) + + session, err := testSetup.M3DBClient().DefaultSession() + require.NoError(t, err) + + for i := 0; i < 2; i++ { + metricName := fmt.Sprintf("metric_%v", i) + tags := ident.StringTag("tag", "value") + timestamp := nowFn().Add(-time.Minute * time.Duration(i+1)) + session.WriteTagged(ns.ID(), ident.StringID(metricName), + ident.NewTagsIterator(ident.NewTags(tags)), timestamp, 0.0, xtime.Second, nil) + } + + _, _, err = session.FetchTagged(ns.ID(), query, queryOpts) + require.True(t, client.IsResourceExhaustedError(err), + "expected resource exhausted error, got: %v", err) +} + +func newTestOptionsWithIndexedNamespace(t *testing.T) (TestOptions, namespace.Metadata) { + idxOpts := namespace.NewIndexOptions().SetEnabled(true) + nsOpts := namespace.NewOptions().SetIndexOptions(idxOpts) + ns, err := namespace.NewMetadata(testNamespaces[0], nsOpts) + require.NoError(t, err) + + testOpts := NewTestOptions(t).SetNamespaces([]namespace.Metadata{ns}) + return testOpts, ns +} + +func newTestSetupWithQueryLimits(t *testing.T, opts TestOptions) TestSetup { + storageLimitsFn := func(storageOpts storage.Options) storage.Options { + queryLookback := limits.DefaultLookbackLimitOptions() + queryLookback.Limit = 1 + queryLookback.Lookback = time.Hour + + limitOpts := limits.NewOptions(). + SetBytesReadLimitOpts(queryLookback). + SetDocsLimitOpts(queryLookback). + SetInstrumentOptions(storageOpts.InstrumentOptions()) + queryLimits, err := limits.NewQueryLimits(limitOpts) + require.NoError(t, err) + + indexOpts := storageOpts.IndexOptions().SetQueryLimits(queryLimits) + return storageOpts.SetIndexOptions(indexOpts) + } + + setup, err := NewTestSetup(t, opts, nil, storageLimitsFn) + require.NoError(t, err) + + return setup +} diff --git a/src/dbnode/storage/limits/errors.go b/src/dbnode/storage/limits/errors.go index a876dd9b7a..d44ed882f7 100644 --- a/src/dbnode/storage/limits/errors.go +++ b/src/dbnode/storage/limits/errors.go @@ -39,10 +39,18 @@ func (err *queryLimitExceededError) Error() string { // IsQueryLimitExceededError returns true if the error is a query limits exceeded error. func IsQueryLimitExceededError(err error) bool { + //nolint:errorlint for err != nil { - if _, ok := err.(*queryLimitExceededError); ok { //nolint:errorlint + if _, ok := err.(*queryLimitExceededError); ok { return true } + if multiErr, ok := err.(xerrors.MultiError); ok { + for _, e := range multiErr.Errors() { + if IsQueryLimitExceededError(e) { + return true + } + } + } err = xerrors.InnerError(err) } return false diff --git a/src/dbnode/storage/limits/errors_test.go b/src/dbnode/storage/limits/errors_test.go new file mode 100644 index 0000000000..96532cb624 --- /dev/null +++ b/src/dbnode/storage/limits/errors_test.go @@ -0,0 +1,96 @@ +// Copyright (c) 2020 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package limits + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + + xerrors "github.com/m3db/m3/src/x/errors" +) + +func TestIsQueryLimitExceededError(t *testing.T) { + randomErr := xerrors.NewNonRetryableError(fmt.Errorf("random error")) + limitExceededErr := NewQueryLimitExceededError("query limit exceeded") + + tests := []struct { + name string + err error + expected bool + }{ + { + "not query limit exceeded", + randomErr, + false, + }, + { + "query limit exceeded", + limitExceededErr, + true, + }, + { + "inner non query limit exceeded", + xerrors.NewInvalidParamsError(randomErr), + false, + }, + { + "inner query limit exceeded", + xerrors.NewInvalidParamsError(limitExceededErr), + true, + }, + { + "empty multi error", + multiError(), + false, + }, + { + "multi error without query limit exceeded", + multiError(randomErr), + false, + }, + { + "multi error with only query limit exceeded", + multiError(limitExceededErr), + true, + }, + { + "multi error with query limit exceeded", + multiError(randomErr, xerrors.NewRetryableError(limitExceededErr)), + true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, IsQueryLimitExceededError(tt.err)) + }) + } +} + +func multiError(errs ...error) error { + multiErr := xerrors.NewMultiError() + for _, e := range errs { + multiErr = multiErr.Add(e) + } + return multiErr.FinalError() +} From a87647121314d349a9175ef9029ed303a42cd2fa Mon Sep 17 00:00:00 2001 From: Vilius Pranckaitis Date: Fri, 18 Dec 2020 15:03:16 +0200 Subject: [PATCH 4/5] rename test --- src/dbnode/integration/query_limit_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dbnode/integration/query_limit_test.go b/src/dbnode/integration/query_limit_test.go index adc49ddbfc..4ce41543fc 100644 --- a/src/dbnode/integration/query_limit_test.go +++ b/src/dbnode/integration/query_limit_test.go @@ -39,7 +39,7 @@ import ( xtime "github.com/m3db/m3/src/x/time" ) -func TestErrorMsg(t *testing.T) { +func TestQueryLimitExceededError(t *testing.T) { testOpts, ns := newTestOptionsWithIndexedNamespace(t) testSetup := newTestSetupWithQueryLimits(t, testOpts) defer testSetup.Close() From 2ad45b7db3180d473abfeaff864c79b68b6661b3 Mon Sep 17 00:00:00 2001 From: Vilius Pranckaitis Date: Mon, 21 Dec 2020 10:32:23 +0200 Subject: [PATCH 5/5] PR comments --- src/dbnode/integration/query_limit_test.go | 10 ++++++---- src/dbnode/storage/limits/errors_test.go | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/dbnode/integration/query_limit_test.go b/src/dbnode/integration/query_limit_test.go index 4ce41543fc..53a00b7665 100644 --- a/src/dbnode/integration/query_limit_test.go +++ b/src/dbnode/integration/query_limit_test.go @@ -51,7 +51,7 @@ func TestQueryLimitExceededError(t *testing.T) { var ( nowFn = testSetup.StorageOpts().ClockOptions().NowFn() - end = nowFn() + end = nowFn().Truncate(time.Hour) start = end.Add(-time.Hour) query = index.Query{Query: idx.NewTermQuery([]byte("tag"), []byte("value"))} queryOpts = index.QueryOptions{StartInclusive: start, EndExclusive: end} @@ -61,9 +61,11 @@ func TestQueryLimitExceededError(t *testing.T) { require.NoError(t, err) for i := 0; i < 2; i++ { - metricName := fmt.Sprintf("metric_%v", i) - tags := ident.StringTag("tag", "value") - timestamp := nowFn().Add(-time.Minute * time.Duration(i+1)) + var ( + metricName = fmt.Sprintf("metric_%v", i) + tags = ident.StringTag("tag", "value") + timestamp = nowFn().Add(-time.Minute * time.Duration(i+1)) + ) session.WriteTagged(ns.ID(), ident.StringID(metricName), ident.NewTagsIterator(ident.NewTags(tags)), timestamp, 0.0, xtime.Second, nil) } diff --git a/src/dbnode/storage/limits/errors_test.go b/src/dbnode/storage/limits/errors_test.go index 96532cb624..751ba66192 100644 --- a/src/dbnode/storage/limits/errors_test.go +++ b/src/dbnode/storage/limits/errors_test.go @@ -21,7 +21,7 @@ package limits import ( - "fmt" + "errors" "testing" "github.com/stretchr/testify/assert" @@ -30,7 +30,7 @@ import ( ) func TestIsQueryLimitExceededError(t *testing.T) { - randomErr := xerrors.NewNonRetryableError(fmt.Errorf("random error")) + randomErr := xerrors.NewNonRetryableError(errors.New("random error")) limitExceededErr := NewQueryLimitExceededError("query limit exceeded") tests := []struct {