From 14319cc54bc08d34551221edf9832dcfe78feca0 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Thu, 2 Apr 2020 12:12:07 -0500 Subject: [PATCH 1/6] Use "make test-race" instead of custom test flags in CI --- .circleci/config.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index ca992bcfe..dc8aa1251 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -70,6 +70,9 @@ jobs: shard_width: type: string default: "20" + test_make_target: + type: string + default: "test" test_flags: type: string default: "" @@ -85,7 +88,7 @@ jobs: - add-github-auth - run: sudo apt-get install lsof - run: - command: make test SHARD_WIDTH=<< parameters.shard_width >> TESTFLAGS="<< parameters.test_flags >>" GOARCH=<< parameters.goarch >> + command: make << parameters.test_make_target >> SHARD_WIDTH=<< parameters.shard_width >> GOARCH=<< parameters.goarch >> no_output_timeout: 30m cluster-tests: executor: @@ -187,7 +190,7 @@ workflows: - setup - test: name: test-race - test_flags: -race -v -timeout=30m + test_make_target: test-race requires: - setup - test: From 6a639efdba92a348681d374c2028e64dc22524f2 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Thu, 2 Apr 2020 12:12:17 -0500 Subject: [PATCH 2/6] Modify Docker Hub deployment filter rules. The documentation is unclear/incorrect, and these filters aren't behaving correctly. This is an attempt at fixing that. More info at: https://discuss.circleci.com/t/job-runs-even-when-tags-ignore-filter-is-triggered-when-combined-with-branches-only/20664/11 --- .circleci/config.yml | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index dc8aa1251..dd72390e8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -211,20 +211,6 @@ workflows: - linter - check-license-headers - test-golang-1.14 - - dockerhub-upload: - name: dockerhub-upload-stable - tag_branch: true - tag_tag: true - tag_latest: true - requires: - - linter - - check-license-headers - - test-golang-1.14 - filters: - branches: - ignore: /.*/ - tags: - only: /^v.*/ - dockerhub-upload: name: dockerhub-upload-unstable tag_branch: true @@ -237,5 +223,17 @@ workflows: filters: branches: only: enterprise + - dockerhub-upload: + name: dockerhub-upload-stable + tag_branch: true + tag_tag: true + tag_latest: true + requires: + - linter + - check-license-headers + - test-golang-1.14 + filters: tags: - ignore: /^v.*/ + only: /^v.*/ + branches: + ignore: /.*/ From 182c1d3c42cf8ea3093f36e7c0041143523a498a Mon Sep 17 00:00:00 2001 From: Travis Date: Tue, 31 Mar 2020 23:05:59 -0500 Subject: [PATCH 3/6] alter tests to allow for shardwidth22 also, reset BitDepth on field and bsiGroup during importRoaringOverwrite --- boltdb/translate_test.go | 64 +++++++++++++++++++++------------------ field.go | 24 +++++++++++++++ fragment.go | 6 ++++ fragment_internal_test.go | 16 +++++++++- server/handler_test.go | 24 +++++++++++++-- 5 files changed, 101 insertions(+), 33 deletions(-) diff --git a/boltdb/translate_test.go b/boltdb/translate_test.go index 9ddb68907..ef9a726b5 100644 --- a/boltdb/translate_test.go +++ b/boltdb/translate_test.go @@ -32,23 +32,22 @@ func TestTranslateStore_TranslateKey(t *testing.T) { defer MustCloseTranslateStore(s) // Ensure initial key translates to first ID for shard - if id, err := s.TranslateKey("foo"); err != nil { + id1, err := s.TranslateKey("foo") + if err != nil { t.Fatal(err) - } else if got, want := id, uint64(247463937); got != want { - t.Fatalf("TranslateKey()=%d, want %d", got, want) } // Ensure next key autoincrements. if id, err := s.TranslateKey("bar"); err != nil { t.Fatal(err) - } else if got, want := id, uint64(247463938); got != want { + } else if got, want := id, id1+1; got != want { t.Fatalf("TranslateKey()=%d, want %d", got, want) } // Ensure retranslating existing key returns original ID. if id, err := s.TranslateKey("foo"); err != nil { t.Fatal(err) - } else if got, want := id, uint64(247463937); got != want { + } else if got, want := id, id1; got != want { t.Fatalf("TranslateKey()=%d, want %d", got, want) } } @@ -58,31 +57,30 @@ func TestTranslateStore_TranslateKeys(t *testing.T) { defer MustCloseTranslateStore(s) // Ensure initial keys translate to incrementing IDs. - if ids, err := s.TranslateKeys([]string{"foo", "bar"}); err != nil { + ids1, err := s.TranslateKeys([]string{"foo", "bar"}) + if err != nil { t.Fatal(err) - } else if got, want := ids[0], uint64(247463937); got != want { - t.Fatalf("TranslateKeys()[0]=%d, want %d", got, want) - } else if got, want := ids[1], uint64(247463938); got != want { + } else if got, want := ids1[1], ids1[0]+1; got != want { t.Fatalf("TranslateKeys()[1]=%d, want %d", got, want) } // Ensure retranslation returns original IDs. if ids, err := s.TranslateKeys([]string{"foo", "bar"}); err != nil { t.Fatal(err) - } else if got, want := ids[0], uint64(247463937); got != want { + } else if got, want := ids[0], ids1[0]; got != want { t.Fatalf("TranslateKeys()[0]=%d, want %d", got, want) - } else if got, want := ids[1], uint64(247463938); got != want { + } else if got, want := ids[1], ids1[1]; got != want { t.Fatalf("TranslateKeys()[1]=%d, want %d", got, want) } // Ensure retranslating with existing and non-existing keys returns correctly. if ids, err := s.TranslateKeys([]string{"foo", "baz", "bar"}); err != nil { t.Fatal(err) - } else if got, want := ids[0], uint64(247463937); got != want { + } else if got, want := ids[0], ids1[0]; got != want { t.Fatalf("TranslateKeys()[0]=%d, want %d", got, want) - } else if got, want := ids[1], uint64(247463939); got != want { + } else if got, want := ids[1], ids1[0]+2; got != want { t.Fatalf("TranslateKeys()[1]=%d, want %d", got, want) - } else if got, want := ids[2], uint64(247463938); got != want { + } else if got, want := ids[2], ids1[1]; got != want { t.Fatalf("TranslateKeys()[2]=%d, want %d", got, want) } } @@ -154,7 +152,8 @@ func TestTranslateStore_EntryReader(t *testing.T) { defer MustCloseTranslateStore(s) // Create multiple new keys. - if _, err := s.TranslateKeys([]string{"foo", "bar"}); err != nil { + ids1, err := s.TranslateKeys([]string{"foo", "bar"}) + if err != nil { t.Fatal(err) } @@ -169,7 +168,7 @@ func TestTranslateStore_EntryReader(t *testing.T) { // Read first entry. if err := r.ReadEntry(&entry); err != nil { t.Fatal(err) - } else if got, want := entry.ID, uint64(247463937); got != want { + } else if got, want := entry.ID, ids1[0]; got != want { t.Fatalf("ReadEntry() ID=%d, want %d", got, want) } else if got, want := entry.Key, "foo"; got != want { t.Fatalf("ReadEntry() Key=%s, want %s", got, want) @@ -178,21 +177,22 @@ func TestTranslateStore_EntryReader(t *testing.T) { // Read next entry. if err := r.ReadEntry(&entry); err != nil { t.Fatal(err) - } else if got, want := entry.ID, uint64(247463938); got != want { + } else if got, want := entry.ID, ids1[1]; got != want { t.Fatalf("ReadEntry() ID=%d, want %d", got, want) } else if got, want := entry.Key, "bar"; got != want { t.Fatalf("ReadEntry() Key=%s, want %s", got, want) } // Insert next key while reader is open. - if _, err := s.TranslateKey("baz"); err != nil { + id2, err := s.TranslateKey("baz") + if err != nil { t.Fatal(err) } // Read newly created entry. if err := r.ReadEntry(&entry); err != nil { t.Fatal(err) - } else if got, want := entry.ID, uint64(247463939); got != want { + } else if got, want := entry.ID, id2; got != want { t.Fatalf("ReadEntry() ID=%d, want %d", got, want) } else if got, want := entry.Key, "baz"; got != want { t.Fatalf("ReadEntry() Key=%s, want %s", got, want) @@ -216,20 +216,25 @@ func TestTranslateStore_EntryReader(t *testing.T) { } defer r.Close() + // cache holds the translated key id so we can check it later + cache := make(chan uint64) + // Insert key in separate goroutine. // Sleep momentarily to reader hangs. translateErr := make(chan error) go func() { time.Sleep(100 * time.Millisecond) - if _, err := s.TranslateKey("foo"); err != nil { + id, err := s.TranslateKey("foo") + if err != nil { translateErr <- err } + cache <- id }() var entry pilosa.TranslateEntry if err := r.ReadEntry(&entry); err != nil { t.Fatal(err) - } else if got, want := entry.ID, uint64(247463937); got != want { + } else if got, want := entry.ID, <-cache; got != want { t.Fatalf("ReadEntry() ID=%d, want %d", got, want) } else if got, want := entry.Key, "foo"; got != want { t.Fatalf("ReadEntry() Key=%s, want %s", got, want) @@ -340,7 +345,8 @@ func TestTranslateStore_ReadWrite(t *testing.T) { } // Populate the store with the keys in batch0. - if _, err := s.TranslateKeys(batch0); err != nil { + batch0IDs, err := s.TranslateKeys(batch0) + if err != nil { t.Fatal(err) } @@ -356,11 +362,12 @@ func TestTranslateStore_ReadWrite(t *testing.T) { } // Populate the store with the keys in batch1. - if _, err := s.TranslateKeys(batch1); err != nil { + batch1IDs, err := s.TranslateKeys(batch1) + if err != nil { t.Fatal(err) } - expIDs := []uint64{247463987, 247464087} + expIDs := []uint64{batch0IDs[50], batch1IDs[50]} // Check the IDs for a key from each batch. if ids, err := s.TranslateKeys([]string{"key50", "key150"}); err != nil { @@ -378,13 +385,12 @@ func TestTranslateStore_ReadWrite(t *testing.T) { // This time, we expect the second key to be different because // we overwrote the store, and then just set that key. - expIDs = []uint64{247463987, 247464037} - - // Check the IDs for a key from each batch. if ids, err := s.TranslateKeys([]string{"key50", "key150"}); err != nil { t.Fatal(err) - } else if !reflect.DeepEqual(expIDs, ids) { - t.Fatalf("last expected ids: %v, but got: %v", expIDs, ids) + } else if ids[0] != expIDs[0] { + t.Fatalf("last expected ids[0]: %d, but got: %d", expIDs[0], ids[0]) + } else if ids[1] == expIDs[1] { + t.Fatalf("last expected different ids[1]: %d, but got: %d", expIDs[1], ids[1]) } }) } diff --git a/field.go b/field.go index 7c3de18ad..f18c15918 100644 --- a/field.go +++ b/field.go @@ -1839,6 +1839,30 @@ func (f *Field) importRoaringOverwrite(ctx context.Context, data []byte, shard u return err } + // If field is int or decimal, then we need to update field.options.BitDepth + // and bsiGroup.BitDepth based on the imported data. + switch f.Options().Type { + case FieldTypeInt, FieldTypeDecimal: + frag.calculateMaxRowID() + maxRowID, _ := frag.maxRow(nil) + + var bitDepth uint + if maxRowID+1 > bsiOffsetBit { + bitDepth = uint(maxRowID + 1 - bsiOffsetBit) + } + + bsig := f.bsiGroup(f.name) + + f.mu.Lock() + defer f.mu.Unlock() + if bitDepth > f.options.BitDepth { + f.options.BitDepth = bitDepth + } + if bsig != nil { + bsig.BitDepth = bitDepth + } + } + return nil } diff --git a/fragment.go b/fragment.go index 28c524025..c89d25c6d 100644 --- a/fragment.go +++ b/fragment.go @@ -1142,6 +1142,12 @@ func (f *fragment) maxRow(filter *Row) (uint64, uint64) { return 0, 0 } +// calculateMaxRowID determines the field's maxRowID value based +// on the contents of its storage, and sets the struct argument. +func (f *fragment) calculateMaxRowID() { + f.maxRowID = f.storage.Max() / ShardWidth +} + // rangeOp returns bitmaps with a bsiGroup value encoding matching the predicate. func (f *fragment) rangeOp(op pql.Token, bitDepth uint, predicate int64) (*Row, error) { switch op { diff --git a/fragment_internal_test.go b/fragment_internal_test.go index 0b7ed8fd4..9eee27f8f 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -3641,7 +3641,21 @@ func TestRemapCache(t *testing.T) { func TestFragment_Bug_Q2DoubleDelete(t *testing.T) { f := mustOpenFragment("i", "f", viewStandard, 0, "") - b := []byte{60, 48, 0, 0, 1, 0, 0, 0, 16, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 24, 0, 0, 0, 1, 0} + + // byShardWidth is a map of the same roaring (fragment) data generated + // with different shard widths. + // TODO: a better approach may be to generate this in the test based + // on shard width. + byShardWidth := make(map[uint64][]byte) + // row/col: 1/1 + byShardWidth[1<<20] = []byte{60, 48, 0, 0, 1, 0, 0, 0, 16, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 24, 0, 0, 0, 1, 0} + byShardWidth[1<<22] = []byte{60, 48, 0, 0, 1, 0, 0, 0, 64, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 24, 0, 0, 0, 1, 0} + + var b []byte + if data, ok := byShardWidth[ShardWidth]; ok { + b = data + } + defer f.Clean(t) err := f.importRoaringT(b, false) if err != nil { diff --git a/server/handler_test.go b/server/handler_test.go index 31e084243..b01b15c59 100644 --- a/server/handler_test.go +++ b/server/handler_test.go @@ -240,7 +240,20 @@ func TestHandler_Endpoints(t *testing.T) { t.Fatal(err) } w := httptest.NewRecorder() - roaringData, _ := hex.DecodeString("3C30000002000000000000000000000001000000200000000000000001000000280000002A00000001000100") + + // byShardWidth is a map of the same roaring (fragment) data generated + // with different shard widths. + // TODO: a better approach may be to generate this in the test based + // on shard width. + byShardWidth := make(map[uint64][]byte) + // col/val: 3/3, 8/8 + byShardWidth[1<<20] = []byte{60, 48, 0, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1, 0, 32, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 48, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 80, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 72, 0, 0, 0, 76, 0, 0, 0, 78, 0, 0, 0, 80, 0, 0, 0, 3, 0, 8, 0, 3, 0, 3, 0, 8, 0} + byShardWidth[1<<22] = []byte{60, 48, 0, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1, 0, 128, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 192, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 64, 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 72, 0, 0, 0, 76, 0, 0, 0, 78, 0, 0, 0, 80, 0, 0, 0, 3, 0, 8, 0, 3, 0, 3, 0, 8, 0} + + var roaringData []byte + if data, ok := byShardWidth[pilosa.ShardWidth]; ok { + roaringData = data + } msg := pilosa.ImportRoaringRequest{ Action: pilosa.RequestActionOverwrite, @@ -263,7 +276,7 @@ func TestHandler_Endpoints(t *testing.T) { if err != nil { t.Fatalf("querying: %v", err) } - if row := resp.Results[0].(*pilosa.Row); !reflect.DeepEqual(row.Columns(), []uint64{1}) { + if row := resp.Results[0].(*pilosa.Row); !reflect.DeepEqual(row.Columns(), []uint64{3, 8}) { t.Fatalf("Unexpected result %v", row.Columns()) } }) @@ -1068,7 +1081,12 @@ func TestHandler_Endpoints(t *testing.T) { if w.Code != gohttp.StatusOK { t.Fatalf("unexpected status code: %d", w.Code) } - target := []uint64{162529281, 159383553, 160432129} + var target []uint64 + if pilosa.ShardWidth == 1<<22 { + target = []uint64{650117121, 637534209, 641728513} + } else { + target = []uint64{162529281, 159383553, 160432129} + } resp := pilosa.TranslateKeysResponse{} err = cmd.API.Serializer.Unmarshal(w.Body.Bytes(), &resp) if err != nil { From f15d031c553eb062214dab3122ebb8c5cfd4ecb4 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 2 Apr 2020 19:05:55 -0500 Subject: [PATCH 4/6] Add options to prometheus client to support setting namespace --- prometheus/prometheus.go | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/prometheus/prometheus.go b/prometheus/prometheus.go index cc6392c77..0a9947151 100644 --- a/prometheus/prometheus.go +++ b/prometheus/prometheus.go @@ -26,8 +26,7 @@ import ( ) const ( - // namespace is prepended to each metric event name with "_" - namespace = "pilosa" + defaultNamespace = "pilosa" ) // Ensure client implements interface. @@ -46,11 +45,22 @@ type prometheusClient struct { gaugeVecs map[string]*prometheus.GaugeVec observers map[string]prometheus.Observer summaryVecs map[string]*prometheus.SummaryVec + namespace string +} + +// ClientOption is a functional option type for prometheusClient +type ClientOption func(c *prometheusClient) + +// OptClientPrefix is a functional option on prometheusClient used to set the namespace +func OptClientNamespace(namespace string) ClientOption { + return func(c *prometheusClient) { + c.namespace = namespace + } } // NewPrometheusClient returns a new instance of StatsClient. -func NewPrometheusClient() (*prometheusClient, error) { - return &prometheusClient{ +func NewPrometheusClient(opts ...ClientOption) (*prometheusClient, error) { + client := &prometheusClient{ logger: logger.NopLogger, counters: make(map[string]prometheus.Counter), counterVecs: make(map[string]*prometheus.CounterVec), @@ -58,7 +68,14 @@ func NewPrometheusClient() (*prometheusClient, error) { gaugeVecs: make(map[string]*prometheus.GaugeVec), observers: make(map[string]prometheus.Observer), summaryVecs: make(map[string]*prometheus.SummaryVec), - }, nil + namespace: defaultNamespace, + } + + for _, opt := range opts { + opt(client) + } + + return client, nil } // Open no-op to satisfy interface @@ -103,7 +120,7 @@ func (c *prometheusClient) Count(name string, value int64, rate float64) { name = strings.Replace(name, ".", "_", -1) labels := c.labels() opts := prometheus.CounterOpts{ - Namespace: namespace, + Namespace: c.namespace, Name: name, } if len(labels) == 0 { @@ -152,7 +169,7 @@ func (c *prometheusClient) Gauge(name string, value float64, rate float64) { name = strings.Replace(name, ".", "_", -1) labels := c.labels() opts := prometheus.GaugeOpts{ - Namespace: namespace, + Namespace: c.namespace, Name: name, } if len(labels) == 0 { @@ -193,7 +210,7 @@ func (c *prometheusClient) Histogram(name string, value float64, rate float64) { name = strings.Replace(name, ".", "_", -1) labels := c.labels() opts := prometheus.SummaryOpts{ - Namespace: namespace, + Namespace: c.namespace, Name: name, Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, } From 110d2b6024ca9aa90fa754c595406612724ba3ce Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 2 Apr 2020 19:07:43 -0500 Subject: [PATCH 5/6] Restore lost comment --- prometheus/prometheus.go | 1 + 1 file changed, 1 insertion(+) diff --git a/prometheus/prometheus.go b/prometheus/prometheus.go index 0a9947151..f79d24655 100644 --- a/prometheus/prometheus.go +++ b/prometheus/prometheus.go @@ -26,6 +26,7 @@ import ( ) const ( + // namespace is prepended to each metric event name with "_" defaultNamespace = "pilosa" ) From dad4ccf103f83e9d4fe64e9ed8474017c238b667 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 2 Apr 2020 19:29:46 -0500 Subject: [PATCH 6/6] Propagate namespace to tags client --- prometheus/prometheus.go | 1 + 1 file changed, 1 insertion(+) diff --git a/prometheus/prometheus.go b/prometheus/prometheus.go index f79d24655..2057bde3a 100644 --- a/prometheus/prometheus.go +++ b/prometheus/prometheus.go @@ -108,6 +108,7 @@ func (c *prometheusClient) WithTags(tags ...string) stats.StatsClient { gaugeVecs: c.gaugeVecs, observers: c.observers, summaryVecs: c.summaryVecs, + namespace: c.namespace, } }