From cf483aca7701f4b30d5fac81244ebe58cf29c828 Mon Sep 17 00:00:00 2001 From: reesporte Date: Mon, 10 Jan 2022 13:07:57 -0600 Subject: [PATCH 1/9] distinct on timestamps can reduce now --- executor.go | 18 +++++++++++++++++ executor_internal_test.go | 41 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/executor.go b/executor.go index b9d83e441..692feef82 100644 --- a/executor.go +++ b/executor.go @@ -1181,6 +1181,8 @@ func (e *executor) executeDistinct(ctx context.Context, qcx *Qcx, index string, return other.Union(v.(*Row)) case nil: return v + case DistinctTimestamp: + return other.Union(v.(DistinctTimestamp)) default: return errors.Errorf("unexpected return type from executeDistinctShard: %+v %T", other, other) } @@ -1633,6 +1635,22 @@ type DistinctTimestamp struct { Name string } +// Union returns the union of the values of `d` and `other` +func (d *DistinctTimestamp) Union(other DistinctTimestamp) DistinctTimestamp { + both := map[string]string{} + for _, val := range d.Values { + both[val] = val + } + for _, val := range other.Values { + both[val] = val + } + vals := []string{} + for key := range both { + vals = append(vals, key) + } + return DistinctTimestamp{Name: d.Name, Values: vals} +} + func executeDistinctShardSet(ctx context.Context, qcx *Qcx, idx *Index, fieldName string, shard uint64, filterBitmap *roaring.Bitmap) (result *Row, err0 error) { index := idx.Name() tx, finisher, err := qcx.GetTx(Txo{Write: !writable, Index: idx, Shard: shard}) diff --git a/executor_internal_test.go b/executor_internal_test.go index 79a9cfe72..953123385 100644 --- a/executor_internal_test.go +++ b/executor_internal_test.go @@ -504,3 +504,44 @@ func TestGetScaledInt(t *testing.T) { } } + +func TestDistinctTimestampUnion(t *testing.T) { + cases := []struct { + name string + a DistinctTimestamp + b DistinctTimestamp + expected DistinctTimestamp + }{ + { + name: "empty other", + a: DistinctTimestamp{Name: "a", Values: []string{"a", "b", "c"}}, + b: DistinctTimestamp{Name: "a", Values: []string{}}, + expected: DistinctTimestamp{Name: "a", Values: []string{"a", "b", "c"}}, + }, + { + name: "one more in other", + a: DistinctTimestamp{Name: "a", Values: []string{"a", "b", "c"}}, + b: DistinctTimestamp{Name: "a", Values: []string{"a", "b", "c", "d"}}, + expected: DistinctTimestamp{Name: "a", Values: []string{"a", "b", "c", "d"}}, + }, + } + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + res := test.a.Union(test.b) + allThere := true + for _, val := range res.Values { + here := false + for _, expected := range test.expected.Values { + if val == expected { + here = true + break + } + } + allThere = allThere && here + } + if !allThere { + t.Errorf("expected %v, got %v", test.expected, res) + } + }) + } +} From 1f370744c55d6d51eaba418ee47248eabb607c66 Mon Sep 17 00:00:00 2001 From: reesporte Date: Mon, 10 Jan 2022 15:41:30 -0600 Subject: [PATCH 2/9] add multi-shard test for distinct(timestamp) --- executor_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/executor_test.go b/executor_test.go index 432499511..990a569dd 100644 --- a/executor_test.go +++ b/executor_test.go @@ -6752,9 +6752,10 @@ func variousQueriesCountDistinctTimestamp(t *testing.T, c *test.Cluster) { c.CreateField(t, index, pilosa.IndexOptions{TrackExistence: true}, field, pilosa.OptFieldTypeTimestamp(time.Unix(0, 0), "s")) // add some data - data := []string{"2010-01-02T12:32:00Z", "2010-04-20T12:32:00Z", "2011-04-20T12:32:00Z"} + data := []string{"2010-01-02T12:32:00Z", "2010-04-20T12:32:00Z", "2011-04-20T12:59:00Z", "2011-04-20T12:40:00Z", "2011-04-20T12:32:00Z"} + for i, datum := range data { - c.Query(t, index, fmt.Sprintf("Set(%d, ts=\"%s\")", i+10, datum)) + c.Query(t, index, fmt.Sprintf("Set(%d, ts=\"%s\")", i*(1<<20), datum)) } // query the Count of Distinct vals in field ts From 55a385ed2d9acc436860bdec2cc4315c862eb00b Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Fri, 7 Jan 2022 14:47:38 -0600 Subject: [PATCH 3/9] add sorting for ints/mutex in batch importer fixes pathological case where imports with randomly ordered IDs which spanned multiple shards and included ints or mutex fields could be incredibly slow due to making 1000s of requests. --- client/batch.go | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/client/batch.go b/client/batch.go index 3b8485279..182d4e476 100644 --- a/client/batch.go +++ b/client/batch.go @@ -2,6 +2,7 @@ package client import ( + "sort" "sync" "time" @@ -20,7 +21,9 @@ const ( // order. Could be worth sorting everything after translation (as an // option?). Instead of sorting all simultaneously, it might be faster // (more cache friendly) to sort ids and save the swap ops to apply to -// everything else that needs to be sorted. +// everything else that needs to be sorted. Note: we're already doing +// some sorting in importValueData and importMutexData, so if we +// implement it at the top level, remember to remove it there. // TODO support clearing values? nil values in records are ignored, // but perhaps we could have a special type indicating that a bit or @@ -1216,6 +1219,22 @@ func (b *Batch) makeFragments(frags, clearFrags fragments) (fragments, fragments return frags, clearFrags, nil } +type valsByIDsSortable struct { + ids []uint64 + vals []int64 + // shard width so we can compare by shard instead of ID + width uint64 +} + +func (v *valsByIDsSortable) Len() int { return len(v.ids) } + +// comparing on shard rather than ID was twice as fast in informal tests +func (v *valsByIDsSortable) Less(i, j int) bool { return v.ids[i]/v.width < v.ids[j]/v.width } +func (v *valsByIDsSortable) Swap(i, j int) { + v.ids[i], v.ids[j] = v.ids[j], v.ids[i] + v.vals[i], v.vals[j] = v.vals[j], v.vals[i] +} + // importValueData imports data for int fields. func (b *Batch) importValueData() error { shardWidth := b.index.ShardWidth() @@ -1246,6 +1265,12 @@ func (b *Batch) importValueData() error { if len(ids) == 0 { continue // TODO test this "all nil" case } + + sc := &valsByIDsSortable{ids: ids, vals: bvalues, width: shardWidth} + if !sort.IsSorted(sc) { + sort.Sort(sc) + } + curShard := ids[0] / shardWidth startIdx := 0 for i := 1; i <= len(ids); i++ { @@ -1285,6 +1310,22 @@ func (b *Batch) importValueData() error { return errors.Wrap(err, "importing value data") } +type rowsByIDsSortable struct { + ids []uint64 + rows []uint64 + // shard width so we can compare by shard instead of ID + width uint64 +} + +func (v *rowsByIDsSortable) Len() int { return len(v.ids) } + +// comparing on shard rather than ID was twice as fast in informal tests +func (v *rowsByIDsSortable) Less(i, j int) bool { return v.ids[i]/v.width < v.ids[j]/v.width } +func (v *rowsByIDsSortable) Swap(i, j int) { + v.ids[i], v.ids[j] = v.ids[j], v.ids[i] + v.rows[i], v.rows[j] = v.rows[j], v.rows[i] +} + // TODO this should work for bools as well - just need to support them // at batch creation time and when calling Add, I think. func (b *Batch) importMutexData() error { @@ -1319,6 +1360,12 @@ func (b *Batch) importMutexData() error { if len(ids) == 0 { continue } + + sc := &rowsByIDsSortable{ids: ids, rows: rowIDs, width: shardWidth} + if !sort.IsSorted(sc) { + sort.Sort(sc) + } + curShard := ids[0] / shardWidth startIdx := 0 for i := 1; i <= len(ids); i++ { From 16161025f2393999bbea2b23180bc335e38ec623 Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Fri, 7 Jan 2022 15:59:54 -0600 Subject: [PATCH 4/9] trying to get sonar coverage reporting working looks like test-report.out and coverage.out aren't about the same tests. I'm unclear on how sonar uses tests.reportPaths vs coverage.reportPaths, but figured I'd try at least generating them from the same run to see if that helped. --- .gitlab/.gitlab-ci.yml | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/.gitlab/.gitlab-ci.yml b/.gitlab/.gitlab-ci.yml index 5f65fc704..c0e279b47 100644 --- a/.gitlab/.gitlab-ci.yml +++ b/.gitlab/.gitlab-ci.yml @@ -90,21 +90,10 @@ run go tests future: script: - echo "Running featurebase unit tests..." - PKG_LIST=$(go list ./... | grep -v internal/clustertests | paste -s -d, -) - - go test -coverprofile=coverage.out -covermode=atomic -coverpkg=${PKG_LIST} ./... + - go test -json -coverprofile=coverage.out -covermode=atomic -coverpkg=${PKG_LIST} ./... | tee test-report.out artifacts: paths: - coverage.out - -run go tests with output: - stage: test - image: golang:$GOVERSION - rules: - - if: '$CI_PIPELINE_SOURCE == "push" || $CI_PIPELINE_SOURCE == "schedule" || $CI_PIPELINE_SOURCE == "web"' - script: - - echo "Running featurebase unit tests to capture JSON output..." - - go test -json > test-report.out - artifacts: - paths: - test-report.out upload to sonarcloud: @@ -118,7 +107,6 @@ upload to sonarcloud: - sonar-scanner -Dsonar.projectKey=molecula_featurebase -Dsonar.organization=molecula -Dsonar.sources=. -Dsonar.host.url=https://sonarcloud.io -Dsonar.go.coverage.reportPaths=coverage.out -Dsonar.go.tests.reportPaths=test-report.out -Dsonar.javascript.lcov.reportPaths=lattice/coverage/lcov.info needs: - job: run go tests - - job: run go tests with output - job: run jest tests build for linux amd64: From 6335b9c801f44d287b2dd31231bf40da583e4afb Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Fri, 7 Jan 2022 16:34:11 -0600 Subject: [PATCH 5/9] disable retryablehttp logger because *wow* that's a lot of output --- ctl/restore.go | 3 +++ http/client.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/ctl/restore.go b/ctl/restore.go index b7f1fb2dc..9c12434f8 100644 --- a/ctl/restore.go +++ b/ctl/restore.go @@ -19,6 +19,7 @@ import ( pilosa "github.com/molecula/featurebase/v2" fb_http "github.com/molecula/featurebase/v2/http" + "github.com/molecula/featurebase/v2/logger" "github.com/molecula/featurebase/v2/server" "github.com/molecula/featurebase/v2/topology" "github.com/pkg/errors" @@ -203,6 +204,8 @@ func (cmd *RestoreCommand) newClient() *retryablehttp.Client { client.RetryWaitMin = min client.RetryMax = int(attempts) client.CheckRetry = retryWith400 + client.Logger = logger.NopLogger + return client } diff --git a/http/client.go b/http/client.go index 8d3437bb5..e1169034b 100644 --- a/http/client.go +++ b/http/client.go @@ -82,7 +82,9 @@ func WithClientRetryPeriod(period time.Duration) InternalClientOption { rc.RetryWaitMin = min rc.RetryMax = int(attempts) rc.CheckRetry = retryWith400Policy + rc.Logger = logger.NopLogger c.retryableClient = rc + } } @@ -123,6 +125,7 @@ func NewInternalClientFromURI(defaultURI *pnet.URI, remoteClient *http.Client, o rc := retryablehttp.NewClient() rc.HTTPClient = ic.httpClient rc.CheckRetry = noRetryPolicy + rc.Logger = logger.NopLogger ic.retryableClient = rc } return ic From 131f891f75fffb31af04b2acd8c275c657c638e0 Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Mon, 10 Jan 2022 08:50:23 -0600 Subject: [PATCH 6/9] fix up error messages in client batch test --- client/ingest_api_batch_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/client/ingest_api_batch_test.go b/client/ingest_api_batch_test.go index df94ae33d..75a4943d4 100644 --- a/client/ingest_api_batch_test.go +++ b/client/ingest_api_batch_test.go @@ -269,37 +269,37 @@ func TestIngestAPIBatch(t *testing.T) { if resp, err := cli.Query(NewPQLBaseQuery("Row(bint==-2)", &Index{name: "test-1", options: &IndexOptions{}}, nil)); err != nil { t.Fatalf("querying: %v", err) } else if len(resp.Result().Row().Columns) != 1 || resp.Result().Row().Columns[0] != uint64(7) { - t.Fatalf("unexpected Row(asr=a) result: %+v", resp.Result().Row().Columns) + t.Fatalf("unexpected Row(bint==-2) result: %+v", resp.Result().Row().Columns) } if resp, err := cli.Query(NewPQLBaseQuery("Row(cid=9)", &Index{name: "test-1", options: &IndexOptions{}}, nil)); err != nil { t.Fatalf("querying: %v", err) } else if len(resp.Result().Row().Columns) != 1 || resp.Result().Row().Columns[0] != uint64(7) { - t.Fatalf("unexpected Row(asr=a) result: %+v", resp.Result().Row().Columns) + t.Fatalf("unexpected Row(cid=9) result: %+v", resp.Result().Row().Columns) } if resp, err := cli.Query(NewPQLBaseQuery("Row(dtimestamp=='2010-10-18T02:07:03Z')", &Index{name: "test-1", options: &IndexOptions{}}, nil)); err != nil { t.Fatalf("querying: %v", err) } else if len(resp.Result().Row().Columns) != 1 || resp.Result().Row().Columns[0] != uint64(7) { - t.Fatalf("unexpected Row(asr=a) result: %+v", resp.Result().Row().Columns) + t.Fatalf("unexpected Row(dtimestamp=='2010-10-18T02:07:03Z') result: %+v", resp.Result().Row().Columns) } if resp, err := cli.Query(NewPQLBaseQuery("Row(etime=e, from='2010-01-01', to='2010-01-02')", &Index{name: "test-1", options: &IndexOptions{}}, nil)); err != nil { t.Fatalf("querying: %v", err) } else if len(resp.Result().Row().Columns) != 1 || resp.Result().Row().Columns[0] != uint64(7) { - t.Fatalf("unexpected Row(asr=a) result: %+v", resp.Result().Row().Columns) + t.Fatalf("unexpected Row(etime=e, from='2010-01-01', to='2010-01-02') result: %+v", resp.Result().Row().Columns) } if resp, err := cli.Query(NewPQLBaseQuery("Row(fdecimal==1.234)", &Index{name: "test-1", options: &IndexOptions{}}, nil)); err != nil { t.Fatalf("querying: %v", err) } else if len(resp.Result().Row().Columns) != 1 || resp.Result().Row().Columns[0] != uint64(7) { - t.Fatalf("unexpected Row(asr=a) result: %+v", resp.Result().Row().Columns) + t.Fatalf("unexpected Row(fdecimal==1.234) result: %+v", resp.Result().Row().Columns) } if resp, err := cli.Query(NewPQLBaseQuery("Row(gbool=true)", &Index{name: "test-1", options: &IndexOptions{}}, nil)); err != nil { t.Fatalf("querying: %v", err) } else if len(resp.Result().Row().Columns) != 1 || resp.Result().Row().Columns[0] != uint64(7) { - t.Fatalf("unexpected Row(asr=a) result: %+v", resp.Result().Row().Columns) + t.Fatalf("unexpected Row(gbool=true) result: %+v", resp.Result().Row().Columns) } } From 7fbd371038c68f2100397f166c985d410f2544b7 Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Mon, 10 Jan 2022 10:49:43 -0600 Subject: [PATCH 7/9] get some of the client tests to actually *run* discovered that client tests weren't running due to integration build tag. Fixed the file I needed to get through SonarCloud and documented rest of what needs to be done in FB-1152 https://molecula.atlassian.net/browse/FB-1152 --- client/batch_test.go | 112 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 101 insertions(+), 11 deletions(-) diff --git a/client/batch_test.go b/client/batch_test.go index f7fde4bad..86494c42d 100644 --- a/client/batch_test.go +++ b/client/batch_test.go @@ -1,21 +1,33 @@ // Copyright 2021 Molecula Corp. All rights reserved. -//go:build integration -// +build integration package client import ( + "math/rand" "reflect" "sort" "strconv" "testing" "time" + "github.com/molecula/featurebase/v2/test" + "github.com/pkg/errors" ) +func NewTestClient(t *testing.T, c *test.Cluster) *Client { + client, err := NewClient(c.Nodes[0].URL()) + if err != nil { + t.Fatal(err) + } + return client +} + func TestStringSliceCombos(t *testing.T) { - client := DefaultClient() + c := test.MustRunCluster(t, 1) + defer c.Close() + client := NewTestClient(t, c) + schema := NewSchema() idx := schema.Index("test-string-slicecombos") fields := make([]*Field, 1) @@ -153,7 +165,10 @@ func ingestRecords(records []Row, batch *Batch) error { } func TestImportBatchInts(t *testing.T) { - client := DefaultClient() + c := test.MustRunCluster(t, 1) + defer c.Close() + client := NewTestClient(t, c) + schema := NewSchema() idx := schema.Index("gopilosatest-blah") field := idx.Field("anint", OptFieldTypeInt()) @@ -212,8 +227,65 @@ func TestImportBatchInts(t *testing.T) { } } +func TestImportBatchSorting(t *testing.T) { + c := test.MustRunCluster(t, 1) + defer c.Close() + client := NewTestClient(t, c) + + schema := NewSchema() + idx := schema.Index("gopilosatest-blah") + field := idx.Field("anint", OptFieldTypeInt()) + field2 := idx.Field("amutex", OptFieldTypeMutex(CacheTypeNone, 0)) + err := client.SyncSchema(schema) + if err != nil { + t.Fatalf("syncing schema: %v", err) + } + + b, err := NewBatch(client, 100, idx, []*Field{field, field2}) + if err != nil { + t.Fatalf("getting batch: %v", err) + } + + r := Row{Values: make([]interface{}, 2)} + + rnd := rand.New(rand.NewSource(7)) + + // generate 100 records randomly spread/ordered across multiple + // shards to test sorting on int/mutex fields + for i := 0; i < 100; i++ { + id := rnd.Intn(10_000_000) + r.ID = uint64(id) + r.Values[0] = int64(id) + r.Values[1] = uint64(id) + err := b.Add(r) + if err != nil && err != ErrBatchNowFull { + t.Fatalf("adding to batch: %v", err) + } + } + err = b.Import() + if err != nil { + t.Fatalf("importing: %v", err) + } + + err = b.Import() + if err != nil { + t.Fatalf("second import: %v", err) + } + + resp, err := client.Query(idx.RawQuery("Count(All())")) + if err != nil { + t.Fatalf("querying: %v", err) + } + if res := resp.Results()[0]; res.Count() != 100 { + t.Fatalf("unexpected result: %+v", res) + } +} + func TestTrimNull(t *testing.T) { - client := DefaultClient() + c := test.MustRunCluster(t, 1) + defer c.Close() + client := NewTestClient(t, c) + schema := NewSchema() idx := schema.Index("gopilosatest-null") field := idx.Field("empty", OptFieldTypeInt()) @@ -300,7 +372,10 @@ func TestTrimNull(t *testing.T) { } func TestStringSliceEmptyAndNil(t *testing.T) { - client := DefaultClient() + c := test.MustRunCluster(t, 1) + defer c.Close() + client := NewTestClient(t, c) + schema := NewSchema() idx := schema.Index("test-string-slice-nil") fields := make([]*Field, 1) @@ -398,7 +473,10 @@ func TestStringSliceEmptyAndNil(t *testing.T) { } func TestStringSlice(t *testing.T) { - client := DefaultClient() + c := test.MustRunCluster(t, 1) + defer c.Close() + client := NewTestClient(t, c) + schema := NewSchema() idx := schema.Index("test-string-slice") fields := make([]*Field, 1) @@ -514,7 +592,10 @@ func TestStringSlice(t *testing.T) { } func TestSingleClearBatchRegression(t *testing.T) { - client := DefaultClient() + c := test.MustRunCluster(t, 1) + defer c.Close() + client := NewTestClient(t, c) + schema := NewSchema() idx := schema.Index("gopilosatest-blah") numFields := 1 @@ -566,7 +647,10 @@ func TestSingleClearBatchRegression(t *testing.T) { } func TestBatches(t *testing.T) { - client := DefaultClient() + c := test.MustRunCluster(t, 1) + defer c.Close() + client := NewTestClient(t, c) + schema := NewSchema() idx := schema.Index("gopilosatest-blah") numFields := 5 @@ -978,7 +1062,10 @@ func TestBatches(t *testing.T) { } func TestBatchesStringIDs(t *testing.T) { - client := DefaultClient() + c := test.MustRunCluster(t, 1) + defer c.Close() + client := NewTestClient(t, c) + schema := NewSchema() idx := schema.Index("gopilosatest-blah", OptIndexKeys(true)) fields := make([]*Field, 3) @@ -1264,7 +1351,10 @@ func TestQuantizedTime(t *testing.T) { } func TestBatchStaleness(t *testing.T) { - client := DefaultClient() + c := test.MustRunCluster(t, 1) + defer c.Close() + client := NewTestClient(t, c) + schema := NewSchema() idx := schema.Index("gopilosatest-blah") field := idx.Field("anint", OptFieldTypeInt()) From db87a3c4f76efc3455a59d2e109b4bc1ff4084eb Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Mon, 10 Jan 2022 10:59:44 -0600 Subject: [PATCH 8/9] fix vet shadow issue --- client/batch_test.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/client/batch_test.go b/client/batch_test.go index 86494c42d..dac3a757c 100644 --- a/client/batch_test.go +++ b/client/batch_test.go @@ -974,8 +974,8 @@ func TestBatches(t *testing.T) { } } res := results[1] - cols := res.Row().Columns - if !reflect.DeepEqual(cols, []uint64{0, 2, 4, 6, 8, 10, 12, 14, 16, 18}) { + + if cols := res.Row().Columns; !reflect.DeepEqual(cols, []uint64{0, 2, 4, 6, 8, 10, 12, 14, 16, 18}) { t.Fatalf("unexpected columns for field 1 row b: %v", cols) } @@ -1003,23 +1003,25 @@ func TestBatches(t *testing.T) { t.Fatalf("querying: %v", err) } results = resp.Results() - cols = results[0].Row().Columns - if !reflect.DeepEqual(cols, []uint64{0, 1, 2, 3, 4, 5, 6, 7, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28}) { + + if cols := results[0].Row().Columns; !reflect.DeepEqual(cols, []uint64{0, 1, 2, 3, 4, 5, 6, 7, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28}) { t.Fatalf("all columns (but 8) should be greater than -11, but got: %v", cols) } - cols = results[1].Row().Columns - if !reflect.DeepEqual(cols, []uint64{19, 21, 23, 25, 27}) { + + if cols := results[1].Row().Columns; !reflect.DeepEqual(cols, []uint64{19, 21, 23, 25, 27}) { t.Fatalf("wrong cols for ==0: %v", cols) } - cols = results[2].Row().Columns - if !reflect.DeepEqual(cols, []uint64{20, 22, 24, 26, 28}) { + + if cols := results[2].Row().Columns; !reflect.DeepEqual(cols, []uint64{20, 22, 24, 26, 28}) { t.Fatalf("wrong cols for ==100: %v", cols) } - cols = results[3].Row().Columns + + cols := results[3].Row().Columns exp := []uint64{0, 2, 4, 6, 10, 12, 14, 16, 18} if !reflect.DeepEqual(cols, exp) { t.Fatalf("wrong cols for January: got/want\n%v\n%v", cols, exp) } + cols = results[4].Row().Columns exp = []uint64{1, 3, 5, 7} if !reflect.DeepEqual(cols, exp) { From 48b4169cb50a0162beaadc74ec7633ad6549de31 Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Mon, 10 Jan 2022 13:34:30 -0600 Subject: [PATCH 9/9] refactor client batch tests to reduce duplication also use a single cluster with each test creating a different index rather than each test creating a whole new cluster. runtime went from 38s to 30s in my informal tests --- client/batch_test.go | 85 ++++++++++++++++---------------------------- http/client.go | 3 +- 2 files changed, 32 insertions(+), 56 deletions(-) diff --git a/client/batch_test.go b/client/batch_test.go index dac3a757c..cc8ab891f 100644 --- a/client/batch_test.go +++ b/client/batch_test.go @@ -23,13 +23,26 @@ func NewTestClient(t *testing.T, c *test.Cluster) *Client { return client } -func TestStringSliceCombos(t *testing.T) { +func TestAgainstCluster(t *testing.T) { c := test.MustRunCluster(t, 1) defer c.Close() client := NewTestClient(t, c) + t.Run("string-slice-combos", func(t *testing.T) { testStringSliceCombos(t, c, client) }) + t.Run("import-batch-ints", func(t *testing.T) { testImportBatchInts(t, c, client) }) + t.Run("import-batch-sorting", func(t *testing.T) { testImportBatchSorting(t, c, client) }) + t.Run("test-trim-null", func(t *testing.T) { testTrimNull(t, c, client) }) + t.Run("test-string-slice-empty-and-nil", func(t *testing.T) { testStringSliceEmptyAndNil(t, c, client) }) + t.Run("test-string-slice", func(t *testing.T) { testStringSlice(t, c, client) }) + t.Run("test-single-clear-batch-regression", func(t *testing.T) { testSingleClearBatchRegression(t, c, client) }) + t.Run("test-batches", func(t *testing.T) { testBatches(t, c, client) }) + t.Run("batches-strings-ids", func(t *testing.T) { testBatchesStringIDs(t, c, client) }) + t.Run("test-batch-staleness", func(t *testing.T) { testBatchStaleness(t, c, client) }) +} + +func testStringSliceCombos(t *testing.T, c *test.Cluster, client *Client) { schema := NewSchema() - idx := schema.Index("test-string-slicecombos") + idx := schema.Index("test-string-slice-combos") fields := make([]*Field, 1) fields[0] = idx.Field("a1", OptFieldKeys(true), OptFieldTypeSet(CacheTypeRanked, 100)) err := client.SyncSchema(schema) @@ -164,13 +177,9 @@ func ingestRecords(records []Row, batch *Batch) error { return nil } -func TestImportBatchInts(t *testing.T) { - c := test.MustRunCluster(t, 1) - defer c.Close() - client := NewTestClient(t, c) - +func testImportBatchInts(t *testing.T, c *test.Cluster, client *Client) { schema := NewSchema() - idx := schema.Index("gopilosatest-blah") + idx := schema.Index("test-import-batch-ints") field := idx.Field("anint", OptFieldTypeInt()) err := client.SyncSchema(schema) if err != nil { @@ -227,13 +236,9 @@ func TestImportBatchInts(t *testing.T) { } } -func TestImportBatchSorting(t *testing.T) { - c := test.MustRunCluster(t, 1) - defer c.Close() - client := NewTestClient(t, c) - +func testImportBatchSorting(t *testing.T, c *test.Cluster, client *Client) { schema := NewSchema() - idx := schema.Index("gopilosatest-blah") + idx := schema.Index("test-import-batch-sorting") field := idx.Field("anint", OptFieldTypeInt()) field2 := idx.Field("amutex", OptFieldTypeMutex(CacheTypeNone, 0)) err := client.SyncSchema(schema) @@ -281,13 +286,9 @@ func TestImportBatchSorting(t *testing.T) { } } -func TestTrimNull(t *testing.T) { - c := test.MustRunCluster(t, 1) - defer c.Close() - client := NewTestClient(t, c) - +func testTrimNull(t *testing.T, c *test.Cluster, client *Client) { schema := NewSchema() - idx := schema.Index("gopilosatest-null") + idx := schema.Index("test-trim-null") field := idx.Field("empty", OptFieldTypeInt()) err := client.SyncSchema(schema) if err != nil { @@ -371,11 +372,7 @@ func TestTrimNull(t *testing.T) { } -func TestStringSliceEmptyAndNil(t *testing.T) { - c := test.MustRunCluster(t, 1) - defer c.Close() - client := NewTestClient(t, c) - +func testStringSliceEmptyAndNil(t *testing.T, c *test.Cluster, client *Client) { schema := NewSchema() idx := schema.Index("test-string-slice-nil") fields := make([]*Field, 1) @@ -472,11 +469,7 @@ func TestStringSliceEmptyAndNil(t *testing.T) { } -func TestStringSlice(t *testing.T) { - c := test.MustRunCluster(t, 1) - defer c.Close() - client := NewTestClient(t, c) - +func testStringSlice(t *testing.T, c *test.Cluster, client *Client) { schema := NewSchema() idx := schema.Index("test-string-slice") fields := make([]*Field, 1) @@ -591,13 +584,9 @@ func TestStringSlice(t *testing.T) { } } -func TestSingleClearBatchRegression(t *testing.T) { - c := test.MustRunCluster(t, 1) - defer c.Close() - client := NewTestClient(t, c) - +func testSingleClearBatchRegression(t *testing.T, c *test.Cluster, client *Client) { schema := NewSchema() - idx := schema.Index("gopilosatest-blah") + idx := schema.Index("test-single-clear-batch-regression") numFields := 1 fields := make([]*Field, numFields) fields[0] = idx.Field("zero", OptFieldKeys(true)) @@ -646,13 +635,9 @@ func TestSingleClearBatchRegression(t *testing.T) { } -func TestBatches(t *testing.T) { - c := test.MustRunCluster(t, 1) - defer c.Close() - client := NewTestClient(t, c) - +func testBatches(t *testing.T, c *test.Cluster, client *Client) { schema := NewSchema() - idx := schema.Index("gopilosatest-blah") + idx := schema.Index("test-batches") numFields := 5 fields := make([]*Field, numFields) fields[0] = idx.Field("zero", OptFieldKeys(true)) @@ -1063,13 +1048,9 @@ func TestBatches(t *testing.T) { // TODO test importing across multiple shards } -func TestBatchesStringIDs(t *testing.T) { - c := test.MustRunCluster(t, 1) - defer c.Close() - client := NewTestClient(t, c) - +func testBatchesStringIDs(t *testing.T, c *test.Cluster, client *Client) { schema := NewSchema() - idx := schema.Index("gopilosatest-blah", OptIndexKeys(true)) + idx := schema.Index("batches-strings-ids", OptIndexKeys(true)) fields := make([]*Field, 3) fields[0] = idx.Field("zero", OptFieldKeys(true)) fields[1] = idx.Field("one", OptFieldTypeMutex(CacheTypeNone, 0), OptFieldKeys(true)) @@ -1352,13 +1333,9 @@ func TestQuantizedTime(t *testing.T) { } -func TestBatchStaleness(t *testing.T) { - c := test.MustRunCluster(t, 1) - defer c.Close() - client := NewTestClient(t, c) - +func testBatchStaleness(t *testing.T, c *test.Cluster, client *Client) { schema := NewSchema() - idx := schema.Index("gopilosatest-blah") + idx := schema.Index("test-batch-staleness") field := idx.Field("anint", OptFieldTypeInt()) err := client.SyncSchema(schema) if err != nil { diff --git a/http/client.go b/http/client.go index e1169034b..6ef35992a 100644 --- a/http/client.go +++ b/http/client.go @@ -75,7 +75,7 @@ func WithClientRetryPeriod(period time.Duration) InternalClientOption { if attempts < 1 { attempts = 1 } - fmt.Println("attempts: ", int(attempts)) + return func(c *InternalClient) { rc := retryablehttp.NewClient() rc.HTTPClient = c.httpClient @@ -84,7 +84,6 @@ func WithClientRetryPeriod(period time.Duration) InternalClientOption { rc.CheckRetry = retryWith400Policy rc.Logger = logger.NopLogger c.retryableClient = rc - } }