From 8a95ac344bfc3d1ddf246bef47bc63bc420d42c8 Mon Sep 17 00:00:00 2001 From: Souhaila Noor Date: Fri, 25 Feb 2022 09:28:13 -0600 Subject: [PATCH 01/34] We check for incomplete deletion when server is started. When deletion is started, _exists field is updated with row+1. After deletion is completed, we delete _exists=row+1. If _exists>=1, then deletion was not completed. Updated go version in docker to match other requirements. Removed duplicate error check for grpc. --- executor.go | 24 ++++---- executor_internal_test.go | 50 ++++++++++++++++ holder.go | 47 +++++++++++++++ holder_internal_test.go | 74 ++++++++++++++++++++++++ internal/clustertests/Dockerfile-fakeIDP | 2 +- server/server.go | 4 -- 6 files changed, 185 insertions(+), 16 deletions(-) diff --git a/executor.go b/executor.go index cd0fe44b4..7dfc994f2 100644 --- a/executor.go +++ b/executor.go @@ -8265,10 +8265,6 @@ func (e *executor) executeDeleteRecordFromShard(ctx context.Context, qcx *Qcx, i if len(row.segments) == 0 { //nothing to remove return false, nil } - columns := row.segments[0].data //should only be one segment - if columns.Count() == 0 { - return false, nil - } // Fetch index. idx := e.Holder.Index(index) @@ -8276,14 +8272,20 @@ func (e *executor) executeDeleteRecordFromShard(ctx context.Context, qcx *Qcx, i return false, newNotFoundError(ErrIndexNotFound, index) } + return DeleteRows(row, idx, shard) +} + +func DeleteRows(row *Row, idx *Index, shard uint64) (bool, error) { + tx := idx.holder.txf.NewTx(Txo{Write: writable, Index: idx, Shard: shard}) + defer tx.Rollback() + + columns := row.segments[0].data //should only be one segment + if columns.Count() == 0 { + return false, nil + } columnIDs := make([]uint64, 0) none := make([]uint64, 0) // no bits will be set - tx, finisher, err := qcx.GetTx(Txo{Write: writable, Index: idx, Shard: shard}) - if err != nil { - return false, err - } - defer finisher(&err) changed := false colCounts := make([]int, 0) toClear := columnIDs[:0] @@ -8301,7 +8303,7 @@ func (e *executor) executeDeleteRecordFromShard(ctx context.Context, qcx *Qcx, i toClear = columnIDs[:0] rowSet = make(map[uint64]struct{}) - err = tx.ApplyFilter(frag.index(), frag.field(), frag.view(), frag.shard, 0, findExisting) + err := tx.ApplyFilter(frag.index(), frag.field(), frag.view(), frag.shard, 0, findExisting) if err != nil { return false, err } @@ -8332,5 +8334,5 @@ func (e *executor) executeDeleteRecordFromShard(ctx context.Context, qcx *Qcx, i } } } - return changed, nil + return changed, tx.Commit() } diff --git a/executor_internal_test.go b/executor_internal_test.go index 171cb2ae3..628b63155 100644 --- a/executor_internal_test.go +++ b/executor_internal_test.go @@ -545,3 +545,53 @@ func TestDistinctTimestampUnion(t *testing.T) { }) } } + +func TestExecutor_DeleteRows(t *testing.T) { + path, _ := testhook.TempDir(t, "pilosa-executor-") + holder := NewHolder(path, mustHolderConfig()) + defer holder.Close() + + if err := holder.Open(); err != nil { + t.Fatalf("opening holder: %v", err) + } + + idx, err := holder.CreateIndex("i", IndexOptions{TrackExistence: true}) + if err != nil { + t.Fatalf("creating index: %v", err) + } + + f, err := idx.CreateField("f", OptFieldTypeDefault()) + if err != nil { + t.Fatalf("creating field: %v", err) + } + + shard := uint64(0) + tx := idx.holder.txf.NewTx(Txo{Write: writable, Index: idx, Shard: shard}) + defer tx.Rollback() + + if _, err = f.SetBit(tx, 1, 1, nil); err != nil { + t.Fatalf("setting bit: %v", err) + } + + if err := tx.Commit(); err != nil { + t.Fatalf("failed to commit transaction: %v", err) + } + + tx = idx.holder.txf.NewTx(Txo{Write: !writable, Index: idx, Shard: shard}) + defer tx.Rollback() + + row, err := f.Row(tx, 1) + if err != nil { + t.Fatalf("failed to read row: %v", err) + } + + changed, err := DeleteRows(row, idx, shard) + if !changed || err != nil { + t.Fatalf("failed to delete row: %v", err) + } + + changed, err = DeleteRows(row, idx, shard) + if changed { + t.Fatalf("expected delete to not clear bit but it did") + } +} diff --git a/holder.go b/holder.go index db38a6db7..2f24793ee 100644 --- a/holder.go +++ b/holder.go @@ -287,6 +287,50 @@ func (h *Holder) IndexesPath() string { return filepath.Join(h.path, IndexesDir) } +// processDeleteInflight checks if deletion was in progress when server shutdown +// the _exists field is set to row+1 when delete is started. Upon completion, the row is deleted. +// if _exists>=1, we finish deleting the rows +func (h *Holder) processDeleteInflight() error { + for _, index := range h.indexes { + if index.trackExistence { + shards := index.AvailableShards(includeRemote).Slice() + + for _, shard := range shards { + inprocessRowIDs := NewRow() + + frag := h.fragment(index.name, existenceFieldName, viewStandard, shard) + if frag == nil { + continue + } + + tx := index.Txf().NewTx(Txo{Write: !writable, Index: index, Shard: shard}) + defer tx.Rollback() + + // filter rows based on having _exists>=1, which is used to flag delete in-flight + rows, err := frag.rows(context.Background(), tx, 1) + if err != nil { + return err + } + + // check if any rows are found + if len(rows) == 0 { + return nil + } + + for _, rowID := range rows { + row, err2 := frag.row(tx, rowID) + if err2 != nil { + return err2 + } + inprocessRowIDs = inprocessRowIDs.Union(row) + } + DeleteRows(inprocessRowIDs, index, shard) + } + } + } + return nil +} + // Open initializes the root data directory for the holder. func (h *Holder) Open() error { h.opening = true @@ -380,6 +424,9 @@ func (h *Holder) Open() error { return errors.Wrap(err, "processing foreign index fields") } + // Check if deletion was in progress when server was shutdown + h.processDeleteInflight() + h.Stats.Open() h.opened.Close() diff --git a/holder_internal_test.go b/holder_internal_test.go index c9e164f27..2fec338af 100644 --- a/holder_internal_test.go +++ b/holder_internal_test.go @@ -2,7 +2,10 @@ package pilosa import ( + "testing" + "github.com/molecula/featurebase/v3/disco" + "github.com/molecula/featurebase/v3/testhook" ) // mustHolderConfig sets up a default holder config for tests. @@ -14,3 +17,74 @@ func mustHolderConfig() *HolderConfig { cfg.Sharder = disco.InMemSharder return cfg } + +func TestHolder_ProcessDeleteInflight(t *testing.T) { + path, _ := testhook.TempDir(t, "delete-inflight") + h := NewHolder(path, mustHolderConfig()) + defer h.Close() + + err := h.Open() + if err != nil { + t.Fatalf("failed to open holder: %v", err) + } + + idx, err := h.CreateIndexIfNotExists("i", IndexOptions{TrackExistence: true}) + if err != nil { + t.Fatalf("failed to create index: %v", err) + } + f, err := idx.CreateFieldIfNotExists("f", OptFieldTypeDefault()) + if err != nil { + t.Fatalf("failed to create field: %v", err) + } + + existencefield := idx.existenceFld + shard := uint64(0) + tx := idx.Txf().NewTx(Txo{Write: true, Index: idx, Shard: shard}) + defer tx.Rollback() + + rowCol := []struct { + row uint64 + col uint64 + }{ + {1, 1}, + {1, 2}, + {30, 33}, + {22, 2}, + } + for _, r := range rowCol { + _, err = f.SetBit(tx, r.row, r.col, nil) + if err != nil { + t.Fatalf("failed to set bit: %v", err) + } + + _, err = existencefield.SetBit(tx, r.row, r.col, nil) + if err != nil { + t.Fatalf("failed to set bit: %v", err) + } + } + + if err = tx.Commit(); err != nil { + t.Fatalf("failed to commit tx: %v", err) + } + + err = h.processDeleteInflight() + if err != nil { + t.Fatalf("failed to delete: %v", err) + } + + tx = idx.Txf().NewTx(Txo{Write: false, Index: idx, Shard: shard}) + defer tx.Rollback() + for _, r := range rowCol { + row, err := f.Row(tx, r.row) + if err != nil { + t.Fatalf("failed to get row: %v", err) + } + existenceRow, err := existencefield.Row(tx, r.row) + if err != nil { + t.Fatalf("failed to get row: %v", err) + } + if len(row.Columns()) != 0 || len(existenceRow.Columns()) != 0 { + t.Fatalf("expected columns for fields to be empty after delete") + } + } +} diff --git a/internal/clustertests/Dockerfile-fakeIDP b/internal/clustertests/Dockerfile-fakeIDP index b53d4d2c2..b46556b80 100644 --- a/internal/clustertests/Dockerfile-fakeIDP +++ b/internal/clustertests/Dockerfile-fakeIDP @@ -1,4 +1,4 @@ -FROM golang:latest +FROM golang:1.16 WORKDIR / COPY fakeidp ./ diff --git a/server/server.go b/server/server.go index 697816f16..c73cabda3 100644 --- a/server/server.go +++ b/server/server.go @@ -519,10 +519,6 @@ func (m *Command) SetupServer() error { // Tell server about its new API, which its client will need. m.Server.SetAPI(m.API) - if err != nil { - return errors.Wrap(err, "new grpc server") - } - var p authz.GroupPermissions if m.Config.Auth.Enable { m.Config.MustValidateAuth() From 574c404a66b8aa28fc8b4206e91256f6514e667d Mon Sep 17 00:00:00 2001 From: Souhaila Noor Date: Fri, 25 Feb 2022 11:05:56 -0600 Subject: [PATCH 02/34] addressed review comments --- holder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/holder.go b/holder.go index 2f24793ee..7e7b4c056 100644 --- a/holder.go +++ b/holder.go @@ -291,7 +291,7 @@ func (h *Holder) IndexesPath() string { // the _exists field is set to row+1 when delete is started. Upon completion, the row is deleted. // if _exists>=1, we finish deleting the rows func (h *Holder) processDeleteInflight() error { - for _, index := range h.indexes { + for _, index := range h.Indexes() { if index.trackExistence { shards := index.AvailableShards(includeRemote).Slice() From 6b23925bd764fe2a7552fa3d739ba2625ebd1c9b Mon Sep 17 00:00:00 2001 From: reesporte Date: Wed, 23 Feb 2022 14:29:15 -0600 Subject: [PATCH 03/34] improve Server WaitGroup concurrent usage Add a lock to the Server WaitGroup so that if the Server WaitGroup is already waiting, we won't concurrently add to it and cause a data race. Also, when adding to the Server WaitGroup, check that the server is not closing already, since that means we really shouldn't be doing more work. --- api.go | 14 +++++++---- server.go | 53 ++++++++++++++++++++++++++++++++++------- server_internal_test.go | 31 ++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 13 deletions(-) diff --git a/api.go b/api.go index 45892dce1..783b22e0e 100644 --- a/api.go +++ b/api.go @@ -1049,9 +1049,17 @@ func (api *API) requestUsageOfNodes() { // Calculates disk usage from scratch if cache has expired for each index and stores the results in the usage cache func (api *API) calculateUsage() { + // don't need to calculateUsage if we're about to close! + if api.isClosing() { + return + } + api.usageCache.muCalculate.Lock() defer api.usageCache.muCalculate.Unlock() - api.server.wg.Add(1) + if ok := api.server.addToWaitGroup(1); !ok { + // the server is closing, so just stop! + return + } defer api.server.wg.Done() api.usageCache.muAssign.Lock() @@ -1065,10 +1073,6 @@ func (api *API) calculateUsage() { if err != nil { api.server.logger.Infof("couldn't get index usage details: %s", err) } - if api.isClosing() { - return - } - totalSize := nodeMetadataBytes for _, s := range indexDetails { totalSize += s.Total diff --git a/server.go b/server.go index 531e56737..b58b1b714 100644 --- a/server.go +++ b/server.go @@ -44,6 +44,7 @@ var _ broadcaster = &Server{} type Server struct { // nolint: maligned // Close management. wg sync.WaitGroup + muWG sync.Mutex closing chan struct{} // Internal @@ -99,6 +100,26 @@ func (s *Server) Holder() *Holder { return s.holder } +// addToWaitGroup adds to the server WaitGroup but makes sure the server isn't +// closing, and that the WaitGroup is not already waiting before it adds +func (s *Server) addToWaitGroup(delta int) bool { + select { + case <-s.closing: + return false + default: + s.muWG.Lock() + defer s.muWG.Unlock() + select { + case <-s.closing: + // if we're closing after having gotten the lock, stop!! + return false + default: + s.wg.Add(delta) + return true + } + } +} + // ServerOption is a functional option type for pilosa.Server type ServerOption func(s *Server) error @@ -590,7 +611,10 @@ func (s *Server) Open() error { // Start background process listening for translation // sync resets. - s.wg.Add(1) + if ok := s.addToWaitGroup(1); !ok { + return fmt.Errorf("closing server while opening server is NOT allowed") + } + go func() { defer s.wg.Done(); s.monitorResetTranslationSync() }() go func() { _ = s.translationSyncer.Reset() }() @@ -617,7 +641,9 @@ func (s *Server) Open() error { return errors.Wrap(err, "setting nodeState") } - s.wg.Add(3) + if ok := s.addToWaitGroup(3); !ok { + return fmt.Errorf("closing server while opening server is NOT allowed") + } go func() { defer s.wg.Done(); s.monitorAntiEntropy() }() go func() { defer s.wg.Done(); s.monitorRuntime() }() go func() { defer s.wg.Done(); s.monitorDiagnostics() }() @@ -631,14 +657,18 @@ func (s *Server) Open() error { return toSend }() - s.wg.Add(1) + if ok := s.addToWaitGroup(1); !ok { + return fmt.Errorf("closing server while opening server is NOT allowed") + } go func() { defer s.wg.Done() ctx, cancel := context.WithCancel(context.Background()) defer cancel() - - s.wg.Add(1) + if ok := s.addToWaitGroup(1); !ok { + // the server is closing, stop!! + return + } go func() { defer s.wg.Done() defer cancel() @@ -716,11 +746,15 @@ func (s *Server) Close() error { case <-s.closing: return nil default: - errE := s.executor.Close() - + // get the muWG lock so that noone adds to the WaitGroup while it Waits + s.muWG.Lock() + defer s.muWG.Unlock() // Notify goroutines to stop. close(s.closing) s.wg.Wait() + + errE := s.executor.Close() + var errh, errd error var errhs error var errc error @@ -776,8 +810,11 @@ func (s *Server) monitorResetTranslationSync() { case <-s.closing: return case <-s.resetTranslationSyncCh: + if ok := s.addToWaitGroup(1); !ok { + // the server is closing!!! stop!! + return + } s.logger.Infof("holder translation sync beginning") - s.wg.Add(1) go func() { // Obtaining this lock ensures that there is only // one instance of resetTranslationSync() running diff --git a/server_internal_test.go b/server_internal_test.go index da6d57578..b2e5d1116 100644 --- a/server_internal_test.go +++ b/server_internal_test.go @@ -35,3 +35,34 @@ func TestMonitorAntiEntropyZero(t *testing.T) { t.Fatalf("monitorAntiEntropy should have returned immediately with duration 0") } } + +func TestAddToWaitGroup(t *testing.T) { + // if this test times out / panics we have a problem, otherwise we're fine + td := t.TempDir() + cfg := &storage.Config{FsyncEnabled: false, Backend: storage.DefaultBackend} + s, err := NewServer(OptServerDataDir(td), OptServerStorageConfig(cfg)) + if err != nil { + t.Fatalf("making new server: %v", err) + } + + oks := make(chan bool, 10) + for i := 0; i < 10; i++ { + go func() { + oks <- s.addToWaitGroup(1) + time.Sleep(10 * time.Millisecond) + defer s.wg.Done() + }() + } + + for i := 0; i < 10; i++ { + ok := <-oks + if !ok { + t.Fatalf("unexpected close during WaitGroup add") + } + } + + s.Close() + if ok := s.addToWaitGroup(1); ok { + t.Fatalf("shouldn't be able to add while server is closing") + } +} From 45633e23a5c5ddce39e4a9ebbbe625adcfc6bb93 Mon Sep 17 00:00:00 2001 From: reesporte Date: Fri, 25 Feb 2022 10:53:45 -0600 Subject: [PATCH 04/34] only set bits after the holder is completely setup This should help prevent a data race. SetBit can, in some cases, cause an asynchronous task to run which tries to update the stats counter. But if that task runs while we are modifying the stats counter itself, we have a data race. --- stats/stats_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/stats/stats_test.go b/stats/stats_test.go index 81b62a5f2..4515636c9 100644 --- a/stats/stats_test.go +++ b/stats/stats_test.go @@ -81,11 +81,6 @@ func TestStatsCount_TopN(t *testing.T) { defer c.Close() hldr := test.Holder{Holder: c.GetNode(0).Server.Holder()} - hldr.SetBit("d", "f", 0, 0) - hldr.SetBit("d", "f", 0, 1) - hldr.SetBit("d", "f", 0, pilosa.ShardWidth) - hldr.SetBit("d", "f", 0, pilosa.ShardWidth+2) - // Execute query. called := false hldr.Holder.Stats = &MockStats{ @@ -101,6 +96,12 @@ func TestStatsCount_TopN(t *testing.T) { called = true }, } + + hldr.SetBit("d", "f", 0, 0) + hldr.SetBit("d", "f", 0, 1) + hldr.SetBit("d", "f", 0, pilosa.ShardWidth) + hldr.SetBit("d", "f", 0, pilosa.ShardWidth+2) + if _, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "d", Query: `TopN(field=f, n=2)`}); err != nil { t.Fatal(err) } From cf1de78efd27244e16617e8281488961e29a07bd Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Fri, 25 Feb 2022 10:38:20 -0600 Subject: [PATCH 05/34] remove string keys on delete to allow for reuse --- boltdb/translate.go | 91 ++++++++++++++++- boltdb/translate_test.go | 49 ++++++++- catcher.go | 5 + dbshard_internal_test.go | 65 ++++++++++++ delete_test.go | 49 ++++++++- executor.go | 167 ++++++++++++++++++++++--------- go.mod | 1 + go.sum | 2 + holder.go | 2 +- rbf.go | 68 +++++++++++++ rbf/cursor.go | 5 +- rbf/db.go | 1 - rbf/tx.go | 5 +- roaring/filter.go | 4 + roaring/roaring.go | 51 ++++++++++ roaring/roaring_internal_test.go | 23 +++++ row.go | 9 ++ stattx.go | 12 +++ translate.go | 13 +++ tx.go | 1 + tx_test.go | 1 - 21 files changed, 566 insertions(+), 58 deletions(-) diff --git a/boltdb/translate.go b/boltdb/translate.go index 8ff56f7e4..a5a17e962 100644 --- a/boltdb/translate.go +++ b/boltdb/translate.go @@ -12,7 +12,8 @@ import ( "sync" "time" - "github.com/molecula/featurebase/v3" + pilosa "github.com/molecula/featurebase/v3" + "github.com/molecula/featurebase/v3/roaring" "github.com/pkg/errors" bolt "go.etcd.io/bbolt" @@ -32,6 +33,8 @@ var ( bucketKeys = []byte("keys") bucketIDs = []byte("ids") + bucketFree = []byte("free") + FreeKey = []byte("free") ) const ( @@ -119,6 +122,8 @@ func (s *TranslateStore) Open() (err error) { return err } else if _, err := tx.CreateBucketIfNotExists(bucketIDs); err != nil { return err + } else if _, err := tx.CreateBucketIfNotExists(bucketFree); err != nil { + return err } return nil }); err != nil { @@ -445,7 +450,7 @@ func (r *TranslateEntryReader) Close() error { return nil } -// ReadEntry reads the next entry from the underlying translate store. +// ReadEntry reads th next entry from the underlying translate store. func (r *TranslateEntryReader) ReadEntry(entry *pilosa.TranslateEntry) error { // Ensure reader has not been closed before read. select { @@ -498,6 +503,88 @@ func (r *TranslateEntryReader) ReadEntry(entry *pilosa.TranslateEntry) error { } } +type boltWrapper struct { + tx *bolt.Tx + db *bolt.DB +} + +func (w *boltWrapper) Commit() error { + if w.tx != nil { + return w.tx.Commit() + } + return nil +} + +func (w *boltWrapper) Rollback() { + if w.tx != nil { + w.tx.Rollback() + } +} +func (s *TranslateStore) FreeIDs() (*roaring.Bitmap, error) { + result := roaring.NewBitmap() + err := s.db.View(func(tx *bolt.Tx) error { + bkt := tx.Bucket(bucketFree) + if bkt == nil { + return errors.Errorf(errFmtTranslateBucketNotFound, bucketKeys) + } + b := bkt.Get(FreeKey) + err := result.UnmarshalBinary(b) + if err != nil { + return err + } + return nil + }) + return result, err +} +func (s *TranslateStore) MergeFree(tx *bolt.Tx, newIDs *roaring.Bitmap) error { + bkt := tx.Bucket(bucketFree) + b := bkt.Get(FreeKey) + buf := new(bytes.Buffer) + if b != nil { //if existing combine with newIDs + before := roaring.NewBitmap() + err := before.UnmarshalBinary(b) + if err != nil { + return err + } + final := newIDs.Union(before) + _, err = final.WriteTo(buf) + if err != nil { + return err + } + } else { + newIDs.WriteTo(buf) + } + return bkt.Put(FreeKey, buf.Bytes()) +} + +// Delete removes the lookeup pairs in order to make avialble for reuse but doesn't commit the +// transaction for that is tied to the associated rbf transaction being successful +func (s *TranslateStore) Delete(records *roaring.Bitmap) (pilosa.Commitor, error) { + tx, err := s.db.Begin(true) + if err != nil { + return nil, err + } + keyBucket := tx.Bucket(bucketKeys) + idBucket := tx.Bucket(bucketIDs) + ids := records.Slice() + for i := range ids { + id := u64tob(ids[i]) + boltKey := idBucket.Get(id) + err = keyBucket.Delete(boltKey) + if err != nil { + tx.Rollback() + return &boltWrapper{}, err + } + err = idBucket.Delete(id) + if err != nil { + tx.Rollback() + return &boltWrapper{}, err + } + + } + return &boltWrapper{tx: tx}, s.MergeFree(tx, records) +} + // emptyKey is a sentinel byte slice which stands for "" as a key. var emptyKey = []byte{ 0x00, 0x00, 0x00, diff --git a/boltdb/translate_test.go b/boltdb/translate_test.go index 201971644..ed367c8b7 100644 --- a/boltdb/translate_test.go +++ b/boltdb/translate_test.go @@ -10,8 +10,9 @@ import ( "testing" "time" - "github.com/molecula/featurebase/v3" + pilosa "github.com/molecula/featurebase/v3" "github.com/molecula/featurebase/v3/boltdb" + "github.com/molecula/featurebase/v3/roaring" "github.com/molecula/featurebase/v3/testhook" "github.com/molecula/featurebase/v3/topology" ) @@ -385,7 +386,53 @@ func MustNewTranslateStore(tb testing.TB) *boltdb.TranslateStore { s.Path = f.Name() return s } +func TestTranslateStore_Delete(t *testing.T) { + s := MustOpenNewTranslateStore(t) + defer MustCloseTranslateStore(s) + // Setup initial keys. + ids, err := s.CreateKeys("foo", "bar", "deleteme") + if err != nil { + t.Fatal(err) + } + + records := roaring.NewBitmap(ids["deleteme"]) + c, err := s.Delete(records) + if err != nil { + t.Fatal(err) + } + if err = c.Commit(); err != nil { + t.Fatal(err) + } + r, e := s.FreeIDs() + if e != nil { + t.Fatal(err) + } + freeids := r.Slice() + if len(freeids) == 0 { + t.Fatalf("expected to have free id") + } + if freeids[0] != ids["deleteme"] { + t.Fatalf("expected [%v] and got %v", ids["deleteme"], freeids[0]) + } + + records2 := roaring.NewBitmap(ids["foo"]) + c, err = s.Delete(records2) + if err != nil { + t.Fatal(err) + } + if err = c.Commit(); err != nil { + t.Fatal(err) + } + r, e = s.FreeIDs() + if e != nil { + t.Fatal(err) + } + freeids = r.Slice() + if len(freeids) != 2 { + t.Fatalf("expected to have 2 free ids") + } +} func TestTranslateStore_ReadWrite(t *testing.T) { t.Run("WriteTo_ReadFrom", func(t *testing.T) { s := MustOpenNewTranslateStore(t) diff --git a/catcher.go b/catcher.go index a1f128d65..0a75ac9f7 100644 --- a/catcher.go +++ b/catcher.go @@ -26,6 +26,11 @@ func init() { var _ Tx = (*catcherTx)(nil) +func (c *catcherTx) RemoveChannel(index, field, view string, shard uint64, a chan uint64, resChan chan countResults) { + c.b.RemoveChannel(index, field, view, shard, a, resChan) + return +} + func (c *catcherTx) NewTxIterator(index, field, view string, shard uint64) *roaring.Iterator { return c.b.NewTxIterator(index, field, view, shard) } diff --git a/dbshard_internal_test.go b/dbshard_internal_test.go index 38f752425..1c173eb9c 100644 --- a/dbshard_internal_test.go +++ b/dbshard_internal_test.go @@ -3,6 +3,7 @@ package pilosa import ( "fmt" + "math/rand" "os" "path/filepath" "strings" @@ -296,3 +297,67 @@ func Test_DBPerShard_GetFieldView2Shards_map_from_RBF(t *testing.T) { panic(fmt.Sprintf("expected '%v' but got view2shard '%v'", exp, view2shard)) } } +func TestTXBigDelete(t *testing.T) { + if _, ok := os.LookupEnv("GAUNTLET"); !ok { + t.Skip("only running this test if GAUNTLET is set") + } + + f, idx, tx := mustOpenFragment(t, "i", "f", viewStandard, 0, "") + _ = idx + defer f.Clean(t) + result := make(map[uint64]struct{}) + var set, none []uint64 + accum := uint64(0) + N := 1000000 + fmt.Println("build big set") + rand.Seed(0) + for i := 0; i < N; i++ { + bd := rand.Intn(15) + accum += uint64(bd) + for row := uint64(0); row < uint64(rand.Intn(10)); row++ { + pos, _ := f.pos(row, accum) + set = append(set, pos) + } + } + fmt.Println("set") + err := f.importPositions(tx, set, none, result) + PanicOn(err) + PanicOn(tx.Commit()) + + // Close and reopen the fragment & verify the data. + fmt.Println("repopen") + err = f.Reopen() // roaring data not being flushed? red on roaring + if err != nil { + t.Fatal(err) + } + fmt.Println("clear") + tx = idx.holder.txf.NewTx(Txo{Write: writable, Index: idx, Fragment: f, Shard: f.shard}) + row, er := f.row(tx, 5) + PanicOn(er) + fmt.Println(row.Count()) + cols := row.Columns() + subset := make([]uint64, len(cols)) + for i := range cols { + pos, e := f.pos(5, cols[i]) + PanicOn(e) + subset[i] = pos + } + PanicOn(f.importPositions(tx, none, subset, result)) + row, er = f.row(tx, 5) + PanicOn(er) + fmt.Println(row.Count()) + PanicOn(tx.Commit()) + + fmt.Println("reopen") + err = f.Reopen() // roaring data not being flushed? red on roaring + if err != nil { + t.Fatal(err) + } + tx = idx.holder.txf.NewTx(Txo{Write: !writable, Index: idx, Fragment: f, Shard: f.shard}) + row, er = f.row(tx, 5) + PanicOn(er) + fmt.Println("delete count should be 0", row.Count()) + row, er = f.row(tx, 2) + PanicOn(er) + fmt.Println(row.Count()) +} diff --git a/delete_test.go b/delete_test.go index df977513b..63c77be34 100644 --- a/delete_test.go +++ b/delete_test.go @@ -8,7 +8,8 @@ import ( "testing" "time" - "github.com/molecula/featurebase/v3" + pilosa "github.com/molecula/featurebase/v3" + "github.com/molecula/featurebase/v3/disco" "github.com/molecula/featurebase/v3/test" "github.com/stretchr/testify/require" ) @@ -49,6 +50,21 @@ func TestExecutor_DeleteRecords(t *testing.T) { }) } + setupBig := func(t *testing.T, r *require.Assertions, c *test.Cluster, Rows uint64) { + t.Helper() + fieldName := "setfield" + c.CreateField(t, indexName, pilosa.IndexOptions{TrackExistence: true}, fieldName) + rows := make([][2]uint64, ShardWidth*Rows) + for columnID := uint64(0); columnID < ShardWidth; columnID++ { + for rowID := uint64(0); rowID < Rows; rowID++ { + if rowID == 0 || (columnID%rowID+1) != 0 { + rows[rowID] = [2]uint64{rowID, columnID} + } + } + } + c.ImportBits(t, indexName, "setfield", rows) + } + setupKeys := func(t *testing.T, r *require.Assertions, c *test.Cluster) { t.Helper() c.CreateField(t, indexName, pilosa.IndexOptions{Keys: true, TrackExistence: true}, "timefield", pilosa.OptFieldKeys(), pilosa.OptFieldTypeTime(pilosa.TimeQuantum("YMDH"))) @@ -131,6 +147,12 @@ func TestExecutor_DeleteRecords(t *testing.T) { m = resp.Results[0].(pilosa.ExtractedTable) after := convertKey(m.Columns) require.Equal([]string{"B", "C", "D", "two"}, after, "these keyed records after delete") + //validate that column keys got deleted + node := c.GetNode(0) + keys := []string{"A", "one"} + res, err := node.API.FindIndexKeys(context.Background(), indexName, keys...) + require.Nil(err) + require.Empty(res) }) t.Run("Delete Row", func(t *testing.T) { setup(t, require, c) @@ -200,8 +222,33 @@ func TestExecutor_DeleteRecords(t *testing.T) { require.Equal([]uint64{0, 1}, after, "these records should be remaining") }) }) + t.Run("DeleteRecordsBigWithRestart", func(t *testing.T) { + c := test.MustNewCluster(t, 1) + for _, n := range c.Nodes { + n.Config.Cluster.ReplicaN = 1 + } + err := c.Start() + defer c.Close() + require.NoError(err, "Start cluster DeleteRecordsBig") + setupBig(t, require, c, 16) + defer tearDown(t, require, c) + node := c.GetNode(0) + resp := c.Query(t, indexName, `Delete(Row(setfield=12))`) + require.NotNil(resp, "Response should not be nil") + require.NotEmpty(resp.Results) + require.Equal(true, resp.Results[0], "Change should have happened") + resp = c.Query(t, indexName, `Count(Row(setfield=12))`) + require.NotNil(resp, "Response should not be nil") + require.NotEmpty(resp.Results) + require.Equal(uint64(0), resp.Results[0], "Should have removed") + err = node.Reopen() + require.NoError(err, "restart cluster DeleteRecordsBig") + err = c.AwaitState(disco.ClusterStateNormal, 10*time.Second) + require.NoError(err, "backToNormal") + }) } + func convert(before []pilosa.ExtractedTableColumn) []uint64 { result := make([]uint64, 0) for _, i := range before { diff --git a/executor.go b/executor.go index 7dfc994f2..d9aba15c3 100644 --- a/executor.go +++ b/executor.go @@ -8252,72 +8252,117 @@ func (e *executor) executeDeleteRecords(ctx context.Context, qcx *Qcx, index str return n, nil } -func (e *executor) executeDeleteRecordFromShard(ctx context.Context, qcx *Qcx, index string, c *pql.Call, shard uint64) (bool, error) { +func transactExistRow(ctx context.Context, idx *Index, shard uint64, frag *fragment, src *Row) (uint64, error) { + tx := idx.Txf().NewTx(Txo{Write: writable, Index: idx, Shard: shard}) + rows, err := frag.rows(ctx, tx, 1) + if err != nil { + tx.Rollback() + return 0, err + } + rowID := uint64(len(rows) + 1) + _, err = frag.setRow(tx, src, rowID) + if err != nil { + tx.Rollback() + return 0, err + } + return rowID, tx.Commit() +} +func (e *executor) executeDeleteRecordFromShard(ctx context.Context, qcx *Qcx, index string, c *pql.Call, shard uint64) (changed bool, err error) { span, _ := tracing.StartSpanFromContext(ctx, "Executor.executeDeleteRecordFromShard") defer span.Finish() //need to build the bitmap in the call child := c.Children[0] - row, err := e.executeBitmapCallShard(ctx, qcx, index, child, shard) - if err != nil { - return false, err + src, er := e.executeBitmapCallShard(ctx, qcx, index, child, shard) + if er != nil { + err = er + return } - if len(row.segments) == 0 { //nothing to remove - return false, nil + if len(src.segments) == 0 { //nothing to remove + return + } + columns := src.segments[0].data //should only be one segment + if columns.Count() == 0 { + return } - // Fetch index. idx := e.Holder.Index(index) if idx == nil { - return false, newNotFoundError(ErrIndexNotFound, index) + err = newNotFoundError(ErrIndexNotFound, index) + return } - return DeleteRows(row, idx, shard) + return DeleteRows(ctx, src, idx, shard) } -func DeleteRows(row *Row, idx *Index, shard uint64) (bool, error) { - tx := idx.holder.txf.NewTx(Txo{Write: writable, Index: idx, Shard: shard}) - defer tx.Rollback() - +func DeleteRows(ctx context.Context, row *Row, idx *Index, shard uint64) (bool, error) { + var existenceFragment *fragment + var deletedRowID uint64 + var commitor Commitor = &NopCommitor{} + var err error columns := row.segments[0].data //should only be one segment - if columns.Count() == 0 { - return false, nil - } - columnIDs := make([]uint64, 0) - none := make([]uint64, 0) // no bits will be set - - changed := false - colCounts := make([]int, 0) - toClear := columnIDs[:0] - rowSet := make(map[uint64]struct{}) - callback := func(pos uint64) error { - toClear = append(toClear, pos) - rowID := pos / ShardWidth - rowSet[rowID] = struct{}{} - return nil - } - findExisting := roaring.NewBitmapBitmapFilter(columns, callback) - - clearFragment := func(frag *fragment) (bool, error) { - // re-zero these - toClear = columnIDs[:0] - rowSet = make(map[uint64]struct{}) - - err := tx.ApplyFilter(frag.index(), frag.field(), frag.view(), frag.shard, 0, findExisting) + if idx.Keys() { + columns := row.segments[0].data + //store columns in exits field ToBeDelete row commited + existenceFragment = idx.Holder().fragment(idx.Name(), existenceFieldName, viewStandard, shard) + if existenceFragment == nil { + //no exists field + return false, errors.New("can't bulk delete without existence field") + } + deletedRowID, err = transactExistRow(ctx, idx, shard, existenceFragment, row) + commitor, err = deleteKeyTranslation(ctx, idx, shard, columns) if err != nil { return false, err } - colCounts = append(colCounts, len(toClear)) - // this will be the remove part - if len(toClear) > 0 { - err = frag.importPositions(tx, none, toClear, rowSet) - if err != nil { - return false, err - } - return true, nil - } - return false, nil } + writeTx := idx.Txf().NewTx(Txo{Write: writable, Index: idx, Shard: shard}) + defer writeTx.Rollback() + if err != nil { + return false, err + } + changed := false + defer func() { + //if there is an error on the bit clearing rollback the keys + if err != nil { + changed = false + commitor.Rollback() + return + } + // if there is an error in the key commit, then rollback the delete + // write records before keys to remove possiblity of unmatch keys=records + err = writeTx.Commit() + if err != nil { + changed = false + commitor.Rollback() + return + } + if er := commitor.Commit(); er != nil { + err = er + } + if err != nil { + idx.Holder().Logger.Errorf("problems committing delete in rbf %v shard %v", err, shard) + } + + }() + findExisting := roaring.NewBitmapBitmapFilter(columns, func(p uint64) error { return nil }) + resChan := make(chan countResults) + clearFragment := func(frag *fragment) (bool, error) { + posChan := make(chan uint64, 8192) + findExisting.SetCallback(func(pos uint64) error { + posChan <- pos + return nil + }) + go writeTx.RemoveChannel(frag.index(), frag.field(), frag.view(), frag.shard, posChan, resChan) + + err = writeTx.ApplyFilter(frag.index(), frag.field(), frag.view(), frag.shard, 0, findExisting) + close(posChan) + if err != nil { + return false, err + } + r := <-resChan + return r.changeCount > 0, r.err + } + for _, field := range idx.Fields() { for _, view := range field.views() { @@ -8332,7 +8377,33 @@ func DeleteRows(row *Row, idx *Index, shard uint64) (bool, error) { if c { changed = true } + } } - return changed, tx.Commit() + close(resChan) + if existenceFragment != nil { //a string keys have been deleted and the deleteRow was created + existenceFragment.clearRow(writeTx, deletedRowID) + } + return changed, nil +} + +type Commitor interface { + Rollback() + Commit() error +} +type NopCommitor struct { +} + +func (c *NopCommitor) Rollback() { + +} +func (c *NopCommitor) Commit() error { + return nil +} + +func deleteKeyTranslation(ctx context.Context, idx *Index, shard uint64, records *roaring.Bitmap) (Commitor, error) { + // ShardToShardParition ... + paritionID := topology.ShardToShardPartition(idx.name, shard, idx.holder.partitionN) + + return idx.TranslateStore(paritionID).Delete(records) } diff --git a/go.mod b/go.mod index 529f2a29d..f089da554 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/benbjohnson/immutable v0.3.0 github.com/buger/jsonparser v1.1.1 github.com/cespare/xxhash v1.1.0 + github.com/claygod/PiHex v0.0.0-20200916193129-5277802bfd7b // indirect github.com/davecgh/go-spew v1.1.1 github.com/desertbit/timer v0.0.0-20180107155436-c41aec40b27f // indirect github.com/dustin/go-humanize v1.0.0 // indirect diff --git a/go.sum b/go.sum index 79fc9f1f6..2e0ea4375 100644 --- a/go.sum +++ b/go.sum @@ -54,6 +54,8 @@ github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghf github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI= github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI= github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU= +github.com/claygod/PiHex v0.0.0-20200916193129-5277802bfd7b h1:LmxuKRxYbpulBnhu2ZYLfN92Zs2uitai6s6hpmCIZ1Q= +github.com/claygod/PiHex v0.0.0-20200916193129-5277802bfd7b/go.mod h1:iQyqZlmS/QK9N12+07jX1OO2xlzguGIE7vDmHh3TX+E= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa h1:OaNxuTZr7kxeODyLWsRMC+OD03aFUH+mW6r2d+MWa5Y= diff --git a/holder.go b/holder.go index 7e7b4c056..4e9030f40 100644 --- a/holder.go +++ b/holder.go @@ -324,7 +324,7 @@ func (h *Holder) processDeleteInflight() error { } inprocessRowIDs = inprocessRowIDs.Union(row) } - DeleteRows(inprocessRowIDs, index, shard) + DeleteRows(context.Background(), inprocessRowIDs, index, shard) } } } diff --git a/rbf.go b/rbf.go index b79b2eb03..5360958cb 100644 --- a/rbf.go +++ b/rbf.go @@ -223,6 +223,74 @@ func (tx *RBFTx) Remove(index, field, view string, shard uint64, a ...uint64) (c // which is expensive in practice and only really useful occasionally. const sortedParanoia = false +type countResults struct { + changeCount int + err error +} + +// RemoveChannel provides a method of streaming in bits or positions and not requiring a large buffer like add and remove +// the bits are input via the posChanel and the results are returned via the retChannel +func (tx *RBFTx) RemoveChannel(index, field, view string, shard uint64, a chan uint64, resChan chan countResults) { + name := rbfName(index, field, view, shard) + var lastHi uint64 = math.MaxUint64 // highbits is always less than this starter. + var rc *roaring.Container + var hi uint64 + var lo uint16 + var err error + changeCount := 0 + i := 0 + for v := range a { + hi, lo = highbits(v), lowbits(v) + if hi != lastHi { + // either first time through, or changed to a different container. + // do we need put the last updated container now? + if i > 0 { + // not first time through, write what we got. + if rc == nil || (rc.N() == 0) { + err = tx.tx.RemoveContainer(name, lastHi) + if err != nil { + resChan <- countResults{0, errors.Wrap(err, "failed to remove container")} + return + } + } else { + err = tx.tx.PutContainer(name, lastHi, rc) + if err != nil { + resChan <- countResults{0, errors.Wrap(err, "failed to put container")} + return + } + } + } + // get the next container + rc, err = tx.tx.Container(name, hi) + if err != nil { + resChan <- countResults{0, errors.Wrap(err, "failed to retrieve container")} + return + } + } // else same container, keep adding bits to rct. + chng := false + rc, chng = rc.Remove(lo) + if chng { + changeCount++ + } + lastHi = hi + i++ + } + // write the last updates. + if rc == nil || rc.N() == 0 { + err = tx.tx.RemoveContainer(name, hi) + if err != nil { + resChan <- countResults{0, errors.Wrap(err, "failed to remove container")} + return + } + } else { + err = tx.tx.PutContainer(name, hi, rc) + if err != nil { + resChan <- countResults{0, errors.Wrap(err, "put to remove container")} + return + } + } + resChan <- countResults{changeCount, nil} +} func (tx *RBFTx) addOrRemove(index, field, view string, shard uint64, remove bool, a ...uint64) (changeCount int, err error) { if len(a) == 0 { return 0, nil diff --git a/rbf/cursor.go b/rbf/cursor.go index 41e9e4d4f..c359d723a 100644 --- a/rbf/cursor.go +++ b/rbf/cursor.go @@ -474,9 +474,11 @@ func (c *Cursor) putLeafCell(in leafCell) (err error) { writeCellN(buf[:], len(group)) offset := dataOffset(len(group)) + X := 0 for j, cell := range group { writeLeafCell(buf[:], j, offset, cell) offset += align8(cell.Size()) + X++ } if err := c.tx.writePage(buf[:]); err != nil { @@ -614,7 +616,6 @@ func (c *Cursor) deleteLeafCell(key uint64) (err error) { copy(cells[elem.index:], cells[elem.index+1:]) cells[len(cells)-1] = leafCell{} cells = cells[:len(cells)-1] - // Write cells to page. buf := allocPage() writePageNo(buf[:], elem.pgno) @@ -626,12 +627,14 @@ func (c *Cursor) deleteLeafCell(key uint64) (err error) { writeLeafCell(buf[:], j, offset, cell) offset += align8(cell.Size()) } + if err := c.tx.writePage(buf[:]); err != nil { return err } // Update the parent's reference key if it's changed. if c.stack.top > 0 && oldPageKey != cells[0].Key { + return c.updateBranchCell(c.stack.top-1, cells[0].Key) } return nil diff --git a/rbf/db.go b/rbf/db.go index 4076fecca..ec75a6064 100644 --- a/rbf/db.go +++ b/rbf/db.go @@ -412,7 +412,6 @@ func (db *DB) checkpoint() (err error) { // Close closes the database. func (db *DB) Close() (err error) { // TODO(bbj): Add wait group to hang until last Tx is complete. - // Wait for writer lock. db.rwmu.Lock() defer db.rwmu.Unlock() diff --git a/rbf/tx.go b/rbf/tx.go index 17647caa3..f02a83783 100644 --- a/rbf/tx.go +++ b/rbf/tx.go @@ -669,6 +669,7 @@ func (tx *Tx) container(name string, key uint64) (*roaring.Container, error) { func (tx *Tx) PutContainer(name string, key uint64, ct *roaring.Container) error { tx.mu.Lock() defer tx.mu.Unlock() + return tx.putContainer(name, key, ct) } @@ -725,7 +726,6 @@ func (tx *Tx) removeContainer(name string, key uint64) error { if exact, err := c.Seek(key); err != nil || !exact { return err } - return c.deleteLeafCell(key) } @@ -1125,7 +1125,7 @@ func (tx *Tx) readPage(pgno uint32) (_ []byte, isHeap bool, err error) { // Verify page number requested is within current size of database. pageN := readMetaPageN(tx.meta[:]) - if pgno > pageN { + if pgno >= pageN { return nil, false, fmt.Errorf("rbf: page read out of bounds: pgno=%d max=%d", pgno, pageN-1) } @@ -1932,6 +1932,7 @@ func (tx *Tx) Pages(pgnos []uint32) ([]Page, error) { // PageInfos returns meta data about all pages in the database. func (tx *Tx) PageInfos() ([]PageInfo, error) { var errorList ErrorList + infos := make([]PageInfo, tx.PageN()) // Read meta page info. diff --git a/roaring/filter.go b/roaring/filter.go index 513f8e8f0..fd1856af4 100644 --- a/roaring/filter.go +++ b/roaring/filter.go @@ -584,6 +584,10 @@ type BitmapBitmapFilter struct { callback func(uint64) error } +func (b *BitmapBitmapFilter) SetCallback(cb func(uint64) error) { + b.callback = cb +} + func (b *BitmapBitmapFilter) ConsiderKey(key FilterKey, n int32) FilterResult { pos := key & keyMask if b.containers[pos] == nil || n == 0 { diff --git a/roaring/roaring.go b/roaring/roaring.go index f632aaca4..9f2ef5039 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -675,6 +675,47 @@ func (b *Bitmap) Intersect(other *Bitmap) *Bitmap { return output } +func (b *Bitmap) Hash(hash uint64) uint64 { + const ( + offset = 14695981039346656037 + prime = 1099511628211 + ) + if hash == 0 { + hash = uint64(offset) + } + + it, _ := b.Containers.Iterator(0) + for it.Next() { + ki, _ := it.Value() + hash ^= uint64(ki) + hash *= prime + } + + it, _ = b.Containers.Iterator(0) + for it.Next() { + _, ci := it.Value() + hash ^= 0 + hash *= prime + if ci.N() > 0 { + var bytes []byte + switch ci.typ() { + + case ContainerArray: + bytes = fromArray16(ci.array()) + case ContainerBitmap: + bytes = fromArray64(ci.bitmap()) + case ContainerRun: + bytes = fromInterval16(ci.runs()) + } + for _, b := range bytes { + hash ^= uint64(b) + hash *= prime + } + } + } + return hash +} + type mutableContainersIterator struct { c Containers @@ -7488,3 +7529,13 @@ func (c *Container) Slice() (r []uint16) { } return r } + +func fromArray16(a []uint16) []byte { + return (*[8192]byte)(unsafe.Pointer(&a[0]))[: len(a)*2 : len(a)*2] +} +func fromArray64(a []uint64) []byte { + return (*[8192]byte)(unsafe.Pointer(&a[0]))[:8192:8192] +} +func fromInterval16(a []Interval16) []byte { + return (*[8192]byte)(unsafe.Pointer(&a[0]))[: len(a)*4 : len(a)*4] +} diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index 7fc0e5bb1..00534cb69 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -4825,3 +4825,26 @@ func TestVariousBitmap(t *testing.T) { t.Fatal("nil AddN should be 0") } } +func TestBitmapHash(t *testing.T) { + a, b := NewContainerBitmapN(getFullBitmap(), MaxContainerVal+1), NewContainerBitmapN(getFullBitmap(), MaxContainerVal+1) + arr := NewContainerArray([]uint16{1, 2, 3, 5, 8}) + run := NewContainerRun([]Interval16{{Start: 0, Last: 32}}) + ba := NewBitmap() + bb := NewBitmap() + ba.Containers.Put(1, arr) + ba.Containers.Put(2, run) + ba.Containers.Put(101, a) + ba.Containers.Put(102, a) + + bb.Containers.Put(1, arr) + bb.Containers.Put(2, run) + bb.Containers.Put(101, b) + bb.Containers.Put(102, b) + if ba.Hash(0) != bb.Hash(0) { + t.Fatal("hash should be equal") + } + bb.Containers.Put(103, b) + if ba.Hash(0) == bb.Hash(0) { + t.Fatal("hash should be different") + } +} diff --git a/row.go b/row.go index 82c8c1029..1639f84ed 100644 --- a/row.go +++ b/row.go @@ -122,6 +122,15 @@ func (r *Row) ToTable() (*pb.TableResponse, error) { return pb.RowsToTable(r, n) } +// Hash calculate checksum code be useful in block hash join +func (r *Row) Hash() uint64 { + hash := uint64(0) + for i := range r.segments { + hash = r.segments[i].data.Hash(hash) + } + return hash +} + // ToRows implements the ToRowser interface. func (r *Row) ToRows(callback func(*pb.RowResponse) error) error { if len(r.Keys) > 0 { diff --git a/stattx.go b/stattx.go index 4780f70bc..5ce265e90 100644 --- a/stattx.go +++ b/stattx.go @@ -159,6 +159,7 @@ const ( kOffsetRange kLast // mark the end, always keep this last. The following aren't tracked atm: kType + kRemoveChannel ) func (k kall) String() string { @@ -205,6 +206,8 @@ func (k kall) String() string { return "kLast" case kType: return "kType" + case kRemoveChannel: + return "kRemoveChannel" } vprint.PanicOn(fmt.Sprintf("unknown kall '%v'", int(k))) return "" @@ -221,6 +224,15 @@ func (c *statTx) NewTxIterator(index, field, view string, shard uint64) *roaring }() return c.b.NewTxIterator(index, field, view, shard) } +func (c *statTx) RemoveChannel(index, field, view string, shard uint64, a chan uint64, resChan chan countResults) { + me := kRemoveChannel + t0 := time.Now() + defer func() { + c.stats.add(me, time.Since(t0)) + }() + c.b.RemoveChannel(index, field, view, shard, a, resChan) + return +} func (c *statTx) ImportRoaringBits(index, field, view string, shard uint64, rit roaring.RoaringIterator, clear bool, log bool, rowSize uint64) (changed int, rowSet map[uint64]int, err error) { me := kImportRoaringBits diff --git a/translate.go b/translate.go index cc36ebbe2..9d5909f28 100644 --- a/translate.go +++ b/translate.go @@ -11,6 +11,7 @@ import ( "sync" "github.com/molecula/featurebase/v3/ingest" + "github.com/molecula/featurebase/v3/roaring" "github.com/molecula/featurebase/v3/topology" "github.com/pkg/errors" ) @@ -84,6 +85,8 @@ type TranslateStore interface { // TODO: refactor this interface; readonly shoul // It should read from the reader and replace the data store with // the read payload. ReadFrom(io.Reader) (int64, error) + + Delete(records *roaring.Bitmap) (Commitor, error) } // This implements ingest's key translator interface, which differs @@ -420,6 +423,16 @@ func (s *InMemTranslateStore) SetReadOnly(v bool) { defer s.mu.Unlock() s.readOnly = v } +func (s *InMemTranslateStore) Delete(records *roaring.Bitmap) (Commitor, error) { + s.mu.Lock() + defer s.mu.Unlock() + for _, id := range records.Slice() { + key := s.keysByID[id] + delete(s.keysByID, id) + delete(s.idsByKey, key) + } + return &NopCommitor{}, nil +} // FindKeys looks up the ID for each key. // Keys are not created if they do not exist. diff --git a/tx.go b/tx.go index 65a3e3c3f..70d7a724a 100644 --- a/tx.go +++ b/tx.go @@ -133,6 +133,7 @@ type Tx interface { GetSortedFieldViewList(idx *Index, shard uint64) (fvs []txkey.FieldView, err error) GetFieldSizeBytes(index, field string) (uint64, error) + RemoveChannel(index, field, view string, shard uint64, a chan uint64, resChan chan countResults) } // GenericApplyFilter implements ApplyFilter in terms of tx.ContainerIterator, diff --git a/tx_test.go b/tx_test.go index 80d72b51c..117198c94 100644 --- a/tx_test.go +++ b/tx_test.go @@ -242,5 +242,4 @@ func TestAPI_ImportAtomicRecord(t *testing.T) { if iraBit { PanicOn("IRA bit should have been cleared") } - } From e64767a88686450bef4b4dbb3f54b1903984c3ca Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Fri, 25 Feb 2022 15:06:02 -0600 Subject: [PATCH 06/34] merge with master --- executor.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/executor.go b/executor.go index d9aba15c3..ad4fabcf8 100644 --- a/executor.go +++ b/executor.go @@ -8295,31 +8295,31 @@ func (e *executor) executeDeleteRecordFromShard(ctx context.Context, qcx *Qcx, i return DeleteRows(ctx, src, idx, shard) } -func DeleteRows(ctx context.Context, row *Row, idx *Index, shard uint64) (bool, error) { +func DeleteRows(ctx context.Context, src *Row, idx *Index, shard uint64) (bool, error) { var existenceFragment *fragment var deletedRowID uint64 var commitor Commitor = &NopCommitor{} var err error - columns := row.segments[0].data //should only be one segment + columns := src.segments[0].data //should only be one segment + if idx.Keys() { - columns := row.segments[0].data //store columns in exits field ToBeDelete row commited existenceFragment = idx.Holder().fragment(idx.Name(), existenceFieldName, viewStandard, shard) if existenceFragment == nil { //no exists field return false, errors.New("can't bulk delete without existence field") } - deletedRowID, err = transactExistRow(ctx, idx, shard, existenceFragment, row) + deletedRowID, err = transactExistRow(ctx, idx, shard, existenceFragment, src) commitor, err = deleteKeyTranslation(ctx, idx, shard, columns) if err != nil { return false, err } } writeTx := idx.Txf().NewTx(Txo{Write: writable, Index: idx, Shard: shard}) - defer writeTx.Rollback() if err != nil { return false, err } + defer writeTx.Rollback() changed := false defer func() { //if there is an error on the bit clearing rollback the keys From ecaaddcf711e119213177162c85ac6bb7f425c75 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Fri, 25 Feb 2022 15:47:30 -0600 Subject: [PATCH 07/34] . --- executor.go | 6 ++++++ roaring/container_stash.go | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/executor.go b/executor.go index ad4fabcf8..9c4ab5c1d 100644 --- a/executor.go +++ b/executor.go @@ -8300,7 +8300,13 @@ func DeleteRows(ctx context.Context, src *Row, idx *Index, shard uint64) (bool, var deletedRowID uint64 var commitor Commitor = &NopCommitor{} var err error + if len(src.segments) == 0 { //nothing to remove + return false, nil + } columns := src.segments[0].data //should only be one segment + if columns.Count() == 0 { + return false, nil + } if idx.Keys() { //store columns in exits field ToBeDelete row commited diff --git a/roaring/container_stash.go b/roaring/container_stash.go index e7e0f7cd3..fcff20daf 100644 --- a/roaring/container_stash.go +++ b/roaring/container_stash.go @@ -618,7 +618,7 @@ func (c *Container) setBitmap(bitmap []uint64) { } } if len(bitmap) != 1024 { - panic("illegal bitmap length") + panic(fmt.Sprintf("illegal bitmap length %v", len(bitmap))) } c.pointer, c.len, c.cap = (*uint16)(unsafe.Pointer(&bitmap[0])), bitmapN, bitmapN c.flags &^= flagPristine From d85ac1dca795d2aa123a6429d3724ae9cc990cc9 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Fri, 25 Feb 2022 16:18:14 -0600 Subject: [PATCH 08/34] missed a test --- executor_internal_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/executor_internal_test.go b/executor_internal_test.go index 628b63155..0fa10f44e 100644 --- a/executor_internal_test.go +++ b/executor_internal_test.go @@ -585,12 +585,13 @@ func TestExecutor_DeleteRows(t *testing.T) { t.Fatalf("failed to read row: %v", err) } - changed, err := DeleteRows(row, idx, shard) + ctx := context.Background() + changed, err := DeleteRows(ctx, row, idx, shard) if !changed || err != nil { t.Fatalf("failed to delete row: %v", err) } - changed, err = DeleteRows(row, idx, shard) + changed, err = DeleteRows(ctx, row, idx, shard) if changed { t.Fatalf("expected delete to not clear bit but it did") } From 0ed85d6d69c40cd81bcec0a4379b85f19ca4ba94 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Fri, 25 Feb 2022 16:40:04 -0600 Subject: [PATCH 09/34] comment cleanup and removed long test --- boltdb/translate.go | 2 +- dbshard_internal_test.go | 65 ---------------------------------------- 2 files changed, 1 insertion(+), 66 deletions(-) diff --git a/boltdb/translate.go b/boltdb/translate.go index a5a17e962..1c6f39a76 100644 --- a/boltdb/translate.go +++ b/boltdb/translate.go @@ -450,7 +450,7 @@ func (r *TranslateEntryReader) Close() error { return nil } -// ReadEntry reads th next entry from the underlying translate store. +// ReadEntry reads the next entry from the underlying translate store. func (r *TranslateEntryReader) ReadEntry(entry *pilosa.TranslateEntry) error { // Ensure reader has not been closed before read. select { diff --git a/dbshard_internal_test.go b/dbshard_internal_test.go index 1c173eb9c..38f752425 100644 --- a/dbshard_internal_test.go +++ b/dbshard_internal_test.go @@ -3,7 +3,6 @@ package pilosa import ( "fmt" - "math/rand" "os" "path/filepath" "strings" @@ -297,67 +296,3 @@ func Test_DBPerShard_GetFieldView2Shards_map_from_RBF(t *testing.T) { panic(fmt.Sprintf("expected '%v' but got view2shard '%v'", exp, view2shard)) } } -func TestTXBigDelete(t *testing.T) { - if _, ok := os.LookupEnv("GAUNTLET"); !ok { - t.Skip("only running this test if GAUNTLET is set") - } - - f, idx, tx := mustOpenFragment(t, "i", "f", viewStandard, 0, "") - _ = idx - defer f.Clean(t) - result := make(map[uint64]struct{}) - var set, none []uint64 - accum := uint64(0) - N := 1000000 - fmt.Println("build big set") - rand.Seed(0) - for i := 0; i < N; i++ { - bd := rand.Intn(15) - accum += uint64(bd) - for row := uint64(0); row < uint64(rand.Intn(10)); row++ { - pos, _ := f.pos(row, accum) - set = append(set, pos) - } - } - fmt.Println("set") - err := f.importPositions(tx, set, none, result) - PanicOn(err) - PanicOn(tx.Commit()) - - // Close and reopen the fragment & verify the data. - fmt.Println("repopen") - err = f.Reopen() // roaring data not being flushed? red on roaring - if err != nil { - t.Fatal(err) - } - fmt.Println("clear") - tx = idx.holder.txf.NewTx(Txo{Write: writable, Index: idx, Fragment: f, Shard: f.shard}) - row, er := f.row(tx, 5) - PanicOn(er) - fmt.Println(row.Count()) - cols := row.Columns() - subset := make([]uint64, len(cols)) - for i := range cols { - pos, e := f.pos(5, cols[i]) - PanicOn(e) - subset[i] = pos - } - PanicOn(f.importPositions(tx, none, subset, result)) - row, er = f.row(tx, 5) - PanicOn(er) - fmt.Println(row.Count()) - PanicOn(tx.Commit()) - - fmt.Println("reopen") - err = f.Reopen() // roaring data not being flushed? red on roaring - if err != nil { - t.Fatal(err) - } - tx = idx.holder.txf.NewTx(Txo{Write: !writable, Index: idx, Fragment: f, Shard: f.shard}) - row, er = f.row(tx, 5) - PanicOn(er) - fmt.Println("delete count should be 0", row.Count()) - row, er = f.row(tx, 2) - PanicOn(er) - fmt.Println(row.Count()) -} From 05e98ee6787ec8ecfd0d7617ccfcd782c55e5c4e Mon Sep 17 00:00:00 2001 From: Samir Patel <48686912+54mir@users.noreply.github.com> Date: Thu, 24 Feb 2022 11:54:58 -0600 Subject: [PATCH 10/34] Check "like" argument applied to keyed fields Check if queries that have a 'like' argument are applied to keyed fields. If not, log that the user is trying to use 'like' on an unsupported field type (as opposed to reporting that there are no results.) --- executor.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/executor.go b/executor.go index 7dfc994f2..e1d38b2ff 100644 --- a/executor.go +++ b/executor.go @@ -6731,6 +6731,17 @@ func (e *executor) translateCall(c *pql.Call, index string, columnKeys map[strin } } } + + // Check if "like" argument is applied to keyed fields. + if _, found := c.Args["like"].(string); found { + fieldName, err := c.FirstStringArg("_field", "field") + if err != nil || fieldName == "" { + return nil, fmt.Errorf("cannot read field name for Rows call") + } + if !idx.Field(fieldName).options.Keys { + return nil, fmt.Errorf("'%s' is not a set/mutex/time field with a string key", fieldName) + } + } } // Translate child calls. From 97ba0c0e4d26573393c37b5de36d7778559329c6 Mon Sep 17 00:00:00 2001 From: Samir Patel <48686912+54mir@users.noreply.github.com> Date: Fri, 25 Feb 2022 09:37:31 -0600 Subject: [PATCH 11/34] add test cases for Rows call w/ "like" --- executor_test.go | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/executor_test.go b/executor_test.go index 303b8f563..f580f76a6 100644 --- a/executor_test.go +++ b/executor_test.go @@ -5452,6 +5452,11 @@ func TestExecutor_Execute_Rows_Keys(t *testing.T) { t.Fatalf("creating field: %v", err) } + _, err = c.GetNode(0).API.CreateField(context.Background(), "i", "f_id") + if err != nil { + t.Fatalf("creating field: %v", err) + } + // setup some data. 10 bits in each of shards 0 through 9. starting at // row/col shardNum and progressing to row/col shardNum+10. Also set the // previous 2 for each bit if row >0. @@ -5474,8 +5479,9 @@ func TestExecutor_Execute_Rows_Keys(t *testing.T) { } tests := []struct { - q string - exp []string + q string + exp []string + expErr string }{ { q: `Rows(f)`, @@ -5557,13 +5563,26 @@ func TestExecutor_Execute_Rows_Keys(t *testing.T) { q: `Rows(f, like="__")`, exp: []string{"10", "11", "12", "13", "14", "15", "16", "17", "18"}, }, + { + q: `Rows(f_id, like=7)`, + expErr: "parsing:", + }, + { + q: `Rows(f_id, like="__")`, + expErr: "executing: translating call:", + }, } for i, test := range tests { t.Run(fmt.Sprintf("#%d_%s", i, test.q), func(t *testing.T) { if res, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: test.q}); err != nil { - t.Fatal(err) + if !strings.HasPrefix(err.Error(), test.expErr) { + t.Fatal(err) + } } else { + if test.expErr != "" { + t.Fatalf("got success, expected error similar to: %+v", test.expErr) + } rows := res.Results[0].(pilosa.RowIdentifiers) if !reflect.DeepEqual(rows.Keys, test.exp) { t.Fatalf("\ngot: %+v\nexp: %+v", rows.Keys, test.exp) From 376af2c25f2839b7a194848b488537f1efbd6303 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Mon, 28 Feb 2022 08:12:03 -0600 Subject: [PATCH 12/34] adust logic to include normalFlow vs recovery after merge --- executor.go | 30 +++++++++++++++++++++++------- rbf/cursor.go | 5 ++--- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/executor.go b/executor.go index 9c4ab5c1d..2b77b727c 100644 --- a/executor.go +++ b/executor.go @@ -8292,10 +8292,13 @@ func (e *executor) executeDeleteRecordFromShard(ctx context.Context, qcx *Qcx, i return } - return DeleteRows(ctx, src, idx, shard) + return DeleteRowsWithFlow(ctx, src, idx, shard, true) } func DeleteRows(ctx context.Context, src *Row, idx *Index, shard uint64) (bool, error) { + return DeleteRowsWithFlow(ctx, src, idx, shard, false) +} +func DeleteRowsWithFlow(ctx context.Context, src *Row, idx *Index, shard uint64, normalFlow bool) (bool, error) { var existenceFragment *fragment var deletedRowID uint64 var commitor Commitor = &NopCommitor{} @@ -8310,12 +8313,14 @@ func DeleteRows(ctx context.Context, src *Row, idx *Index, shard uint64) (bool, if idx.Keys() { //store columns in exits field ToBeDelete row commited - existenceFragment = idx.Holder().fragment(idx.Name(), existenceFieldName, viewStandard, shard) - if existenceFragment == nil { - //no exists field - return false, errors.New("can't bulk delete without existence field") + if normalFlow { // normalFlow is the standard path, "not normal" is recoverory + existenceFragment = idx.Holder().fragment(idx.Name(), existenceFieldName, viewStandard, shard) + if existenceFragment == nil { + //no exists field + return false, errors.New("can't bulk delete without existence field") + } + deletedRowID, err = transactExistRow(ctx, idx, shard, existenceFragment, src) } - deletedRowID, err = transactExistRow(ctx, idx, shard, existenceFragment, src) commitor, err = deleteKeyTranslation(ctx, idx, shard, columns) if err != nil { return false, err @@ -8388,7 +8393,18 @@ func DeleteRows(ctx context.Context, src *Row, idx *Index, shard uint64) (bool, } close(resChan) if existenceFragment != nil { //a string keys have been deleted and the deleteRow was created - existenceFragment.clearRow(writeTx, deletedRowID) + if normalFlow { + existenceFragment.clearRow(writeTx, deletedRowID) + } else { + // this is if we are recovering from failure and cleaning up + rows, err := existenceFragment.rows(ctx, writeTx, 1) + if err != nil { + return false, err + } + for _, rowId := range rows { + existenceFragment.clearRow(writeTx, rowId) + } + } } return changed, nil } diff --git a/rbf/cursor.go b/rbf/cursor.go index c359d723a..b0fb50df5 100644 --- a/rbf/cursor.go +++ b/rbf/cursor.go @@ -474,11 +474,11 @@ func (c *Cursor) putLeafCell(in leafCell) (err error) { writeCellN(buf[:], len(group)) offset := dataOffset(len(group)) - X := 0 + x := 0 for j, cell := range group { writeLeafCell(buf[:], j, offset, cell) offset += align8(cell.Size()) - X++ + x++ } if err := c.tx.writePage(buf[:]); err != nil { @@ -634,7 +634,6 @@ func (c *Cursor) deleteLeafCell(key uint64) (err error) { // Update the parent's reference key if it's changed. if c.stack.top > 0 && oldPageKey != cells[0].Key { - return c.updateBranchCell(c.stack.top-1, cells[0].Key) } return nil From 6fa4e1242c20ce8faa835fc0ff26d27493163d88 Mon Sep 17 00:00:00 2001 From: pokeeffe-molecula Date: Mon, 28 Feb 2022 09:43:19 -0600 Subject: [PATCH 13/34] only run able perf on master --- .gitlab/.gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab/.gitlab-ci.yml b/.gitlab/.gitlab-ci.yml index 866bd14f5..d2c78d989 100644 --- a/.gitlab/.gitlab-ci.yml +++ b/.gitlab/.gitlab-ci.yml @@ -483,7 +483,7 @@ s3 dump: perf_able: stage: performance rules: - - if: '$CI_PIPELINE_SOURCE == "push"' + - if: '$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH && $CI_PIPELINE_SOURCE == "push"' trigger: include: .gitlab/.perf-able-gitlab-ci.yml variables: From 5901bcd5d6abb8aea6b82f92fdcd3ce591f190b6 Mon Sep 17 00:00:00 2001 From: Seebs Date: Fri, 18 Feb 2022 11:12:57 -0600 Subject: [PATCH 14/34] drop unused helper functions I have no idea what these functions were for, but we aren't using them so let's not have them. --- rbf/rbf_test.go | 73 ------------------------------------------------- 1 file changed, 73 deletions(-) diff --git a/rbf/rbf_test.go b/rbf/rbf_test.go index 3ff80350b..0a2432128 100644 --- a/rbf/rbf_test.go +++ b/rbf/rbf_test.go @@ -164,79 +164,6 @@ func GenerateValues(rand *rand.Rand, n int) []uint64 { return a } -var _ = ToRows - -// ToRows returns a sorted list of rows from a set of values. -func ToRows(values []uint64) []*Row { - m := make(map[uint64][]uint64) - for _, v := range values { - id := v / rbf.ShardWidth - m[id] = append(m[id], v&rbf.RowValueMask) - } - - a := make([]*Row, 0, len(m)) - for id, values := range m { - a = append(a, &Row{ID: id, Values: values}) - } - sort.Slice(a, func(i, j int) bool { return a[i].ID < a[j].ID }) - return a -} - -var _ = Row{} - -type Row struct { - ID uint64 - Values []uint64 -} - -func (r *Row) Bitmap() []uint64 { - a := make([]uint64, rbf.ShardWidth/64) - for _, v := range r.Values { - a[v/64] |= 1 << (v % 64) - } - return a -} - -// Union returns the union of r and other's values. -func (r *Row) Union(other *Row) []uint64 { - m := make(map[uint64]struct{}) - for _, v := range r.Values { - m[v] = struct{}{} - } - for _, v := range other.Values { - m[v] = struct{}{} - } - - a := make([]uint64, 0, len(m)) - for v := range m { - a = append(a, v) - } - sort.Slice(a, func(i, j int) bool { return a[i] < a[j] }) - return a -} - -// Intersect returns the intersection of r & other's values. -func (r *Row) Intersect(other *Row) []uint64 { - m := make(map[uint64]struct{}) - for _, v := range r.Values { - m[v] = struct{}{} - } - - a := make([]uint64, 0) - used := make(map[uint64]struct{}) - for _, v := range other.Values { - if _, ok := used[v]; ok { - continue - } - if _, ok := m[v]; ok { - used[v] = struct{}{} - a = append(a, v) - } - } - sort.Slice(a, func(i, j int) bool { return a[i] < a[j] }) - return a -} - // QuickCheck executes fn multiple times with a different PRNG. func QuickCheck(t *testing.T, fn func(t *testing.T, rand *rand.Rand)) { for i := 0; i < *quickCheckN; i++ { From 3ced08127192f81002c49350684f97b36efc6695 Mon Sep 17 00:00:00 2001 From: Seebs Date: Fri, 18 Feb 2022 11:32:31 -0600 Subject: [PATCH 15/34] prevent crashes when closing db When closing, we need to wait for existing Tx to exit before truncating files and unmapping things. This shouldn't matter, because we don't actually close the DB until all transactions are done, normally... except for the background usage-gathering task. But really, it's probably just better to be conservative. The actual logic is fancier than it looks. We can't hold db.mu.Lock during this, or the existing Tx can't exit. So we first grab the lock, set the closed flag, set up a waiter for all current Tx to exit, and then release the lock. Now we wait on the current Tx exiting. Once that's done, we grab the locks. Anything coming in that tries to start a Tx will fail out fairly quickly because the opened flag is now false, so even if other things get those locks before we do, they won't keep them or create new Tx. This makes one test deadlock because it opens a Tx and never closes it, so we change that test to close its Tx. --- fragment_internal_test.go | 1 + rbf/db.go | 18 +++++++-- rbf/db_test.go | 84 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 3 deletions(-) diff --git a/fragment_internal_test.go b/fragment_internal_test.go index 6fd803954..3beea99ec 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -1570,6 +1570,7 @@ func TestFragment_WriteTo_ReadFrom(t *testing.T) { } // make a read-only Tx after ReadFrom has committed. tx = idx.holder.txf.NewTx(Txo{Write: !writable, Index: idx, Fragment: f1, Shard: f1.shard}) + defer tx.Rollback() // Verify cache is in other fragment. if n := f1.cache.Len(); n != 1 { diff --git a/rbf/db.go b/rbf/db.go index ec75a6064..bd92f2271 100644 --- a/rbf/db.go +++ b/rbf/db.go @@ -411,16 +411,28 @@ func (db *DB) checkpoint() (err error) { // Close closes the database. func (db *DB) Close() (err error) { - // TODO(bbj): Add wait group to hang until last Tx is complete. + // mark db as closed, spawn a thing to wait for existing tx to drain, then + // release the lock so they CAN drain. We do this before getting the + // write lock, so if something else is waiting on rwmu.Lock, and will be + // competing with us, we can ensure that it'll exit out quickly. + db.mu.Lock() + db.opened = false + // wait for transactions to complete + ch := make(chan struct{}) + db.afterCurrentTx(func() { + close(ch) + }) + db.mu.Unlock() + <-ch + // Wait for writer lock. db.rwmu.Lock() defer db.rwmu.Unlock() + // and main DB lock. db.mu.Lock() defer db.mu.Unlock() - db.opened = false - // Close mmap handle. if db.data != nil { if e := syswrap.Munmap(db.data); e != nil && err == nil { diff --git a/rbf/db_test.go b/rbf/db_test.go index 3e6bdd1b1..2b886677c 100644 --- a/rbf/db_test.go +++ b/rbf/db_test.go @@ -139,6 +139,90 @@ func TestDB_WAL(t *testing.T) { t.Fatal(err) } }) + + // initially this is just a cut and paste of the Halt test, except that + // we close the DB while the reads are still running. + t.Run("Close", func(t *testing.T) { + if testing.Short() { + t.Skip("-short enabled, skipping") + } + + config := rbfcfg.NewDefaultConfig() + config.MaxWALSize = 16 * rbf.PageSize + config.MaxWALCheckpointSize = 8 * rbf.PageSize + config.MinWALCheckpointSize = 4 * rbf.PageSize + + db := MustOpenDB(t, config) + + // Continuously run read overlapping transactions. + ctx, cancel := context.WithCancel(context.Background()) + g, ctx := errgroup.WithContext(ctx) + for i := 0; i < 10; i++ { + i := i + g.Go(func() error { + time.Sleep(time.Duration(i) * 10 * time.Millisecond) // stagger + for { + if err := ctx.Err(); err != nil { + return nil + } + + if err := func() error { + tx, err := db.Begin(false) + if err != nil { + return err + } + // give the db time to close between when we opened and + // when we run the Container call + time.Sleep(10 * time.Millisecond) + _, err = tx.Container("x", 0) + if err != nil { + t.Fatalf("requesting container: %v", err) + } + defer tx.Rollback() + return nil + }(); err != nil { + // it's okay to ErrClosed, because we plan to close + // the database out from under us. + if err != rbf.ErrClosed { + return err + } else { + return nil + } + } + } + }) + } + + // Generate updates to the DB/WAL. + for i := 0; i < 100; i++ { + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + + if err := tx.CreateBitmapIfNotExists("x"); err != nil { + t.Fatal(err) + } else if _, err := tx.Add("x", uint64(i)); err != nil { + t.Fatal(err) + } else if err := tx.Commit(); err != nil { + t.Fatal(err) + } + time.Sleep(1 * time.Millisecond) + }() + } + // close the db now. + err := db.Close() + if err != nil { + t.Fatalf("closing db: %v", err) + } + // delay a bit to let some readers try to read + time.Sleep(20 * time.Millisecond) + + // Stop read transactions & wait. + cancel() + if err := g.Wait(); err != nil { + t.Fatal(err) + } + }) } func TestDB_Recovery(t *testing.T) { From 7dd7557e21dcd48a45cc9f54c0254c8671dff6f1 Mon Sep 17 00:00:00 2001 From: Seebs Date: Fri, 18 Feb 2022 11:44:19 -0600 Subject: [PATCH 16/34] don't dump stuff to stdout for tests We have some tests that cover stuff like the DumpDot functionality, but we don't need them to actually write to stdout during ordinary testing. Dump to buffers which we politely ignore. Yes, we could have used a dummy writer, but this way it's super easy to display the contents if we find ourselves suddenly caring. --- rbf/cursor_test.go | 5 ++++- rbf/tx_test.go | 51 +++++++++++++++++++++++++--------------------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/rbf/cursor_test.go b/rbf/cursor_test.go index 9c5603a3a..7de5f7d77 100644 --- a/rbf/cursor_test.go +++ b/rbf/cursor_test.go @@ -2,6 +2,7 @@ package rbf_test import ( + "bytes" "io" "math/bits" "math/rand" @@ -852,8 +853,10 @@ func TestDumpDot(t *testing.T) { if err != nil { t.Fatal(err) } - rbf.Dumpdot(tx, 0, " ", os.Stdout) + var b bytes.Buffer + rbf.Dumpdot(tx, 0, " ", &b) } + func TestCursor_UpdateBranchCells(t *testing.T) { db := MustOpenDB(t) defer MustCloseDB(t, db) diff --git a/rbf/tx_test.go b/rbf/tx_test.go index e9e493afc..a02361315 100644 --- a/rbf/tx_test.go +++ b/rbf/tx_test.go @@ -2,6 +2,7 @@ package rbf_test import ( + "bytes" "encoding/binary" "fmt" "math/rand" @@ -742,7 +743,11 @@ func TestTx_DeleteBitmapsWithPrefix(t *testing.T) { t.Fatal(err) } } - checkInfos := func() { + var b bytes.Buffer + pBuf := func(msg string, args ...interface{}) (int, error) { + return fmt.Fprintf(&b, msg, args...) + } + checkInfos := func(pf func(string, ...interface{}) (int, error)) { tx := MustBegin(t, db, false) defer tx.Rollback() infos, err := tx.PageInfos() @@ -750,34 +755,34 @@ func TestTx_DeleteBitmapsWithPrefix(t *testing.T) { for pgno, info := range infos { switch info := info.(type) { case *rbf.MetaPageInfo: - fmt.Printf("%-8d ", pgno) - fmt.Printf("%-10s ", "meta") - fmt.Printf("pageN=%d,walid=%d,rootrec=%d,freelist=%d\n", info.PageN, info.WALID, info.RootRecordPageNo, info.FreelistPageNo) + pf("%-8d ", pgno) + pf("%-10s ", "meta") + pf("pageN=%d,walid=%d,rootrec=%d,freelist=%d\n", info.PageN, info.WALID, info.RootRecordPageNo, info.FreelistPageNo) case *rbf.RootRecordPageInfo: - fmt.Printf("%-8d ", pgno) - fmt.Printf("%-10s ", "rootrec") - fmt.Printf("next=%d\n", info.Next) + pf("%-8d ", pgno) + pf("%-10s ", "rootrec") + pf("next=%d\n", info.Next) case *rbf.LeafPageInfo: - fmt.Printf("%-8d ", pgno) - fmt.Printf("%-10s ", "leaf") - fmt.Printf("flags=x%x,celln=%d\n", info.Flags, info.CellN) + pf("%-8d ", pgno) + pf("%-10s ", "leaf") + pf("flags=x%x,celln=%d\n", info.Flags, info.CellN) case *rbf.BranchPageInfo: - fmt.Printf("%-8d ", pgno) - fmt.Printf("%-10s ", "branch") - fmt.Printf("flags=x%x,celln=%d\n", info.Flags, info.CellN) + pf("%-8d ", pgno) + pf("%-10s ", "branch") + pf("flags=x%x,celln=%d\n", info.Flags, info.CellN) case *rbf.BitmapPageInfo: - fmt.Printf("%-8d ", pgno) - fmt.Printf("%-10s ", "bitmap") - fmt.Printf("-\n") + pf("%-8d ", pgno) + pf("%-10s ", "bitmap") + pf("-\n") case *rbf.FreePageInfo: - fmt.Printf("%-8d ", pgno) - fmt.Printf("%-10s ", "free") - fmt.Printf("-\n") + pf("%-8d ", pgno) + pf("%-10s ", "free") + pf("-\n") default: t.Fatal(fmt.Sprintf("unexpected page info type %T", info)) @@ -806,19 +811,19 @@ func TestTx_DeleteBitmapsWithPrefix(t *testing.T) { ifError(tx.Commit()) } - checkInfos() + checkInfos(pBuf) populate() - checkInfos() + checkInfos(pBuf) ifError(db.Check()) tx := MustBegin(t, db, true) tx.DeleteBitmapsWithPrefix(prefix) ifError(tx.Commit()) ifError(db.Check()) - checkInfos() + checkInfos(pBuf) populate() ifError(db.Check()) - checkInfos() + checkInfos(pBuf) } From 248dc4fe85a28778fa860f5102ce7a7db371a09f Mon Sep 17 00:00:00 2001 From: reesporte Date: Fri, 25 Feb 2022 14:59:25 -0600 Subject: [PATCH 17/34] rip out ui/usage addresses concerns in [fb-1127](https://molecula.atlassian.net/browse/FB-1127) TLDR; /ui/usage was a hotbed for issues and SEs have been turning it off anyway for ages --- api.go | 291 ++---------------- ctl/server.go | 3 - http_handler.go | 58 ---- install/featurebase.conf | 22 -- .../clustertests/testdata/featurebase.conf | 22 -- internal_client.go | 32 -- .../App/Home/ClusterHealth/ClusterHealth.tsx | 16 +- .../src/App/Home/ClusterHealth/Node/Node.tsx | 162 +--------- .../MoleculaTable/MoleculaTable.tsx | 114 +------ .../src/App/MoleculaTables/MoleculaTables.tsx | 37 +-- .../MoleculaTablesContainer.tsx | 56 +--- .../UsageBreakdown/UsageBreakdown.module.scss | 62 ---- .../UsageBreakdown/UsageBreakdown.tsx | 183 ----------- .../MoleculaTables/UsageBreakdown/index.ts | 1 - lattice/src/services/eventServices.tsx | 3 - server/config.go | 6 - server/handler_test.go | 42 --- server/server.go | 2 - txfactory.go | 187 ----------- 19 files changed, 29 insertions(+), 1270 deletions(-) delete mode 100644 lattice/src/App/MoleculaTables/UsageBreakdown/UsageBreakdown.module.scss delete mode 100644 lattice/src/App/MoleculaTables/UsageBreakdown/UsageBreakdown.tsx delete mode 100644 lattice/src/App/MoleculaTables/UsageBreakdown/index.ts diff --git a/api.go b/api.go index 783b22e0e..9e3aa0753 100644 --- a/api.go +++ b/api.go @@ -50,7 +50,6 @@ type API struct { importWorkerPoolSize int importWork chan importJob - usageCache *usageCache schemaDetailsOn bool Serializer Serializer @@ -938,260 +937,6 @@ func (api *API) PrimaryNode() *topology.Node { return snap.PrimaryFieldTranslationNode() } -// Cache of disk usage statistics -type usageCache struct { - data map[string]NodeUsage - refreshInterval time.Duration - lastUpdated time.Time - resetTrigger chan bool - lastCalcDuration time.Duration - waitMultiplier float64 - disable bool - - muCalculate sync.Mutex - muAssign sync.Mutex -} - -var usageCacheMinDuration = 5 * time.Second // If usage takes less than this duration to calculate, don't use the cache. -var usageCacheMinInterval = time.Hour // Refresh interval is forced to be >= this duration. -var usageCacheInitialInterval = time.Hour // Refresh interval starts with this duration. - -// NodeUsage represents all usage measurements for one node. -type NodeUsage struct { - Disk DiskUsage `json:"diskUsage"` - Memory MemoryUsage `json:"memoryUsage"` - LastUpdated time.Time `json:"lastUpdated"` -} - -// DiskUsage represents the storage space used on disk by one node. -type DiskUsage struct { - Capacity uint64 `json:"capacity,omitempty"` - TotalUse uint64 `json:"totalInUse"` - IndexUsage map[string]IndexUsage `json:"indexes"` -} - -// IndexUsage represents the storage space used on disk by one index, on one node. -type IndexUsage struct { - Total uint64 `json:"total"` - IndexKeys uint64 `json:"indexKeys"` - FieldKeysTotal uint64 `json:"fieldKeysTotal"` - Fragments uint64 `json:"fragments"` - Metadata uint64 `json:"metadata"` - Fields map[string]FieldUsage `json:"fields"` -} - -// FieldUsage represents the storage space used on disk by one field, on one node -type FieldUsage struct { - Total uint64 `json:"total"` - Fragments uint64 `json:"fragments"` - Keys uint64 `json:"keys"` - Metadata uint64 `json:"metadata"` -} - -// MemoryUsage represents the memory used by one node. -type MemoryUsage struct { - Capacity uint64 `json:"capacity"` - TotalUse uint64 `json:"totalInUse"` -} - -// Returns disk usage from cache if cache is large. It will recalculate on the spot if the last cacluation was under 5 seconds. -func (api *API) Usage(ctx context.Context, remote bool) (map[string]NodeUsage, error) { - span, _ := tracing.StartSpanFromContext(ctx, "API.Usage") - defer span.Finish() - - if api.usageCache.disable { - resp := make(map[string]NodeUsage) - return resp, nil - } - - api.usageCache.muAssign.Lock() - lastCalc := api.usageCache.lastCalcDuration - api.usageCache.muAssign.Unlock() - if lastCalc < usageCacheMinDuration { - err := api.ResetUsageCache() - if err != nil { - api.server.logger.Infof("could not reset usageCache: %s", err) - } - } - - api.usageCache.muAssign.Lock() - lastUpdated := api.usageCache.lastUpdated - api.usageCache.muAssign.Unlock() - if lastUpdated == (time.Time{}) { - api.calculateUsage() - } - - if !remote { - api.requestUsageOfNodes() - } - - return api.usageCache.data, nil -} - -// Makes a ui/usage request for each node in cluster to calculates its usage and adds it to the cache -func (api *API) requestUsageOfNodes() { - nodes := api.cluster.Nodes() - for _, node := range nodes { - if node.ID == api.server.nodeID { - continue - } - - nodeUsage, err := api.server.defaultClient.GetNodeUsage(context.Background(), &node.URI) - if err != nil { - api.server.logger.Infof("couldn't collect disk usage from %s: %s", node.URI, err) - } - - api.usageCache.muAssign.Lock() - api.usageCache.data[node.ID] = nodeUsage[node.ID] - api.usageCache.muAssign.Unlock() - } -} - -// Calculates disk usage from scratch if cache has expired for each index and stores the results in the usage cache -func (api *API) calculateUsage() { - // don't need to calculateUsage if we're about to close! - if api.isClosing() { - return - } - - api.usageCache.muCalculate.Lock() - defer api.usageCache.muCalculate.Unlock() - if ok := api.server.addToWaitGroup(1); !ok { - // the server is closing, so just stop! - return - } - defer api.server.wg.Done() - - api.usageCache.muAssign.Lock() - lastUpdated := api.usageCache.lastUpdated - api.usageCache.muAssign.Unlock() - - if time.Since(lastUpdated) <= api.usageCache.refreshInterval { - return - } - indexDetails, nodeMetadataBytes, err := api.holder.Txf().IndexUsageDetails(api.isClosing) - if err != nil { - api.server.logger.Infof("couldn't get index usage details: %s", err) - } - totalSize := nodeMetadataBytes - for _, s := range indexDetails { - totalSize += s.Total - } - - // NOTE: these errors are ignored in api.Info(), but checked here - si := api.server.systemInfo - diskCapacity, err := si.DiskCapacity(api.holder.path) - if err != nil { - api.server.logger.Infof("couldn't read disk capacity: %s", err) - } - - memoryCapacity, err := si.MemTotal() - if err != nil { - api.server.logger.Infof("couldn't read memory capacity: %s", err) - } - memoryUse, err := si.MemUsed() - if err != nil { - api.server.logger.Infof("couldn't read memory usage: %s", err) - } - - lastUpdated = time.Now() - // Insert into result. - nodeUsage := NodeUsage{ - Disk: DiskUsage{ - Capacity: diskCapacity, - TotalUse: totalSize, - IndexUsage: indexDetails, - }, - Memory: MemoryUsage{ - Capacity: memoryCapacity, - TotalUse: memoryUse, - }, - LastUpdated: lastUpdated, - } - api.usageCache.muAssign.Lock() - api.usageCache.data = make(map[string]NodeUsage) - api.usageCache.data[api.server.nodeID] = nodeUsage - api.usageCache.lastUpdated = lastUpdated - api.usageCache.muAssign.Unlock() -} - -// Periodically calculates disk/memory usage in terms of the duty cycle. The duty cycle represents the percentage of -// time that is spent recalculating this cache. It is specified relatively, rather than by a set interval, because -// scans can take an unpredictably long time. -func (api *API) RefreshUsageCache(dutyCycle float64) { - - if dutyCycle == 0 { - api.server.logger.Warnf("usage-duty-cycle set to 0, usage cache and /ui/usage endpoint are disabled") - api.usageCache = &usageCache{ - disable: true, - } - return - } - - trigger := make(chan bool) - defer close(trigger) - - multiplier := 100/dutyCycle - 1 - - api.usageCache = &usageCache{ - data: make(map[string]NodeUsage), - refreshInterval: usageCacheInitialInterval, - resetTrigger: trigger, - lastCalcDuration: 0, - waitMultiplier: multiplier, - } - api.server.logger.Infof("monitoring resource usage with duty cycle %v%%\n", dutyCycle) - for { - start := time.Now() - api.calculateUsage() - api.setRefreshInterval(time.Since(start)) - api.server.logger.Infof("updated resource usage cache at %v, took %v, next update in %v\n", api.usageCache.lastUpdated.Format(time.RFC3339), api.usageCache.lastCalcDuration.Truncate(time.Millisecond), api.usageCache.refreshInterval.Truncate(100*time.Millisecond)) - select { - case <-trigger: - continue - case <-api.server.closing: - return - case <-time.After(api.usageCache.refreshInterval): - continue - } - } -} - -// Refresh interval set in relation to how long the last calculation took. -func (api *API) setRefreshInterval(dur time.Duration) { - refresh := time.Duration(float64(dur) * api.usageCache.waitMultiplier) - if refresh < usageCacheMinInterval { - refresh = usageCacheMinInterval - } - api.usageCache.muAssign.Lock() - api.usageCache.refreshInterval = refresh - api.usageCache.lastCalcDuration = dur - api.usageCache.muAssign.Unlock() -} - -// Resets the lastUpdated time and awakens RefreshUsageCache() -func (api *API) ResetUsageCache() error { - if api.usageCache != nil { - api.usageCache.muAssign.Lock() - api.usageCache.lastUpdated = time.Time{} - api.usageCache.muAssign.Unlock() - } else { - return errors.New("invalidating cache: cache not initialized") - } - api.usageCache.resetTrigger <- true - return nil -} - -// isClosing returns true if the server is shutting down. -func (api *API) isClosing() bool { - select { - case <-api.server.closing: - return true - default: - return false - } -} - // RecalculateCaches forces all TopN caches to be updated. // This is done internally within a TopN query, but a user may want to do it ahead of time? func (api *API) RecalculateCaches(ctx context.Context) error { @@ -3256,24 +3001,24 @@ var methodsResizing = map[apiMethod]struct{}{ apiSchema: {}, } -var methodsDegraded = map[apiMethod]struct{}{ - apiExportCSV: {}, - apiFragmentBlockData: {}, - apiFragmentBlocks: {}, - apiField: {}, - apiIndex: {}, - apiQuery: {}, - apiRecalculateCaches: {}, - apiRemoveNode: {}, - apiShardNodes: {}, - apiSchema: {}, - apiViews: {}, - apiStartTransaction: {}, - apiFinishTransaction: {}, - apiTransactions: {}, - apiGetTransaction: {}, - apiActiveQueries: {}, -} +// var methodsDegraded = map[apiMethod]struct{}{ +// apiExportCSV: {}, +// apiFragmentBlockData: {}, +// apiFragmentBlocks: {}, +// apiField: {}, +// apiIndex: {}, +// apiQuery: {}, +// apiRecalculateCaches: {}, +// apiRemoveNode: {}, +// apiShardNodes: {}, +// apiSchema: {}, +// apiViews: {}, +// apiStartTransaction: {}, +// apiFinishTransaction: {}, +// apiTransactions: {}, +// apiGetTransaction: {}, +// apiActiveQueries: {}, +// } var methodsNormal = map[apiMethod]struct{}{ apiCreateField: {}, diff --git a/ctl/server.go b/ctl/server.go index 2d8de2df2..278005a40 100644 --- a/ctl/server.go +++ b/ctl/server.go @@ -90,9 +90,6 @@ func BuildServerFlags(cmd *cobra.Command, srv *server.Command) { flags.Uint16Var(&srv.Config.Postgres.ConnectionLimit, "postgres.connection-limit", srv.Config.Postgres.ConnectionLimit, "Maximum number of simultaneous postgres connections to allow. (set 0 to disable)") flags.Uint16Var(&srv.Config.Postgres.SqlVersion, "postgres.sql-version", srv.Config.Postgres.SqlVersion, "Molecula Sql Handling Version (default 1)") - // Disk and Memory usage cache for ui/usage endpoint - flags.Float64Var(&srv.Config.UsageDutyCycle, "usage-duty-cycle", srv.Config.UsageDutyCycle, "Sets the percentage of time that is spent recalculating the disk and memory usage cache. 100.0 for always-running, 0 disables the cache and the /ui/usage endpoint.") - // Future flags. flags.BoolVar(&srv.Config.Future.Rename, "future.rename", false, "Present application name as FeatureBase. Defaults to false, will default to true in an upcoming release.") diff --git a/http_handler.go b/http_handler.go index 44663ef14..fdbbb0688 100644 --- a/http_handler.go +++ b/http_handler.go @@ -452,7 +452,6 @@ func newRouter(handler *Handler) http.Handler { router.HandleFunc("/version", handler.handleGetVersion).Methods("GET").Name("GetVersion") // /ui endpoints are for UI use; they may change at any time. - router.HandleFunc("/ui/usage", handler.chkAuthZ(handler.handleGetUsage, authz.Read)).Methods("GET").Name("GetUsage") router.HandleFunc("/ui/transaction", handler.chkAuthZ(handler.handleGetTransactionList, authz.Read)).Methods("GET").Name("GetTransactionList") router.HandleFunc("/ui/transaction/", handler.chkAuthZ(handler.handleGetTransactionList, authz.Read)).Methods("GET").Name("GetTransactionList") router.HandleFunc("/ui/shard-distribution", handler.chkAuthZ(handler.handleGetShardDistribution, authz.Admin)).Methods("GET").Name("GetShardDistribution") @@ -987,63 +986,6 @@ func (h *Handler) handlePostSchema(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } -// handleGetUsage handles GET /ui/usage requests. -func (h *Handler) handleGetUsage(w http.ResponseWriter, r *http.Request) { - if !validHeaderAcceptJSON(r.Header) { - http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) - return - } - - q := r.URL.Query() - remoteStr := q.Get("remote") - var remote bool - if remoteStr == "true" { - remote = true - } - - nodeUsages, err := h.api.Usage(r.Context(), remote) - if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - } - - // if auth is turned on, filter results - if h.auth != nil { - g := r.Context().Value(contextKeyGroupMembership) - if g == nil { - http.Error(w, "Forbidden", http.StatusForbidden) - return - } - if !h.permissions.IsAdmin(g.([]authn.Group)) { - allowed := h.permissions.GetAuthorizedIndexList(g.([]authn.Group), authz.Read) - filteredNodeUsages := map[string]NodeUsage{} - - for nodeId, nodeUsage := range nodeUsages { - filteredIndexUsage := NodeUsage{ - Disk: DiskUsage{ - IndexUsage: map[string]IndexUsage{}, - }, - } - for index, idxUsage := range nodeUsage.Disk.IndexUsage { - // is it in auth list - for _, authd := range allowed { - if index == authd { - filteredIndexUsage.Disk.IndexUsage[index] = idxUsage - break - } - } - } - filteredNodeUsages[nodeId] = filteredIndexUsage - } - nodeUsages = filteredNodeUsages - } - } - - w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(nodeUsages); err != nil { - h.logger.Errorf("write status response error: %s", err) - } -} - // handleGetShardDistribution handles GET /ui/shard-distribution requests. func (h *Handler) handleGetShardDistribution(w http.ResponseWriter, r *http.Request) { dist := h.api.ShardDistribution(r.Context()) diff --git a/install/featurebase.conf b/install/featurebase.conf index a8894e8f9..6068046b0 100644 --- a/install/featurebase.conf +++ b/install/featurebase.conf @@ -244,28 +244,6 @@ log-path = "/var/log/molecula/featurebase.log" # enable-client-verification = true - -# ============================================================================== -# Usage Duty Cycle - Featurebase maintains a disk/memory usage cache that is -# calculated periodically in the background and accessed by the UI/usage -# endpoint. Since this disk scan can take a long and unpredictable amount of -# time, its timing behavior is specified in a relative, rather than absolute -# sense. That is, the duty cycle sets the percentage of time that is spent -# recalculating this cache. This setting affects the results received from -# the "/ui/usage" http endpoint, as well as all data file and memory usage -# values and graphs on the webui "tables" page - -# Special considerations: -# * If disk usage can be calculated quickly (less than 5 seconds), fresh -# results will be calculated when accessed -# * When disk usage takes longer to calculate, there is a minimum of one -# hour wait between cache recalculations -# Setting this value to 0 will completely disable the calculation of disk usage -# -# usage-duty-cycle = 20 - - - # ============================================================================== # Use [metric] stanza to define attributes for monitoring. # [metric] diff --git a/internal/clustertests/testdata/featurebase.conf b/internal/clustertests/testdata/featurebase.conf index eb587fbcb..e71ac5a9c 100644 --- a/internal/clustertests/testdata/featurebase.conf +++ b/internal/clustertests/testdata/featurebase.conf @@ -244,28 +244,6 @@ # enable-client-verification = true - -# ============================================================================== -# Usage Duty Cycle - Featurebase maintains a disk/memory usage cache that is -# calculated periodically in the background and accessed by the UI/usage -# endpoint. Since this disk scan can take a long and unpredictable amount of -# time, its timing behavior is specified in a relative, rather than absolute -# sense. That is, the duty cycle sets the percentage of time that is spent -# recalculating this cache. This setting affects the results received from -# the "/ui/usage" http endpoint, as well as all data file and memory usage -# values and graphs on the webui "tables" page - -# Special considerations: -# * If disk usage can be calculated quickly (less than 5 seconds), fresh -# results will be calculated when accessed -# * When disk usage takes longer to calculate, there is a minimum of one -# hour wait between cache recalculations -# Setting this value to 0 will completely disable the calculation of disk usage -# -# usage-duty-cycle = 20 - - - # ============================================================================== # Use [metric] stanza to define attributes for monitoring. # [metric] diff --git a/internal_client.go b/internal_client.go index fa1bc6653..70d2205bb 100644 --- a/internal_client.go +++ b/internal_client.go @@ -1383,38 +1383,6 @@ func (c *InternalClient) TranslateIDsNode(ctx context.Context, uri *pnet.URI, in return tkresp.Keys, nil } -// GetNodeUsage retrieves the size-on-disk information for the specified node. -func (c *InternalClient) GetNodeUsage(ctx context.Context, uri *pnet.URI) (map[string]NodeUsage, error) { - u := uri.Path("/ui/usage?remote=true") - req, err := http.NewRequest("GET", u, nil) - if err != nil { - return nil, errors.Wrap(err, "creating request") - } - - req.Header.Set("Accept", "application/json") - req.Header.Set("User-Agent", "pilosa/"+Version) - req = AddAuthToken(ctx, req) - - // Execute request against the host. - resp, err := c.executeRequest(req.WithContext(ctx)) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - // Read body and unmarshal response. - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - return nil, errors.Wrap(err, "reading") - } - - nodeUsages := make(map[string]NodeUsage) // map of size 1 - if err := json.Unmarshal(body, &nodeUsages); err != nil { - return nil, fmt.Errorf("unmarshal response: %s", err) - } - return nodeUsages, nil -} - // GetPastQueries retrieves the query history log for the specified node. func (c *InternalClient) GetPastQueries(ctx context.Context, uri *pnet.URI) ([]PastQueryStatus, error) { u := uri.Path("/query-history?remote=true") diff --git a/lattice/src/App/Home/ClusterHealth/ClusterHealth.tsx b/lattice/src/App/Home/ClusterHealth/ClusterHealth.tsx index 2809b2fd2..5dc836ad7 100644 --- a/lattice/src/App/Home/ClusterHealth/ClusterHealth.tsx +++ b/lattice/src/App/Home/ClusterHealth/ClusterHealth.tsx @@ -19,14 +19,12 @@ export const ClusterHealth: FC = () => { const [cluster, setCluster] = useState(); const [metrics, setMetrics] = useState(); const [info, setInfo] = useState(); - const [clusterData, setClusterData] = useState(); const [expanded, setExpanded] = useState([]); const [showMetrics, setShowMetrics] = useState(); const allExpanded = cluster && expanded.length === cluster.nodes.length; useEffectOnce(() => { getClusterHealth(); - getClusterData(); }); const refreshMetrics = useCallback(() => { @@ -38,15 +36,11 @@ export const ClusterHealth: FC = () => { useEffect(() => { const interval = setInterval(() => { - if (!clusterData) { - getClusterData(); - } - getClusterHealth(); refreshMetrics(); }, 15000); return () => clearInterval(interval); - }, [refreshMetrics, cluster, clusterData]); + }, [refreshMetrics, cluster]); const getClusterHealth = () => { pilosa.get @@ -76,13 +70,6 @@ export const ClusterHealth: FC = () => { .catch(() => setMetrics(undefined)); }; - const getClusterData = () => { - pilosa.get - .usage() - .then((res) => setClusterData(res.data)) - .catch(() => setClusterData(undefined)); - }; - const toggleAccordion = (nodeId: string) => { const isExpanded = expanded.includes(nodeId); if (isExpanded) { @@ -140,7 +127,6 @@ export const ClusterHealth: FC = () => { key={node.id} node={node} info={info} - usage={clusterData ? clusterData[node.id] : undefined} expanded={expanded.includes(node.id)} onToggle={() => toggleAccordion(node.id)} onMetricClick={() => setShowMetrics(node)} diff --git a/lattice/src/App/Home/ClusterHealth/Node/Node.tsx b/lattice/src/App/Home/ClusterHealth/Node/Node.tsx index d9a8a5cf3..92f88430f 100644 --- a/lattice/src/App/Home/ClusterHealth/Node/Node.tsx +++ b/lattice/src/App/Home/ClusterHealth/Node/Node.tsx @@ -21,7 +21,6 @@ import css from './Node.module.scss'; type NodeType = { node: any; info: any; - usage: any; expanded: boolean; onToggle: () => void; onMetricClick: () => void; @@ -30,24 +29,13 @@ type NodeType = { export const Node: FC = ({ node, info, - usage, expanded, onToggle, - onMetricClick + onMetricClick, }) => { const [copyHost, setCopyHost] = useState('Copy Host'); const [copyID, setCopyID] = useState('Click to Copy'); const { id, isPrimary, state } = node; - const diskTotalInUse = usage?.diskUsage?.totalInUse; - const diskCapacity = usage?.diskUsage?.capacity; - const diskUsagePercentage = diskCapacity - ? (diskTotalInUse / diskCapacity) * 100 - : undefined; - const memoryTotalInUse = usage?.memoryUsage?.totalInUse; - const memoryCapacity = usage?.memoryUsage?.capacity; - const memoryUsagePercentage = memoryCapacity - ? (memoryTotalInUse / memoryCapacity) * 100 - : undefined; const keys = Object.keys(info); const onCopyHostClick = () => { @@ -103,154 +91,6 @@ export const Node: FC = ({ -
-
-
Disk Usage:
-
- {usage ? ( - - - {formatBytes(diskTotalInUse)} - {diskCapacity - ? ` used out of ${formatBytes(diskCapacity)}` - : null} - -
- {diskUsagePercentage ? ( - - {diskUsagePercentage < 1 - ? '< 1' - : diskUsagePercentage.toLocaleString( - undefined, - { maximumFractionDigits: 1 } - )} - % used - - } - placement="top" - arrow - > -
- - ) : ( - - - {formatBytes(diskTotalInUse)} used - - } - placement="top" - arrow - > -
- - - Node disk capacity unknown - - - )} -
- - ) : ( - - Calculating... - - )} -
-
-
-
Memory Usage:
-
- {usage ? ( - - - {formatBytes(memoryTotalInUse)} - {memoryCapacity - ? ` used out of ${formatBytes(memoryCapacity)}` - : null} - -
- {memoryUsagePercentage ? ( - - {memoryUsagePercentage < 1 - ? '< 1' - : memoryUsagePercentage.toLocaleString( - undefined, - { maximumFractionDigits: 1 } - )} - % used - - } - placement="top" - arrow - > -
- - ) : ( - - - {formatBytes(memoryTotalInUse)} used - - } - placement="top" - arrow - > -
- - - Node memory capacity unknown - - - )} -
- - ) : ( - - Calculating... - - )} -
-
-
{keys.map((key) => { const showNode = Find(nodeInfo, (node) => node.name === key); diff --git a/lattice/src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx b/lattice/src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx index 72faaa4c1..b9e4c841f 100644 --- a/lattice/src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx +++ b/lattice/src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx @@ -20,19 +20,16 @@ import Tooltip from '@material-ui/core/Tooltip'; import Typography from '@material-ui/core/Typography'; import { Block } from 'shared/Block'; import { Pager } from 'shared/Pager'; -import { UsageBreakdown } from '../UsageBreakdown'; import css from './MoleculaTable.module.scss'; type MoleculaTableProps = { table: any; - dataDistribution: any; lastUpdated: string; }; export const MoleculaTable: FC = ({ table, - dataDistribution, - lastUpdated + lastUpdated, }) => { const [page, setPage] = useState(1); const [resultsPerPage, setResultsPerPage] = useState(10); @@ -45,42 +42,13 @@ export const MoleculaTable: FC = ({ const [sortDir, setSortDir] = useState<'asc' | 'desc'>('desc'); const lastUpdatedMoment = lastUpdated ? moment(lastUpdated).utc() : undefined; - useEffect(() => { - if (dataDistribution && !dataDistribution.uncached) { - const aggregatedFieldsData = Reduce( - dataDistribution.fields, - (result, value) => { - let newResult = {}; - const keys = Object.keys(value); - keys.forEach( - (key) => - (newResult[key] = { - total: result[key].total + value[key].total, - fragments: result[key].fragments + value[key].fragments, - keys: result[key].keys + value[key].keys, - metadata: result[key].metadata + value[key].metadata - }) - ); - return newResult; - } - ); - - const sorted = OrderBy(aggregatedFieldsData, ['total'], ['desc']); - if (sorted.length > 0) { - setMaxFieldSize(sorted[0].total); - } - - setFieldsData(aggregatedFieldsData); - } - }, [dataDistribution]); - useEffect(() => { if (searchText.length > 1) { const fuse = new Fuse(table.fields, { keys: ['name'], minMatchCharLength: 2, ignoreLocation: true, - threshold: 0 + threshold: 0, }); const result = fuse.search(searchText); @@ -131,46 +99,6 @@ export const MoleculaTable: FC = ({ {table.name} - {lastUpdatedMoment ? ( -
- {dataDistribution && dataDistribution.uncached ? ( - - Disk usage will be calculated at the next{` `} - - Disk and memory information shown here are read from a - cache, the behavior of which can be controlled with the{` `} - - --usage-duty-cycle - {' '} - command line flag. - - } - placement="top" - arrow - > - cache refresh - - . - - ) : ( - - Disk usage last updated{' '} - - - {lastUpdatedMoment.fromNow()} - - - . - - )} -
- ) : null}
@@ -180,9 +108,6 @@ export const MoleculaTable: FC = ({
-
- -
@@ -211,14 +136,14 @@ export const MoleculaTable: FC = ({ onSortClick('name')} > Name{' '} @@ -226,21 +151,6 @@ export const MoleculaTable: FC = ({ Type Cardinality Options - - onSortClick('total')} - > - Disk Usage{' '} - - - @@ -295,22 +205,6 @@ export const MoleculaTable: FC = ({ })}
- - - ); })} diff --git a/lattice/src/App/MoleculaTables/MoleculaTables.tsx b/lattice/src/App/MoleculaTables/MoleculaTables.tsx index cae1590fb..083c095d6 100644 --- a/lattice/src/App/MoleculaTables/MoleculaTables.tsx +++ b/lattice/src/App/MoleculaTables/MoleculaTables.tsx @@ -8,42 +8,29 @@ import Tooltip from '@material-ui/core/Tooltip'; import Typography from '@material-ui/core/Typography'; import { Block } from 'shared/Block'; import { SortBy } from 'shared/SortBy'; -import { UsageBreakdown } from './UsageBreakdown'; import { useHistory } from 'react-router-dom'; import css from './MoleculaTables.module.scss'; type MoleculaTablesProps = { tables: any; - dataDistribution: any; lastUpdated: string; maxSize: number; }; export const MoleculaTables: FC = ({ tables, - dataDistribution, lastUpdated, - maxSize + maxSize, }) => { const history = useHistory(); const [sortedTables, setSortedTables] = useState([]); const lastUpdatedMoment = lastUpdated ? moment(lastUpdated).utc() : undefined; useEffect(() => { - if (tables && dataDistribution) { - let aggregatedData: any[] = []; - tables.forEach((i) => - aggregatedData.push({ - ...dataDistribution[i.name], - ...i - }) - ); - - setSortedTables(aggregatedData); - } else if (tables) { + if (tables) { setSortedTables(tables); } - }, [tables, dataDistribution]); + }, [tables]); const handleSortChange = (value: any) => { const sortDirection = value === 'name' ? 'asc' : 'desc'; @@ -96,7 +83,7 @@ export const MoleculaTables: FC = ({ { label: 'Index Keys Size', value: 'indexKeys' }, { label: 'Fragment Size', value: 'fragments' }, { label: 'Field Keys Size', value: 'fieldKeysTotal' }, - { label: 'Metadata Size', value: 'metadata' } + { label: 'Metadata Size', value: 'metadata' }, ]} defaultValue="name" onChange={handleSortChange} @@ -111,22 +98,6 @@ export const MoleculaTables: FC = ({
{name}
-
- -
keys diff --git a/lattice/src/App/MoleculaTables/MoleculaTablesContainer.tsx b/lattice/src/App/MoleculaTables/MoleculaTablesContainer.tsx index 0e88eddfd..557c89cf7 100644 --- a/lattice/src/App/MoleculaTables/MoleculaTablesContainer.tsx +++ b/lattice/src/App/MoleculaTables/MoleculaTablesContainer.tsx @@ -12,7 +12,6 @@ export const MoleculaTablesContainer = () => { const history = useHistory(); const [tables, setTables] = useState(); const [selectedTable, setSelectedTable] = useState(); - const [dataDistribution, setDataDistribution] = useState(); const [maxSize, setMaxSize] = useState(0); const [lastUpdated, setLastUpdated] = useState(''); @@ -26,48 +25,6 @@ export const MoleculaTablesContainer = () => { .then((res) => setTables(res.data.indexes)) .catch((err) => console.log(err)) ); - - pilosa.get.usage().then((res) => { - const nodes = Object.keys(res.data); - let data = {}; - nodes.forEach((node) => { - const nodeIndexes = res.data[node].diskUsage.indexes; - const indexList = Object.keys(nodeIndexes); - indexList.forEach((i) => { - const nodeData = nodeIndexes[i]; - if (data[i]) { - data[i] = { - total: data[i].total + nodeData.total, - fieldKeysTotal: data[i].fieldKeysTotal + nodeData.fieldKeysTotal, - indexKeys: data[i].indexKeys + nodeData.indexKeys, - fragments: data[i].fragments + nodeData.fragments, - metadata: data[i].metadata + nodeData.metadata, - fields: [...data[i].fields, nodeData.fields] - }; - } else { - data[i] = { - total: nodeData.total, - fieldKeysTotal: nodeData.fieldKeysTotal, - indexKeys: nodeData.indexKeys, - fragments: nodeData.fragments, - metadata: nodeData.metadata, - fields: [nodeData.fields] - }; - } - }); - - if(!lastUpdated) { - setLastUpdated(res.data[node].lastUpdated); - } - }); - - const sorted = OrderBy(data, ['total'], ['desc']); - if (sorted.length > 0) { - setMaxSize(sorted[0].total); - } - - setDataDistribution(data); - }); }); useEffect(() => { @@ -85,21 +42,10 @@ export const MoleculaTablesContainer = () => { }, [match, tables, history]); return selectedTable ? ( - + ) : ( diff --git a/lattice/src/App/MoleculaTables/UsageBreakdown/UsageBreakdown.module.scss b/lattice/src/App/MoleculaTables/UsageBreakdown/UsageBreakdown.module.scss deleted file mode 100644 index 6e3cf558f..000000000 --- a/lattice/src/App/MoleculaTables/UsageBreakdown/UsageBreakdown.module.scss +++ /dev/null @@ -1,62 +0,0 @@ -.label { - font-size: 0.75rem; - color: var(--text-secondary); - margin-bottom: 4px; - font-weight: 400; -} - -.usageBreakdown { - display: flex; - align-items: center; - - .usageBreakdownLabel { - white-space: nowrap; - margin-right: 8px; - - &.smallLabel { - font-size: 12px; - } - } -} - -.breakdown { - display: flex; - align-items: center; - height: 13px; - border-radius: 4px; - background: rgba(var(--contrast-rgb), 0.1); - - .fieldKeysTotal { - height: 13px; - background: rgba(88, 80, 141, 0.7); - } - - .indexKeys { - height: 13px; - background: rgba(255, 99, 97, 0.7); - } - - .keys { - height: 13px; - background: rgba(88, 80, 141, 0.7); - } - - .fragments { - height: 13px; - background: rgba(255, 166, 0, 0.7); - } - - .metadata { - height: 13px; - background: rgba(188, 80, 144, 0.7); - } - - .bar:first-child { - border-top-left-radius: 4px; - border-bottom-left-radius: 4px; - } - .bar:last-child { - border-top-right-radius: 4px; - border-bottom-right-radius: 4px; - } -} diff --git a/lattice/src/App/MoleculaTables/UsageBreakdown/UsageBreakdown.tsx b/lattice/src/App/MoleculaTables/UsageBreakdown/UsageBreakdown.tsx deleted file mode 100644 index 78cc13c3d..000000000 --- a/lattice/src/App/MoleculaTables/UsageBreakdown/UsageBreakdown.tsx +++ /dev/null @@ -1,183 +0,0 @@ -import React, { FC, Fragment } from 'react'; -import classNames from 'classnames'; -import Tooltip from '@material-ui/core/Tooltip'; -import Typography from '@material-ui/core/Typography'; -import { formatBytes } from 'shared/utils/formatBytes'; -import css from './UsageBreakdown.module.scss'; - -type UsageBreakdownProps = { - data: any; - width?: string; - showLabel?: boolean; - usageValueSize?: 'small' | 'medium'; -}; - -export const UsageBreakdown: FC = ({ - data = {}, - width, - showLabel = true, - usageValueSize = 'medium' -}) => { - const { - total, - fieldKeysTotal, - indexKeys, - fragments, - metadata, - keys, - uncached - } = data; - const fieldKeysPercentage = - fieldKeysTotal && total ? (fieldKeysTotal / total) * 100 : 0; - const indexKeysPercentage = indexKeys ? (indexKeys / total) * 100 : 0; - const fragmentsPercentage = fragments ? (fragments / total) * 100 : 0; - const metadataPercentage = metadata ? (metadata / total) * 100 : 0; - const keysPercentage = keys && total ? (keys / total) * 100 : 0; - - return ( - - {showLabel ? : null} -
- {total ? ( - - - {formatBytes(total)} - -
- {fieldKeysTotal ? ( - - - - {formatBytes(fieldKeysTotal)} ( - {fieldKeysPercentage.toLocaleString(undefined, { - maximumFractionDigits: 1 - })} - %) - - - } - placement="top" - arrow - > -
- - ) : null} - {indexKeys ? ( - - - - {formatBytes(indexKeys)} ( - {indexKeysPercentage.toLocaleString(undefined, { - maximumFractionDigits: 1 - })} - %) - - - } - placement="top" - arrow - > -
- - ) : null} - {keys ? ( - - - - {formatBytes(keys)} ( - {keysPercentage.toLocaleString(undefined, { - maximumFractionDigits: 1 - })} - %) - - - } - placement="top" - arrow - > -
- - ) : null} - {fragments ? ( - - - - {formatBytes(fragments)} ( - {fragmentsPercentage.toLocaleString(undefined, { - maximumFractionDigits: 1 - })} - %) - - - } - placement="top" - arrow - > -
- - ) : null} - {metadata ? ( - - - - {formatBytes(metadata)} ( - {metadataPercentage.toLocaleString(undefined, { - maximumFractionDigits: 1 - })} - %) - - - } - placement="top" - arrow - > -
- - ) : null} -
- - ) : uncached ? ( - - Waiting... - - ) : ( - - Calculating... - - )} -
- - ); -}; diff --git a/lattice/src/App/MoleculaTables/UsageBreakdown/index.ts b/lattice/src/App/MoleculaTables/UsageBreakdown/index.ts deleted file mode 100644 index 36362bf49..000000000 --- a/lattice/src/App/MoleculaTables/UsageBreakdown/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './UsageBreakdown'; diff --git a/lattice/src/services/eventServices.tsx b/lattice/src/services/eventServices.tsx index b2adcfd33..a3a56e189 100644 --- a/lattice/src/services/eventServices.tsx +++ b/lattice/src/services/eventServices.tsx @@ -42,9 +42,6 @@ export const pilosa = { metrics() { return api.get('/metrics.json'); }, - usage() { - return api.get('/ui/usage'); - }, queryHistory() { return api.get('/query-history'); }, diff --git a/server/config.go b/server/config.go index 50b8ad261..29038e8b4 100644 --- a/server/config.go +++ b/server/config.go @@ -214,9 +214,6 @@ type Config struct { // LookupDBDSN is an external database to connect to for `ExternalLookup` queries. LookupDBDSN string `toml:"lookup-db-dsn"` - // The percentage of time spent recalculating the disk and memory usage cache. - UsageDutyCycle float64 `toml:"usage-duty-cycle"` - // Future flags are used to represent features or functionality which is not // yet the default behavior, but will be in a future release. Future struct { @@ -390,9 +387,6 @@ func NewConfig() *Config { c.Etcd.PeerCertFile = "" c.Etcd.PeerKeyFile = "" - // Disk and Memory Usage - c.UsageDutyCycle = 20.0 - // Future flags. c.Future.Rename = false diff --git a/server/handler_test.go b/server/handler_test.go index 15712f414..b0c074b63 100644 --- a/server/handler_test.go +++ b/server/handler_test.go @@ -517,48 +517,6 @@ func TestHandler_Endpoints(t *testing.T) { } }) - // UI/usage returns disk and memory usage from a precalculated cache. - // Since the cache calculates the cache on server startup, and tests create indexes thereafter - // the cache initially has 0 indexes when the test suite is ran. Therefore, this test first - // resets the cache. - t.Run("UI/usage", func(t *testing.T) { - if cmd.API.ResetUsageCache() != nil { - t.Fatal(err) - } - w := httptest.NewRecorder() - h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/ui/usage", nil)) - if w.Code != gohttp.StatusOK { - t.Fatalf("unexpected status code: %d", w.Code) - } - nodeUsages := make(map[string]pilosa.NodeUsage) - if err := json.Unmarshal(w.Body.Bytes(), &nodeUsages); err != nil { - t.Fatalf("unmarshal") - } - - for _, nodeUsage := range nodeUsages { - if nodeUsage.Disk.TotalUse < 1 { - t.Fatalf("expected some disk use, got %d", nodeUsage.Disk.TotalUse) - } - if nodeUsage.Disk.Capacity < 1 { - t.Fatalf("expected some disk capacity, got %d", nodeUsage.Disk.Capacity) - } - if nodeUsage.Memory.TotalUse < 1 { - t.Fatalf("expected some memory use, got %d", nodeUsage.Memory.TotalUse) - } - if nodeUsage.Memory.Capacity < 1 { - t.Fatalf("expected some memory capacity, got %d", nodeUsage.Memory.Capacity) - } - numIndexes := len(nodeUsage.Disk.IndexUsage) - if numIndexes != 3 { - t.Fatalf("wrong length index usage list: expected %d, got %d", 3, numIndexes) - } - numFields := len(nodeUsage.Disk.IndexUsage["i1"].Fields) - if numFields != len(i1.Fields()) { - t.Fatalf("wrong length field usage list: expected %d, got %d", len(i1.Fields()), numFields) - } - } - }) - t.Run("UI/shard-distribution", func(t *testing.T) { // This tests the response structure, not the cluster behavior. w := httptest.NewRecorder() diff --git a/server/server.go b/server/server.go index c73cabda3..5d02232d3 100644 --- a/server/server.go +++ b/server/server.go @@ -271,8 +271,6 @@ func (m *Command) Start() (err error) { } } - go m.API.RefreshUsageCache(m.Config.UsageDutyCycle) - _ = testhook.Opened(pilosa.NewAuditor(), m, nil) close(m.Started) return nil diff --git a/txfactory.go b/txfactory.go index 54dfa4390..4fddf79a9 100644 --- a/txfactory.go +++ b/txfactory.go @@ -4,7 +4,6 @@ package pilosa import ( "fmt" "os" - "path" "strings" "sync" @@ -471,192 +470,6 @@ func (f *TxFactory) DeleteFragmentFromStore( return f.dbPerShard.DeleteFragment(index, field, view, shard, frag) } -// IndexUsageDetails computes the sum of filesizes used by the node, broken down -// by index, field, fragments and keys. -func (f *TxFactory) IndexUsageDetails(isClosing func() bool) (map[string]IndexUsage, uint64, error) { - indexUsage := make(map[string]IndexUsage) - holderPath, err := expandDirName(f.holder.path) - if err != nil { - return indexUsage, 0, errors.Wrap(err, "expanding data directory") - } - indexesPath, err := expandDirName(f.holder.IndexesPath()) - if err != nil { - return indexUsage, 0, errors.Wrap(err, "expanding indexes directory") - } - - idxs := f.holder.Indexes() - - qcx := f.NewQcx() - defer qcx.Abort() - for _, idx := range idxs { - index := idx.name - indexPath := path.Join(indexesPath, index) - - // field usage - fieldUsages := make(map[string]FieldUsage) - fragmentsTotal := uint64(0) - fieldKeysTotal := uint64(0) - fieldMetaBytesTotal := uint64(0) - fieldsTotal := uint64(0) - flds := idx.Fields() - for _, fld := range flds { - field := fld.Name() - if field == "_keys" { - continue - } - fUsage, err := f.fieldUsage(indexPath, fld) - if err != nil { - return indexUsage, 0, errors.Wrapf(err, "getting disk usage for index (%s)", index) - } - - // non-roaring field usage - fragmentUsage := uint64(0) - - for _, shard := range fld.AvailableShards(true).Slice() { - if isClosing() { - return nil, 0, nil - } - if err := func() error { - tx, finisher, err := qcx.GetTx(Txo{Write: !writable, Index: idx, Shard: shard}) - if err != nil { - return errors.Wrap(err, "qcx.GetTx") - } - defer finisher(nil) - - fieldBytes, err := tx.GetFieldSizeBytes(index, field) - if err != nil { - return errors.Wrapf(err, "getting disk usage for non-roaring fragments (%s)", field) - } - fragmentUsage += fieldBytes - return nil - }(); err != nil { - return indexUsage, 0, err - } - } - - // add non-roaring to roaring - fUsage.Fragments += fragmentUsage - fUsage.Total += fragmentUsage - - // add to running total - fieldMetaBytesTotal += fUsage.Metadata - fieldKeysTotal += fUsage.Keys - fragmentsTotal += fUsage.Fragments - fieldsTotal += fUsage.Total - - fieldUsages[field] = fUsage - } - - // index metadata - indexMetaBytes, err := directoryUsage(indexPath, false) - if err != nil { - return indexUsage, 0, errors.Wrapf(err, "getting disk usage for index metadata (%s)", index) - } - - // index keys usage - indexKeysBytes := uint64(0) - if idx.keys { - keysPath := path.Join(indexPath, translateStoreDir) - indexKeysBytes, _ = directoryUsage(keysPath, true) // if directory doesn't exist, size = 0 - } - - indexUsage[index] = IndexUsage{ - Total: indexMetaBytes + indexKeysBytes + fieldsTotal, - Metadata: indexMetaBytes + fieldMetaBytesTotal, - IndexKeys: indexKeysBytes, - FieldKeysTotal: fieldKeysTotal, - Fragments: fragmentsTotal, - Fields: fieldUsages, - } - } - - // node metadata, e.g. id allocator - nodeMetaBytes, err := directoryUsage(holderPath, false) - if err != nil { - return indexUsage, 0, errors.Wrapf(err, "getting disk usage for node metadata") - } - - return indexUsage, nodeMetaBytes, nil -} - -// fieldUsage computes the sum of filesizes used by a field in -// the filesystem tree (roaring storage), broken down by keys and fragments. -func (f *TxFactory) fieldUsage(indexPath string, fld *Field) (FieldUsage, error) { - fieldUsage := FieldUsage{} - - field := fld.name - - // row keys - keysBytes := int64(0) - var err error - keysBytes, err = fileSize(fld.TranslateStorePath()) - if err != nil { - // if file doesn't exist, size = 0 - keysBytes = 0 - } - - // field metadata - fieldPath := path.Join(indexPath, FieldsDir, field) - metaBytes, err := directoryUsage(fieldPath, false) // this includes keys - if err != nil { - return fieldUsage, errors.Wrapf(err, "getting disk usage for field meta (%s)", field) - } - - // fragment data - viewsPath := path.Join(fieldPath, "views") - fragmentBytes := uint64(0) - if dirExists(viewsPath) { - fragmentBytes, err = directoryUsage(viewsPath, true) - if err != nil { - return fieldUsage, errors.Wrapf(err, "getting disk usage for field fragments (%s)", field) - } - } - - fieldUsage = FieldUsage{ - Total: metaBytes + fragmentBytes, // metaBytes includes keys - Metadata: metaBytes - uint64(keysBytes), - Fragments: fragmentBytes, - Keys: uint64(keysBytes), - } - - return fieldUsage, nil -} - -// NOTE: Go 1.16 introduced a new Readdir() method that is supposed to be more performant. -// Not yet upgraded b/c new method is not compatible with older versions of Go. -func directoryUsage(fname string, recursive bool) (uint64, error) { - if !dirExists(fname) { - return 0, errors.Errorf("directory does not exist (%s)", fname) - } - - var size uint64 - - dir, err := os.Open(fname) - if err != nil { - return 0, errors.Wrap(err, "opening data subdirectory") - } - defer dir.Close() - - files, err := dir.Readdir(-1) - if err != nil { - return 0, errors.Wrap(err, "reading data subdirectory") - } - - for _, file := range files { - if recursive && file.IsDir() { - sz, err := directoryUsage(path.Join(fname, file.Name()), true) - if err != nil { - return 0, err - } - size += sz - } else { - size += uint64(file.Size()) // NOTE this cast is safe for regular files, not necessarily others - } - } - - return size, nil -} - // CloseIndex is a no-op. This seems to be in place for debugging purposes. func (f *TxFactory) CloseIndex(idx *Index) error { return nil From 241550c751c9a99d5a1f09d07fc1722e4cd701cc Mon Sep 17 00:00:00 2001 From: reesporte Date: Mon, 28 Feb 2022 10:33:37 -0600 Subject: [PATCH 18/34] fix sonarcloud code smells --- lattice/src/App/Home/ClusterHealth/Node/Node.tsx | 3 +-- .../src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx | 7 +------ lattice/src/App/MoleculaTables/MoleculaTablesContainer.tsx | 5 ++--- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/lattice/src/App/Home/ClusterHealth/Node/Node.tsx b/lattice/src/App/Home/ClusterHealth/Node/Node.tsx index 92f88430f..8d934fac4 100644 --- a/lattice/src/App/Home/ClusterHealth/Node/Node.tsx +++ b/lattice/src/App/Home/ClusterHealth/Node/Node.tsx @@ -1,4 +1,4 @@ -import React, { FC, Fragment, useState } from 'react'; +import React, { FC, useState } from 'react'; import Button from '@material-ui/core/Button'; import copy from 'copy-to-clipboard'; import EqualizerIcon from '@material-ui/icons/EqualizerSharp'; @@ -11,7 +11,6 @@ import Find from 'lodash/find'; import IconButton from '@material-ui/core/IconButton'; import InfoIcon from '@material-ui/icons/Info'; import Tooltip from '@material-ui/core/Tooltip'; -import Typography from '@material-ui/core/Typography'; import { formatBytes } from 'shared/utils/formatBytes'; import { nodeInfo } from './nodeInfo'; import { NODE_STATE } from './nodeStatus'; diff --git a/lattice/src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx b/lattice/src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx index b9e4c841f..bb93b33ce 100644 --- a/lattice/src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx +++ b/lattice/src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx @@ -4,19 +4,16 @@ import Breadcrumbs from '@material-ui/core/Breadcrumbs'; import classNames from 'classnames'; import Fuse from 'fuse.js'; import Highlighter from 'react-highlight-words'; -import isEmpty from 'lodash/isEmpty'; import Link from '@material-ui/core/Link'; import map from 'lodash/map'; import moment from 'moment'; import OrderBy from 'lodash/orderBy'; -import Reduce from 'lodash/reduce'; import Table from '@material-ui/core/Table'; import TableBody from '@material-ui/core/TableBody'; import TableCell from '@material-ui/core/TableCell'; import TableHead from '@material-ui/core/TableHead'; import TableRow from '@material-ui/core/TableRow'; import TextField from '@material-ui/core/TextField'; -import Tooltip from '@material-ui/core/Tooltip'; import Typography from '@material-ui/core/Typography'; import { Block } from 'shared/Block'; import { Pager } from 'shared/Pager'; @@ -36,11 +33,9 @@ export const MoleculaTable: FC = ({ const sliceStart = (page - 1) * resultsPerPage; const [searchText, setSearchText] = useState(''); const [filteredFields, setFiltereedFields] = useState(table.fields); - const [fieldsData, setFieldsData] = useState<{}>({}); - const [maxFieldSize, setMaxFieldSize] = useState(0); + const [fieldsData] = useState<{}>({}); const [sort, setSort] = useState('total'); const [sortDir, setSortDir] = useState<'asc' | 'desc'>('desc'); - const lastUpdatedMoment = lastUpdated ? moment(lastUpdated).utc() : undefined; useEffect(() => { if (searchText.length > 1) { diff --git a/lattice/src/App/MoleculaTables/MoleculaTablesContainer.tsx b/lattice/src/App/MoleculaTables/MoleculaTablesContainer.tsx index 557c89cf7..c84a84243 100644 --- a/lattice/src/App/MoleculaTables/MoleculaTablesContainer.tsx +++ b/lattice/src/App/MoleculaTables/MoleculaTablesContainer.tsx @@ -1,5 +1,4 @@ import React, { useEffect, useState } from 'react'; -import OrderBy from 'lodash/orderBy'; import { MoleculaTable } from './MoleculaTable'; import { MoleculaTables } from './MoleculaTables'; import { pilosa } from 'services/eventServices'; @@ -12,8 +11,8 @@ export const MoleculaTablesContainer = () => { const history = useHistory(); const [tables, setTables] = useState(); const [selectedTable, setSelectedTable] = useState(); - const [maxSize, setMaxSize] = useState(0); - const [lastUpdated, setLastUpdated] = useState(''); + const [maxSize] = useState(0); + const [lastUpdated] = useState(''); useEffectOnce(() => { pilosa.get From 0f70253cc068dc7214cfc642fa92885644a7215b Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Mon, 28 Feb 2022 12:17:07 -0700 Subject: [PATCH 19/34] Add SQL SELECT mapping test --- sql/handler_test.go | 52 +++++++++++++++++++++++++++++++++++++++++++++ sql/query.go | 18 +++++++++------- sql/select.go | 4 ++-- 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/sql/handler_test.go b/sql/handler_test.go index 6eaba476f..26ffce0ce 100644 --- a/sql/handler_test.go +++ b/sql/handler_test.go @@ -3,10 +3,13 @@ package sql_test import ( "context" + "math" "testing" + "github.com/molecula/featurebase/v3" "github.com/molecula/featurebase/v3/sql" "github.com/molecula/featurebase/v3/test" + "vitess.io/vitess/go/vt/sqlparser" ) func TestHandler(t *testing.T) { @@ -28,3 +31,52 @@ func TestHandler(t *testing.T) { } } + +func TestSelectHandler_MapSelect(t *testing.T) { + cluster := test.MustRunCluster(t, 1) + defer cluster.Close() + api := cluster.GetNode(0).API + + if _, err := api.CreateIndex(context.Background(), "i", pilosa.IndexOptions{}); err != nil { + t.Fatal(err) + } else if _, err = api.CreateField(context.Background(), "i", "bytes", pilosa.OptFieldTypeInt(math.MinInt64, math.MaxInt64)); err != nil { + t.Fatal(err) + } else if _, err = api.CreateField(context.Background(), "i", "duration_time", pilosa.OptFieldTypeInt(math.MinInt64, math.MaxInt64)); err != nil { + t.Fatal(err) + } else if _, err = api.CreateField(context.Background(), "i", "timestamp", pilosa.OptFieldTypeTimestamp(pilosa.DefaultEpoch, pilosa.TimeUnitSeconds)); err != nil { + t.Fatal(err) + } + + for _, tt := range []struct { + name string + input string + output string + }{ + { + name: "WhereTimestamp", + input: `SELECT * FROM i WHERE timestamp>"2000-01-01T00:00:00Z"`, + output: `Extract(Row(timestamp>"2000-01-01T00:00:00Z"),Rows(bytes),Rows(duration_time),Rows(timestamp))`, + }, + + { + name: "WhereTimestampWithSpaces", + input: `SELECT * FROM i WHERE timestamp > "2000-01-01T00:00:00Z"`, + output: `Extract(Row(timestamp>"2000-01-01T00:00:00Z"),Rows(bytes),Rows(duration_time),Rows(timestamp))`, + }, + } { + t.Run(tt.name, func(t *testing.T) { + query, err := sql.NewMapper().MapSQL(tt.input) + if err != nil { + t.Fatal(err) + } + + h := sql.NewSelectHandler(api) + mr, err := h.MapSelect(context.Background(), query.Statement.(*sqlparser.Select), query.Mask) + if err != nil { + t.Fatal(err) + } else if got, want := mr.Query, tt.output; got != want { + t.Fatalf("unexpected pql\npql: %s\nwant: %s", got, want) + } + }) + } +} diff --git a/sql/query.go b/sql/query.go index 0f4db98b7..23eaa28fb 100644 --- a/sql/query.go +++ b/sql/query.go @@ -15,32 +15,32 @@ const timeFormat = "2006-01-02T15:04" // LT creates a less than query. func LT(fieldName string, value interface{}) string { - return fmt.Sprintf("Row(%s<%s)", fieldName, intOrFloat(value)) + return fmt.Sprintf("Row(%s<%s)", fieldName, formatValue(value)) } // LTE creates a less than or equal query. func LTE(fieldName string, value interface{}) string { - return fmt.Sprintf("Row(%s<=%s)", fieldName, intOrFloat(value)) + return fmt.Sprintf("Row(%s<=%s)", fieldName, formatValue(value)) } // GT creates a greater than query. func GT(fieldName string, value interface{}) string { - return fmt.Sprintf("Row(%s>%s)", fieldName, intOrFloat(value)) + return fmt.Sprintf("Row(%s>%s)", fieldName, formatValue(value)) } // GTE creates a greater than or equal query. func GTE(fieldName string, value interface{}) string { - return fmt.Sprintf("Row(%s>=%s)", fieldName, intOrFloat(value)) + return fmt.Sprintf("Row(%s>=%s)", fieldName, formatValue(value)) } // Equals creates an equals query. func Equals(fieldName string, value interface{}) string { - return fmt.Sprintf("Row(%s=%s)", fieldName, intOrFloat(value)) + return fmt.Sprintf("Row(%s=%s)", fieldName, formatValue(value)) } // NotEquals creates a not equals query. func NotEquals(fieldName string, value interface{}) string { - return fmt.Sprintf("Row(%s!=%s)", fieldName, intOrFloat(value)) + return fmt.Sprintf("Row(%s!=%s)", fieldName, formatValue(value)) } // NotNull creates a not equal to null query. @@ -94,7 +94,7 @@ func Like(fieldName string, pattern string) string { // Between creates a between query. func Between(fieldName string, a interface{}, b interface{}) string { - return fmt.Sprintf("Row(%s >< [%s,%s])", fieldName, intOrFloat(a), intOrFloat(b)) + return fmt.Sprintf("Row(%s >< [%s,%s])", fieldName, formatValue(a), formatValue(b)) } // Distinct creates a Distinct query. @@ -269,8 +269,10 @@ func formatIDKey(idKey interface{}) (string, error) { } } -func intOrFloat(value interface{}) string { +func formatValue(value interface{}) string { switch value.(type) { + case string: + return fmt.Sprintf("%q", value) case float64, float32: // In order to test expected values, we set the precision // to 8. TODO: It's likely we'll need to address this diff --git a/sql/select.go b/sql/select.go index 3297ee0fc..d7afc2eec 100644 --- a/sql/select.go +++ b/sql/select.go @@ -34,14 +34,14 @@ func (s *SelectHandler) Handle(ctx context.Context, mapped *MappedSQL) (pproto.T if !ok { return nil, fmt.Errorf("statement is not type select: %T", mapped.Statement) } - mr, err := s.mapSelect(ctx, stmt, mapped.Mask) + mr, err := s.MapSelect(ctx, stmt, mapped.Mask) if err != nil { return nil, errors.Wrap(err, "mapping select") } return s.execMappingResult(ctx, mr, mapped.SQL) } -func (s *SelectHandler) mapSelect(ctx context.Context, selectStmt *sqlparser.Select, qm QueryMask) (*MappingResult, error) { +func (s *SelectHandler) MapSelect(ctx context.Context, selectStmt *sqlparser.Select, qm QueryMask) (*MappingResult, error) { // Get the handler for this query mask. hndlr := s.router.handler(qm) if hndlr == nil { From 6948b18052a03d68fd46619f7e4603a2b374398b Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Wed, 23 Feb 2022 16:23:48 -0700 Subject: [PATCH 20/34] Add test coverage for RBF deletion --- rbf/tx.go | 48 ++++++++ rbf/tx_test.go | 305 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 352 insertions(+), 1 deletion(-) diff --git a/rbf/tx.go b/rbf/tx.go index f02a83783..83ffe92e2 100644 --- a/rbf/tx.go +++ b/rbf/tx.go @@ -201,6 +201,29 @@ func (tx *Tx) BitmapNames() ([]string, error) { return a, nil } +// BitmapExist returns true if bitmap exists. +func (tx *Tx) BitmapExists(name string) (bool, error) { + tx.mu.Lock() + defer tx.mu.Unlock() + return tx.bitmapExists(name) +} + +func (tx *Tx) bitmapExists(name string) (bool, error) { + if tx.db == nil { + return false, ErrTxClosed + } else if name == "" { + return false, ErrBitmapNameRequired + } + + // Read root records and find entry for bitmap. + records, err := tx.RootRecords() + if err != nil { + return false, err + } + _, ok := records.Get(name) + return ok, nil +} + // CreateBitmap creates a new empty bitmap with the given name. // Returns an error if the bitmap already exists. func (tx *Tx) CreateBitmap(name string) error { @@ -561,6 +584,31 @@ func (tx *Tx) Contains(name string, v uint64) (bool, error) { return c.Contains(v) } +// Depth returns the depth of the b-tree for a bitmap. +func (tx *Tx) Depth(name string) (int, error) { + tx.mu.RLock() + defer tx.mu.RUnlock() + + if tx.db == nil { + return 0, ErrTxClosed + } else if name == "" { + return 0, ErrBitmapNameRequired + } + + c, err := tx.cursor(name) + if err == ErrBitmapNotFound { + return 0, nil + } else if err != nil { + return 0, err + } + defer c.Close() + + if err := c.First(); err != nil { + return 0, err + } + return c.stack.top + 1, nil +} + // Cursor returns an instance of a cursor this bitmap. func (tx *Tx) Cursor(name string) (*Cursor, error) { tx.mu.RLock() diff --git a/rbf/tx_test.go b/rbf/tx_test.go index a02361315..1dc90a22b 100644 --- a/rbf/tx_test.go +++ b/rbf/tx_test.go @@ -444,7 +444,7 @@ func TestTx_DeallocateToFreeList(t *testing.T) { } } -func TestTx_Remove(t *testing.T) { +func TestTx_RemoveContainer(t *testing.T) { t.Parallel() db := MustOpenDB(t) @@ -542,6 +542,309 @@ func TestTx_AddRemove_Quick(t *testing.T) { }) } +func TestTx_Remove(t *testing.T) { + t.Run("FullContiguous", func(t *testing.T) { + if testing.Short() { + t.Skip("-short enabled, skipping") + } + + for _, bitN := range []uint64{1000, 100000, 2000000} { + t.Run(fmt.Sprint(bitN), func(t *testing.T) { + db := MustOpenDB(t) + defer MustCloseDB(t, db) + + // Add bits + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + + if err := tx.CreateBitmap("x"); err != nil { + t.Fatal(err) + } + for i := uint64(0); i < bitN; i++ { + if _, err := tx.Add("x", i); err != nil { + t.Fatalf("Add(%d) err=%q", i, err) + } + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + }() + + // Remove bits + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + + for i := uint64(0); i < bitN; i++ { + if _, err := tx.Remove("x", i); err != nil { + t.Fatalf("Remove(%d) err=%q", i, err) + } + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + }() + + // Verify that all bits have been removed. + tx := MustBegin(t, db, false) + defer tx.Rollback() + if n, err := tx.Count("x"); err != nil { + t.Fatal(err) + } else if got, want := n, uint64(0); got != want { + t.Fatalf("Count=%d, want %d", got, want) + } + }) + } + }) + + t.Run("PartialContiguous", func(t *testing.T) { + db := MustOpenDB(t) + defer MustCloseDB(t, db) + + // Add bits + const bitN = 100000 + const multiplier = 7 // space out bits so we span more containers + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + + if err := tx.CreateBitmap("x"); err != nil { + t.Fatal(err) + } + for i := uint64(0); i < bitN; i++ { + if _, err := tx.Add("x", i*multiplier); err != nil { + t.Fatalf("Add(%d) err=%q", i, err) + } + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + }() + + // Remove some bits in small contiguous chunks. + var deleteN int + for i := uint64(bitN / 2); i < bitN; { + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + for j := uint64(0); j < 100; i, j = i+1, j+1 { + if n, err := tx.Remove("x", i*multiplier); err != nil || n != 1 { + t.Fatalf("Remove(%d)=(%v,%q)", i, n, err) + } + deleteN++ + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + }() + } + + // Verify that we have the correct count afterward. + tx := MustBegin(t, db, false) + defer tx.Rollback() + if n, err := tx.Count("x"); err != nil { + t.Fatal(err) + } else if got, want := n, uint64(bitN-deleteN); got != want { + t.Fatalf("Count=%d, want %d", got, want) + } + }) + + t.Run("PartialNonContiguous", func(t *testing.T) { + db := MustOpenDB(t) + defer MustCloseDB(t, db) + + // Add bits + const bitN = 100000 + const multiplier = 7 // space out bits + bits := make([]uint64, 0, bitN) + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + + if err := tx.CreateBitmap("x"); err != nil { + t.Fatal(err) + } + for i := uint64(0); i < bitN; i++ { + if _, err := tx.Add("x", i*multiplier); err != nil { + t.Fatalf("Add(%d) err=%q", i, err) + } + bits = append(bits, i*multiplier) + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + }() + + // Remove some bits in small contiguous chunks. + var deleteN int + perm := rand.Perm(len(bits)) + for i := uint64(bitN / 2); i < bitN; { + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + for j := uint64(0); j < 100; i, j = i+1, j+1 { + value := bits[perm[i]] + if n, err := tx.Remove("x", value); err != nil || n != 1 { + t.Fatalf("Remove(%d)=(%v,%q)", value, n, err) + } + deleteN++ + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + }() + } + + // Verify that we have the correct count afterward. + tx := MustBegin(t, db, false) + defer tx.Rollback() + if n, err := tx.Count("x"); err != nil { + t.Fatal(err) + } else if got, want := n, uint64(bitN-deleteN); got != want { + t.Fatalf("Count=%d, want %d", got, want) + } + }) + + t.Run("DeleteEmptyBitmap", func(t *testing.T) { + db := MustOpenDB(t) + defer MustCloseDB(t, db) + + // Create bitmap. + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + if err := tx.CreateBitmap("x"); err != nil { + t.Fatal(err) + } else if err := tx.Commit(); err != nil { + t.Fatal(err) + } + }() + + // Remove bitmap. + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + if err := tx.DeleteBitmap("x"); err != nil { + t.Fatal(err) + } else if err := tx.Commit(); err != nil { + t.Fatal(err) + } + }() + + // Ensure bitmap no longer exists. + tx := MustBegin(t, db, false) + defer tx.Rollback() + if exists, err := tx.BitmapExists("x"); err != nil { + t.Fatal(err) + } else if exists { + t.Fatal("expected bitmap to be removed") + } + }) + + t.Run("WithTreeDepth", func(t *testing.T) { + for depth := 1; depth <= 3; depth++ { + t.Run(fmt.Sprint(depth), func(t *testing.T) { + db := MustOpenDB(t) + defer MustCloseDB(t, db) + + // Create bitmap & insert until we hit a tree depth. + var bitN int + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + if err := tx.CreateBitmap("x"); err != nil { + t.Fatal(err) + } + for i := uint64(0); ; i++ { + if _, err := tx.Add("x", i<<16); err != nil { + t.Fatalf("Add(%d) err=%q", i<<16, err) + } + bitN++ + + if d, err := tx.Depth("x"); err != nil { + t.Fatal(err) + } else if d == depth { + break + } + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + }() + + // Remove all bits in reverse order. + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + for i := bitN - 1; i >= 0; i-- { + if n, err := tx.Remove("x", uint64(i)<<16); err != nil || n != 1 { + t.Fatalf("Remove(%d)=(%v,%q)", uint64(i)<<16, n, err) + } + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + }() + + // Ensure bitmap no longer exists. + tx := MustBegin(t, db, false) + defer tx.Rollback() + for i := uint64(0); i < uint64(bitN); i++ { + if ok, err := tx.Contains("x", i<<16); err != nil || ok { + t.Fatalf("Contains(%d)=(%v,%q)", i<<16, ok, err) + } + } + }) + } + }) + + t.Run("RollbackAfterDelete", func(t *testing.T) { + db := MustOpenDB(t) + defer MustCloseDB(t, db) + + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + + if err := tx.CreateBitmap("x"); err != nil { + t.Fatal(err) + } else if err := tx.Commit(); err != nil { + t.Fatal(err) + } + }() + + // Add bits + const bitN = 1000 + for i := uint64(0); i < bitN; i++ { + func() { + tx := MustBegin(t, db, true) + defer tx.Rollback() + + if _, err := tx.Add("x", i<<16); err != nil { + t.Fatalf("Add(%d) err=%q", i<<16, err) + } + + // Only commit every other bit. + if i%2 == 1 { + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + } + }() + } + + // Verify that we have the correct count afterward. + tx := MustBegin(t, db, false) + defer tx.Rollback() + if n, err := tx.Count("x"); err != nil { + t.Fatal(err) + } else if got, want := n, uint64(bitN/2); got != want { + t.Fatalf("Count=%d, want %d", got, want) + } + }) +} + func TestTx_Multiple_CreateBitmap(t *testing.T) { rand := rand.New(rand.NewSource(0)) db := MustOpenDB(t) From 67f231215069665d2fc8261071b0c24bfb797b00 Mon Sep 17 00:00:00 2001 From: Seebs Date: Thu, 17 Feb 2022 14:16:40 -0600 Subject: [PATCH 21/34] trust cell.BitN now We used to manually do this because we had a number of cases where BitN wasn't being updated, but so far as we know we've fixed them and we have run a fair amount of stuff with sanity checks on and not hit anything, so eliminating the constant recounting on bitwise containers seems like a win. --- rbf/cursorx.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rbf/cursorx.go b/rbf/cursorx.go index fafdbf512..0a29f07f3 100644 --- a/rbf/cursorx.go +++ b/rbf/cursorx.go @@ -177,7 +177,7 @@ func intoContainer(l leafCell, tx *Tx, replacing *roaring.Container, target []by case ContainerTypeBitmapPtr: _, bm, _ := tx.leafCellBitmap(toPgno(cpMaybe)) cloneMaybe := bm - c = roaring.RemakeContainerBitmap(replacing, cloneMaybe) + c = roaring.RemakeContainerBitmapN(replacing, cloneMaybe, int32(l.BitN)) case ContainerTypeBitmap: c = roaring.RemakeContainerBitmapN(replacing, toArray64(cpMaybe), int32(l.BitN)) case ContainerTypeRLE: @@ -216,9 +216,9 @@ func toContainer(l leafCell, tx *Tx) (c *roaring.Container) { case ContainerTypeBitmapPtr: _, bm, _ := tx.leafCellBitmap(toPgno(cpMaybe)) cloneMaybe := bm - c = roaring.NewContainerBitmap(-1, cloneMaybe) + c = roaring.NewContainerBitmap(l.BitN, cloneMaybe) case ContainerTypeBitmap: - c = roaring.NewContainerBitmap(-1, toArray64(cpMaybe)) + c = roaring.NewContainerBitmap(l.BitN, toArray64(cpMaybe)) case ContainerTypeRLE: c = roaring.NewContainerRun(toInterval16(cpMaybe)) } From eb26a865187521356a442ad53fbf1044802137bc Mon Sep 17 00:00:00 2001 From: Seebs Date: Thu, 17 Feb 2022 13:29:10 -0600 Subject: [PATCH 22/34] implement a BSI-aware filter to avoid OffsetRange calls in fragment.sum We don't really need to fully extract every row, we just need counts. This naive approach uses logic similar to BitmapBitmapFilter, but tweaks it so that we can intercept the existence and sign bit rows, work with an optional filter, and yield a sum. We accumulate the statistics internally, rather than using a callback, because I tried to make it work with a callback and it was a complete mess. Note the fancy check for container reuse in the BSI Count filter. This is because intersection(full container, X) is just the original X, *not* a copy, but in this case we need a copy because RBF ApplyFilter will in fact reuse a single container's storage for each consecutive container. --- fragment.go | 63 ++++++++-------------- roaring/filter.go | 133 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 41 deletions(-) diff --git a/fragment.go b/fragment.go index 54d68eed9..c02e24699 100644 --- a/fragment.go +++ b/fragment.go @@ -770,50 +770,31 @@ func (f *fragment) setValueBase(txOrig Tx, columnID uint64, bitDepth uint64, val // sum returns the sum of a given bsiGroup as well as the number of columns involved. // A bitmap can be passed in to optionally filter the computed columns. func (f *fragment) sum(tx Tx, filter *Row, bitDepth uint64) (sum int64, count uint64, err error) { - // Compute count based on the existence row. - consider, err := f.row(tx, bsiExistsBit) - if err != nil { - return sum, count, err - } else if filter != nil { - consider = consider.Intersect(filter) - } - count = consider.Count() - - // Get negative set - nrow, err := f.row(tx, bsiSignBit) - if err != nil { - return sum, count, err - } - - // Filter negative set - nrow = consider.Intersect(nrow) - - // Get postive set - prow := consider.Difference(nrow) - - // Compute the sum based on the bit count of each row multiplied by the - // place value of each row. For example, 10 bits in the 1's place plus - // 4 bits in the 2's place plus 3 bits in the 4's place equals a total - // sum of 30: - // - // 10*(2^0) + 4*(2^1) + 3*(2^2) = 30 - // - // Execute once for positive numbers and once for negative. Subtract the - // negative sum from the positive sum. - for i := uint64(0); i < bitDepth; i++ { - row, err := f.row(tx, uint64(bsiOffsetBit+i)) - if err != nil { - return sum, count, err + // If there's a provided filter, but it has no contents for this particular + // shard, we're done and can return early. If there's no provided filter, + // though, we want to run with no-filter, as opposed to an empty filter. + var filterData *roaring.Bitmap + if filter != nil { + for _, seg := range filter.segments { + if seg.shard == f.shard { + filterData = seg.data + break + } } - - psum := int64((1 << i) * row.intersectionCount(prow)) - nsum := int64((1 << i) * row.intersectionCount(nrow)) - - // Squash to reduce the possibility of overflow. - sum += psum - nsum + // if filter is empty, we're done + if filterData == nil { + return 0, 0, nil + } + } + bsiFilt := roaring.NewBitmapBSICountFilter(filterData) + err = tx.ApplyFilter(f.index(), f.field(), f.view(), f.shard, 0, bsiFilt) + if err != nil && err != io.EOF { + return sum, count, errors.Wrap(err, "finding existing positions") } - return sum, count, nil + c32, sum := bsiFilt.Total() + + return sum, uint64(c32), nil } // min returns the min of a given bsiGroup as well as the number of columns involved. diff --git a/roaring/filter.go b/roaring/filter.go index fd1856af4..873337c23 100644 --- a/roaring/filter.go +++ b/roaring/filter.go @@ -879,3 +879,136 @@ func ApplyFilterToIterator(filter BitmapFilter, iter ContainerIterator) error { } return nil } + +// BitmapBSICountFilter gives counts of values in each value-holding row +// of a BSI field, constrained by a filter. The first row of the data is +// taken to be an existence bit, which is intersected into the filter to +// constrain it, and the second is used as a sign bit. The rows after that +// are treated as value rows, and their counts of bits, overlapping with +// positive and negative bits in the sign rows, are returned to a callback +// function. +// +// The total counts of positions evaluated are returned with a row count +// of ^uint64(0) prior to row counts. +type BitmapBSICountFilter struct { + containers []*Container + positive []*Container + negative []*Container + nextOffsets []uint64 + count int32 + psum, nsum uint64 +} + +func (b *BitmapBSICountFilter) Total() (count int32, total int64) { + return b.count, int64(b.psum) - int64(b.nsum) +} + +func (b *BitmapBSICountFilter) ConsiderKey(key FilterKey, n int32) FilterResult { + pos := key & keyMask + if b.containers[pos] == nil || n == 0 { + return key.RejectUntilOffset(b.nextOffsets[pos]) + } + return key.NeedData() +} + +func (b *BitmapBSICountFilter) ConsiderData(key FilterKey, data *Container) FilterResult { + pos := key & keyMask + filter := b.containers[pos] + if filter == nil { + key.RejectUntilOffset(b.nextOffsets[pos]) + } + row := uint64(key >> rowExponent) // row count within the fragment + // How do we translate the filter and existence bit into actionable things? + // Assume the sign row is empty. We want positive values for anything in + // the intersection of the filter and the positive bits. If the sign row + // isn't empty, we want positive values for that intersection, less the + // sign row, and negative for the intersection of the filter/positive and + // the sign bits. So we can just stash the intermediate filter+existence + // as positive, then split it up if we have sign bits, which we often don't. + setup := false + switch row { + case 0: // existence bit + b.positive[pos] = intersect(b.containers[pos], data) + if b.positive[pos] == data { + b.positive[pos] = b.positive[pos].Clone() + } + b.count += int32(b.positive[pos].N()) + setup = true + case 1: // sign bit + // split into negative/positive components. doesn't affect total + // count. + b.negative[pos] = intersect(b.positive[pos], data) + if b.negative[pos] == data { + b.negative[pos] = b.negative[pos].Clone() + } + b.positive[pos] = difference(b.positive[pos], data) + setup = true + } + // if we were doing setup (first two rows), we're done + if setup { + return key.MatchOneUntilOffset(b.nextOffsets[pos]) + } + // helpful reminder: a nil container is a valid empty container, and + // intersectionCount knows this. + pcount := intersectionCount(b.positive[pos], data) + ncount := intersectionCount(b.negative[pos], data) + b.psum += (uint64(pcount) << (row - 2)) + b.nsum += (uint64(ncount) << (row - 2)) + return key.MatchOneUntilOffset(b.nextOffsets[pos]) +} + +// NewBitmapBSICountFilter creates a BitmapBSICountFilter, used for tasks +// like computing the sum of a BSI field matching a given filter. +// +// The input filter is assumed to represent one "row" of a shard's data, +// which is to say, a range of up to rowWidth consecutive containers starting +// at some multiple of rowWidth. We coerce that to the 0..rowWidth range +// because offset-within-row is what we care about. +func NewBitmapBSICountFilter(filter *Bitmap) *BitmapBSICountFilter { + containers := make([]*Container, rowWidth*3) + b := &BitmapBSICountFilter{ + containers: containers[:rowWidth], + positive: containers[rowWidth : rowWidth*2], + negative: containers[rowWidth*2 : rowWidth*3], + nextOffsets: make([]uint64, rowWidth), + } + if filter == nil { + for i := range b.containers { + b.containers[i] = NewContainerRun([]Interval16{{Start: 0, Last: 65535}}) + b.nextOffsets[i] = uint64(i+1) % rowWidth + } + return b + } + count := 0 + iter, _ := filter.Containers.Iterator(0) + last := uint64(0) + for iter.Next() { + k, v := iter.Value() + // Coerce container key into the 0-rowWidth range we'll be + // using to compare against containers within each row. + k = k & keyMask + b.containers[k] = v + last = k + count++ + } + // if there's only one container, we need to populate everything with + // its position. + if count == 1 { + for i := range b.containers { + b.nextOffsets[i] = last + } + } else { + // Point each container at the offset of the next valid container. + // With sparse bitmaps this will potentially make skipping faster. + for i := range b.containers { + if b.containers[i] != nil { + for int(last) != i { + b.nextOffsets[last] = uint64(i) + last = (last + 1) % rowWidth + } + } + } + } + + return b +} From f5954d3cc6be8ab33ee16b2e6d78d09352f546e2 Mon Sep 17 00:00:00 2001 From: Seebs Date: Thu, 24 Feb 2022 14:59:40 -0600 Subject: [PATCH 23/34] use a pool for containerFilter objects We create a lot of these during a large GroupBy query or anything else that creates a ton of filters. Use a pool so we can reuse them, since most of their data doesn't need to be zeroed out, and typical use patterns have a lot of sequential creation of these short-lived things within a goroutine. --- rbf/tx.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/rbf/tx.go b/rbf/tx.go index 83ffe92e2..690937691 100644 --- a/rbf/tx.go +++ b/rbf/tx.go @@ -1258,6 +1258,22 @@ func (tx *Tx) ContainerIterator(name string, key uint64) (citer roaring.Containe return &containerIterator{cursor: c}, exact, nil } +// Shared pool for in-memory database pages. +// These are used before being flushed to disk. +var containerFilterPool = &sync.Pool{} + +func getContainerFilter(c *Cursor, filter roaring.BitmapFilter, tx *Tx) *containerFilter { + existing := containerFilterPool.Get() + if existing == nil { + return &containerFilter{cursor: c, filter: filter, tx: tx} + } + f := existing.(*containerFilter) + f.cursor = c + f.filter = filter + f.tx = tx + return f +} + func (tx *Tx) ApplyFilter(name string, key uint64, filter roaring.BitmapFilter) (err error) { tx.mu.RLock() defer tx.mu.RUnlock() @@ -1273,7 +1289,7 @@ func (tx *Tx) ApplyFilter(name string, key uint64, filter roaring.BitmapFilter) if err != nil { return err } - f := containerFilter{cursor: c, filter: filter, tx: tx} + f := getContainerFilter(c, filter, tx) defer f.Close() return f.Apply() } @@ -1615,6 +1631,8 @@ type containerFilter struct { func (s *containerFilter) Close() { s.cursor.Close() + s.cursor = nil + containerFilterPool.Put(s) } func (s *containerFilter) Apply() (err error) { From 287332d820e754d6cda80b8501ad5588e6c9b4c3 Mon Sep 17 00:00:00 2001 From: reesporte Date: Thu, 17 Feb 2022 17:44:12 -0600 Subject: [PATCH 24/34] use free id bucket to re-use ids this way memory usage doesn't grow without bound when we have lots of deletes and writes. fixes [fb-1187](https://molecula.atlassian.net/browse/FB-1187) --- boltdb/translate.go | 100 ++++++++++++++++++++++++++-- boltdb/translate_internal_test.go | 107 ++++++++++++++++++++++++++++++ boltdb/translate_test.go | 4 +- 3 files changed, 204 insertions(+), 7 deletions(-) create mode 100644 boltdb/translate_internal_test.go diff --git a/boltdb/translate.go b/boltdb/translate.go index 1c6f39a76..fe3d85d2c 100644 --- a/boltdb/translate.go +++ b/boltdb/translate.go @@ -34,7 +34,7 @@ var ( bucketKeys = []byte("keys") bucketIDs = []byte("ids") bucketFree = []byte("free") - FreeKey = []byte("free") + freeKey = []byte("free") ) const ( @@ -235,14 +235,26 @@ func (s *TranslateStore) CreateKeys(keys ...string) (map[string]uint64, error) { if idBucket == nil { return errors.Errorf(errFmtTranslateBucketNotFound, bucketIDs) } + freeBucket := tx.Bucket(bucketFree) + if freeBucket == nil { + return errors.Errorf(errFmtTranslateBucketNotFound, bucketFree) + } puts := 0 + + // we create a freeIDGetter to reduce marshalling + getter := newFreeIDGetter(freeBucket) + defer getter.Close() + for idx, key := range keys { id, boltKey := findIDByKey(keyBucket, key) if id != 0 { result[key] = id continue } - id = pilosa.GenerateNextPartitionedID(s.index, maxID(tx), s.partitionID, s.partitionN) + // see if we can re-use any IDs first + if id = getter.GetFreeID(); id == 0 { + id = pilosa.GenerateNextPartitionedID(s.index, maxID(tx), s.partitionID, s.partitionN) + } idBytes := idScratch[puts*8 : puts*8+8] binary.BigEndian.PutUint64(idBytes, id) puts++ @@ -527,7 +539,7 @@ func (s *TranslateStore) FreeIDs() (*roaring.Bitmap, error) { if bkt == nil { return errors.Errorf(errFmtTranslateBucketNotFound, bucketKeys) } - b := bkt.Get(FreeKey) + b := bkt.Get(freeKey) err := result.UnmarshalBinary(b) if err != nil { return err @@ -538,7 +550,7 @@ func (s *TranslateStore) FreeIDs() (*roaring.Bitmap, error) { } func (s *TranslateStore) MergeFree(tx *bolt.Tx, newIDs *roaring.Bitmap) error { bkt := tx.Bucket(bucketFree) - b := bkt.Get(FreeKey) + b := bkt.Get(freeKey) buf := new(bytes.Buffer) if b != nil { //if existing combine with newIDs before := roaring.NewBitmap() @@ -554,7 +566,7 @@ func (s *TranslateStore) MergeFree(tx *bolt.Tx, newIDs *roaring.Bitmap) error { } else { newIDs.WriteTo(buf) } - return bkt.Put(FreeKey, buf.Bytes()) + return bkt.Put(freeKey, buf.Bytes()) } // Delete removes the lookeup pairs in order to make avialble for reuse but doesn't commit the @@ -608,6 +620,84 @@ func findIDByKey(bkt *bolt.Bucket, key string) (uint64, []byte) { return 0, boltKey } +// freeIDGetter reduces the amount of marshaling required to get multiple ids +type freeIDGetter struct { + freeBucket *bolt.Bucket + b *roaring.Bitmap + changed bool +} + +// newFreeIDGetter initializes a new freeIDGetter. If at any point there is a +// failure, it returns an error. +// +// NOTE: For changes to be persisted to the bucket, you must call +// (*freeIDGetter).Close() +func newFreeIDGetter(freeBucket *bolt.Bucket) *freeIDGetter { + g := &freeIDGetter{ + freeBucket: freeBucket, + } + // we ignore this value because it's okay if we dont have a bitmap just yet + _ = g.getBitmap() + return g +} + +func (g *freeIDGetter) getBitmap() bool { + if g.b == nil { + // get the bitmap from freeBucket + value := g.freeBucket.Get(freeKey) + if value == nil { + return false + } + // turn the value into a bitmap + b := roaring.NewBitmap() + if err := b.UnmarshalBinary(value); err != nil { + return false + } + g.b = b + } + return true +} + +// GetFreeID tries to get a free ID from the free id bucket. If at any point it +// fails to do so, it returns a 0. Otherwise, it returns the first free ID in the +// bucket +func (g *freeIDGetter) GetFreeID() (id uint64) { + if !g.getBitmap() { + return 0 + } + // get the first free id + id, ok := g.b.Min() + if !ok { + return 0 + } + // remove that id from the free id bitmap + if changed, err := g.b.RemoveN(id); changed == 0 || err != nil { + return 0 + } else { + g.changed = true + } + return id +} + +// Close persists any changes to the bitmap back to the bucket and then nils the +// references for safety. +func (g *freeIDGetter) Close() error { + if g.changed { + // convert bitmap to binary + buf, err := g.b.MarshalBinary() + if err != nil { + return errors.Wrap(err, "closing free ID Getter") + } + // put updated bitmap back into the freeBucket + if err := g.freeBucket.Put(freeKey, buf); err != nil { + return errors.Wrap(err, "closing free ID Getter") + } + } + g.b = nil + g.freeBucket = nil + return nil +} + func findKeyByID(bkt *bolt.Bucket, id uint64) string { boltKey := bkt.Get(u64tob(id)) if bytes.Equal(boltKey, emptyKey) { diff --git a/boltdb/translate_internal_test.go b/boltdb/translate_internal_test.go new file mode 100644 index 000000000..29d5c6fbb --- /dev/null +++ b/boltdb/translate_internal_test.go @@ -0,0 +1,107 @@ +package boltdb + +import ( + "path/filepath" + "testing" + + "github.com/molecula/featurebase/v3/roaring" + bolt "go.etcd.io/bbolt" +) + +func TestGetFreeID(t *testing.T) { + boltDir := t.TempDir() + db, err := bolt.Open(filepath.Join(boltDir, "testDB"), 0600, nil) + if err != nil { + t.Fatalf("unexpected error opening test boltdb: %v", err) + } + defer db.Close() + + makeTestBucket := func(tx *bolt.Tx, b *roaring.Bitmap) *bolt.Bucket { + if b == nil { + t.Fatalf("unexpected nil bitmap") + } + free, err := tx.CreateBucketIfNotExists(bucketFree) + if err != nil { + t.Fatalf("unexpected error making freeBucket: %v", err) + } + buf, err := b.MarshalBinary() + if err != nil { + t.Fatalf("unexpected error marshaling bitmap (%v) to binary: %v", b, err) + } + if err := free.Put(freeKey, buf); err != nil { + t.Fatalf("unexpected error adding data (%v) to freeBucket: %v", b, err) + } + return free + } + + for name, test := range map[string]struct { + bits *roaring.Bitmap + want uint64 + }{ + "bucket is there, but nobody's home": { + bits: roaring.NewBitmap(), + want: 0, + }, + "good bucket": { + bits: roaring.NewBitmap(1, 2, 34, 55, 9000), + want: 1, + }, + } { + t.Run(name, func(t *testing.T) { + tx, err := db.Begin(true) + if err != nil { + t.Fatalf("unexpected error starting bolt transaction: %v", err) + } + defer tx.Rollback() + freeBucket := makeTestBucket(tx, test.bits) + + getter := newFreeIDGetter(freeBucket) + defer getter.Close() + if got := getter.GetFreeID(); got != test.want { + t.Fatalf("expected %v got %v", test.want, got) + } + }) + } + + t.Run("CorrectOrdering", func(t *testing.T) { + tx, err := db.Begin(true) + if err != nil { + t.Fatalf("unexpected error starting bolt transaction: %v", err) + } + defer tx.Rollback() + + bucket := makeTestBucket(tx, roaring.NewBitmap(1, 34, 2, 55, 9000)) + + getter := newFreeIDGetter(bucket) + defer getter.Close() + for _, want := range []uint64{1, 2, 34, 55, 9000} { + if got := getter.GetFreeID(); got != want { + t.Fatalf("expected %v got %v", want, got) + } + } + if got := getter.GetFreeID(); got != 0 { + t.Fatalf("expected 0 got %v", got) + } + }) + + t.Run("NotABitmap", func(t *testing.T) { + tx, err := db.Begin(true) + if err != nil { + t.Fatalf("unexpected error starting bolt transaction: %v", err) + } + defer tx.Rollback() + + free, err := tx.CreateBucketIfNotExists(bucketFree) + if err != nil { + t.Fatalf("unexpected error making freeBucket: %v", err) + } + if err := free.Put(freeKey, []byte("this isn't right!")); err != nil { + t.Fatalf("unexpected error adding data to freeBucket: %v", err) + } + getter := newFreeIDGetter(free) + defer getter.Close() + if got := getter.GetFreeID(); got != 0 { + t.Fatalf("expected 0 got %v", got) + } + }) +} diff --git a/boltdb/translate_test.go b/boltdb/translate_test.go index ed367c8b7..cd74244eb 100644 --- a/boltdb/translate_test.go +++ b/boltdb/translate_test.go @@ -455,7 +455,7 @@ func TestTranslateStore_ReadWrite(t *testing.T) { // Put the contents of the store into a buffer. buf := bytes.NewBuffer(nil) - expN := int64(32768) + expN := s.Size() // After this, the buffer should contain batch0. if n, err := s.WriteTo(buf); err != nil { @@ -505,7 +505,7 @@ func TestTranslateStore_ReadWrite(t *testing.T) { func MustOpenNewTranslateStore(tb testing.TB) *boltdb.TranslateStore { s := MustNewTranslateStore(tb) if err := s.Open(); err != nil { - panic(err) + tb.Fatalf("opening s: %v", err) } return s } From 18bddca86f2efe32157404be49ec6392565f33b9 Mon Sep 17 00:00:00 2001 From: reesporte Date: Mon, 28 Feb 2022 12:05:30 -0600 Subject: [PATCH 25/34] add get internal mem usage endpoint for use in benchmarking deletes --- http_handler.go | 20 ++++++++++++++++++++ http_handler_internal_test.go | 16 ++++++++++++++++ util.go | 17 +++++++++++++++++ util_test.go | 6 ++++++ 4 files changed, 59 insertions(+) diff --git a/http_handler.go b/http_handler.go index fdbbb0688..13ecd1f95 100644 --- a/http_handler.go +++ b/http_handler.go @@ -465,6 +465,7 @@ func newRouter(handler *Handler) http.Handler { router.HandleFunc("/internal/translate/data", handler.chkAuthZ(handler.handlePostTranslateData, authz.Write)).Methods("POST").Name("PostTranslateData") // other ones + router.HandleFunc("/internal/mem-usage", handler.chkAuthZ(handler.handleGetMemUsage, authz.Read)).Methods("GET").Name("GetUsage") router.HandleFunc("/internal/fragment/block/data", handler.chkAuthN(handler.handleGetFragmentBlockData)).Methods("GET").Name("GetFragmentBlockData") router.HandleFunc("/internal/fragment/blocks", handler.chkAuthN(handler.handleGetFragmentBlocks)).Methods("GET").Name("GetFragmentBlocks") router.HandleFunc("/internal/fragment/data", handler.chkAuthN(handler.handleGetFragmentData)).Methods("GET").Name("GetFragmentData") @@ -986,6 +987,25 @@ func (h *Handler) handlePostSchema(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } +// handleGetMemUsage handles GET /internal/mem-usage requests. +func (h *Handler) handleGetMemUsage(w http.ResponseWriter, r *http.Request) { + if !validHeaderAcceptJSON(r.Header) { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } + + use, err := GetMemoryUsage() + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(use); err != nil { + h.logger.Errorf("write mem usage response error: %s", err) + } +} + // handleGetShardDistribution handles GET /ui/shard-distribution requests. func (h *Handler) handleGetShardDistribution(w http.ResponseWriter, r *http.Request) { dist := h.api.ShardDistribution(r.Context()) diff --git a/http_handler_internal_test.go b/http_handler_internal_test.go index 455a40c64..59ae88e66 100644 --- a/http_handler_internal_test.go +++ b/http_handler_internal_test.go @@ -771,3 +771,19 @@ func NewTestAuth(t *testing.T) *authn.Auth { } return a } + +func TestHandleGetMemUsage(t *testing.T) { + h := Handler{ + logger: logger.NewStandardLogger(os.Stdout), + queryLogger: logger.NewStandardLogger(os.Stdout), + } + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/whatever", nil) + + h.handleGetMemUsage(w, r) + + resp := w.Result() + if resp.StatusCode != http.StatusOK { + t.Fatalf("expected %v, got %v", http.StatusOK, resp.StatusCode) + } +} diff --git a/util.go b/util.go index eb9f958ce..ce4323ec3 100644 --- a/util.go +++ b/util.go @@ -4,8 +4,11 @@ package pilosa // util.go: a place for generic, reusable utilities. import ( + "fmt" "reflect" "time" + + "github.com/shirou/gopsutil/v3/mem" ) // LeftShifted16MaxContainerKey is 0xffffffffffff0000. It is similar @@ -54,3 +57,17 @@ func GetLoopProgress(start time.Time, now time.Time, iteration uint, total uint) func FormatTimestampNano(value, base int64, timeUnit string) string { return time.Unix(0, (value+base)*TimeUnitNanos(timeUnit)).UTC().Format(time.RFC3339Nano) } + +type MemoryUsage struct { + Capacity uint64 `json:"capacity"` + TotalUse uint64 `json:"totalUsed"` +} + +// GetMemoryUsage gets the memory usage +func GetMemoryUsage() (MemoryUsage, error) { + usage, err := mem.VirtualMemory() + if usage == nil || err != nil { + return MemoryUsage{}, fmt.Errorf("reading virtual memory: %v", err) + } + return MemoryUsage{Capacity: usage.Total, TotalUse: usage.Used}, nil +} diff --git a/util_test.go b/util_test.go index 870625ad8..9a1bfe7f9 100644 --- a/util_test.go +++ b/util_test.go @@ -90,3 +90,9 @@ func TestFormatTimestampNano(t *testing.T) { t.Fatal("Timestamp not formatted properly") } } + +func TestGetMemoryUsage(t *testing.T) { + if _, err := GetMemoryUsage(); err != nil { + t.Fatalf("unexpected error getting memory usage: %v", err) + } +} From 422f532b89b7b17b2915ee6512262691955ef73e Mon Sep 17 00:00:00 2001 From: reesporte Date: Tue, 1 Mar 2022 10:54:53 -0600 Subject: [PATCH 26/34] go mod tidy --- go.mod | 1 - go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/go.mod b/go.mod index f089da554..529f2a29d 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,6 @@ require ( github.com/benbjohnson/immutable v0.3.0 github.com/buger/jsonparser v1.1.1 github.com/cespare/xxhash v1.1.0 - github.com/claygod/PiHex v0.0.0-20200916193129-5277802bfd7b // indirect github.com/davecgh/go-spew v1.1.1 github.com/desertbit/timer v0.0.0-20180107155436-c41aec40b27f // indirect github.com/dustin/go-humanize v1.0.0 // indirect diff --git a/go.sum b/go.sum index 2e0ea4375..79fc9f1f6 100644 --- a/go.sum +++ b/go.sum @@ -54,8 +54,6 @@ github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghf github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI= github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI= github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU= -github.com/claygod/PiHex v0.0.0-20200916193129-5277802bfd7b h1:LmxuKRxYbpulBnhu2ZYLfN92Zs2uitai6s6hpmCIZ1Q= -github.com/claygod/PiHex v0.0.0-20200916193129-5277802bfd7b/go.mod h1:iQyqZlmS/QK9N12+07jX1OO2xlzguGIE7vDmHh3TX+E= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa h1:OaNxuTZr7kxeODyLWsRMC+OD03aFUH+mW6r2d+MWa5Y= From e69ad74532b73419cb4bb0da71af3506d84b077e Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Tue, 1 Mar 2022 10:28:11 -0700 Subject: [PATCH 27/34] Enable multi-field WHERE clause for GROUP BY SQL queries --- sql/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/router.go b/sql/router.go index 8c6035dee..5f01579df 100644 --- a/sql/router.go +++ b/sql/router.go @@ -58,7 +58,7 @@ func newRouter() *router { groupByOptional := NewQueryMask( SelectPartField|SelectPartFields|SelectPartCountStar|SelectPartSumField, FromPartTable, - WherePartFieldCondition, // TODO: this can probably handle fields as well + WherePartFieldCondition|WherePartMultiFieldCondition, GroupByPartField|GroupByPartFields, HavingPartCondition, ) From 21a478a7281108243d0894a253fcd637ff0d04f0 Mon Sep 17 00:00:00 2001 From: Seebs Date: Tue, 1 Mar 2022 09:40:37 -0600 Subject: [PATCH 28/34] don't look up a field by name to find out its name If a field doesn't exist, looking up that field produces a nil, and querying the name of a nil field fails. Don't do that. Instead, just use the name you're looking it up by. We could in theory return an error here, but we already handle nonexistent fields elsewhere and checking this when we already have checks for it seems unnecessary, I think? Also, we add a test for this. The test is over in server/grpc_test.go because we have infrastructure there for testing the SQL server functionality, and you can't actually write reasonable self-contained tests for the SQL stuff because it has no way to create a working server. --- server/grpc_test.go | 4 ++++ sql/select.go | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/server/grpc_test.go b/server/grpc_test.go index e2ecae03f..782063859 100644 --- a/server/grpc_test.go +++ b/server/grpc_test.go @@ -1007,6 +1007,10 @@ func TestQuerySQLWithError(t *testing.T) { sql: "select _id, age, field_not_found from grouper", err: pilosa.ErrFieldNotFound, }, + { + sql: "select age, color, count(*) from grouper group by field_not_found, age, color", + err: pilosa.ErrFieldNotFound, + }, } for i, test := range tests { diff --git a/sql/select.go b/sql/select.go index d7afc2eec..2682a5349 100644 --- a/sql/select.go +++ b/sql/select.go @@ -598,8 +598,7 @@ func (h handlerSelectGroupBy) Apply(stmt *sqlparser.Select, qm QueryMask, indexF rowsQueries := []string{} for _, fieldName := range groupByFieldNames { - field := index.Field(fieldName) - rowsQueries = append(rowsQueries, Rows(field.Name())) + rowsQueries = append(rowsQueries, Rows(fieldName)) } var wherePQL string From 1222bf22cd8b2888af94712b5400352b3fb72748 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Tue, 1 Mar 2022 15:01:01 -0700 Subject: [PATCH 29/34] Use Distinct() call for SQL DISTINCT --- sql/query.go | 10 ++++++++-- sql/reduce.go | 28 ++++++++++++++++++++++++++++ sql/router.go | 11 +++++++++++ sql/select.go | 31 +++++++++++++------------------ 4 files changed, 60 insertions(+), 20 deletions(-) diff --git a/sql/query.go b/sql/query.go index 23eaa28fb..80a1d2f49 100644 --- a/sql/query.go +++ b/sql/query.go @@ -98,8 +98,14 @@ func Between(fieldName string, a interface{}, b interface{}) string { } // Distinct creates a Distinct query. -func Distinct(indexName, fieldName string) string { - return fmt.Sprintf("Distinct(Row(%s!=null),index='%s',field='%s')", fieldName, indexName, fieldName) +func Distinct(indexName, fieldName, rowCall string) string { + var b strings.Builder + fmt.Fprintf(&b, `Distinct(`) + if rowCall != "" { + fmt.Fprintf(&b, `%s, `, rowCall) + } + fmt.Fprintf(&b, `index='%s',field='%s')`, indexName, fieldName) + return b.String() } // RowDistinct creates a Distinct query with the given row filter. diff --git a/sql/reduce.go b/sql/reduce.go index 8de56c1f0..4aa9f09e7 100644 --- a/sql/reduce.go +++ b/sql/reduce.go @@ -347,6 +347,34 @@ func AssignHeaders(rowser pproto.ToRowser, headers ...Column) pproto.ToRowser { return &assignHeadersRowser{rowser, headers} } +type staticHeaderRowser struct { + rowser pproto.ToRowser + cols []Column +} + +func (a *staticHeaderRowser) ToRows(fn func(*pproto.RowResponse) error) error { + return a.rowser.ToRows(func(row *pproto.RowResponse) error { + var out pproto.RowResponse + + headers := make([]*pproto.ColumnInfo, len(row.Headers)) + for i := range row.Headers { + header := row.Headers[i] + header.Name = a.cols[i].Name() + headers[i] = header + } + out.Headers = headers + + out.Columns = row.Columns + + return fn(&out) + }) +} + +// StaticHeaders assigns fixed cols to a ToRowser. +func StaticHeaders(rowser pproto.ToRowser, cols ...Column) pproto.ToRowser { + return &staticHeaderRowser{rowser, cols} +} + var ( ErrIncompleteHeaders = errors.New("incomplete header assignment") ErrFieldNotInHeaders = errors.New("field not found in source header") diff --git a/sql/router.go b/sql/router.go index 5f01579df..381b180af 100644 --- a/sql/router.go +++ b/sql/router.go @@ -29,6 +29,17 @@ func newRouter() *router { handlerSelectFieldsFromTableWhere{}, ) //// + selectRouter.addFilter( + NewQueryMask( + SelectPartDistinct|SelectPartField, + FromPartTable, + WherePartFieldCondition|WherePartMultiFieldCondition, + 0, + 0, + ), + []QueryMask{}, + handlerSelectDistinctFromTable{}, + ) selectRouter.addRoute("select distinct fld from tbl", handlerSelectDistinctFromTable{}) //// selectRouter.addFilter( diff --git a/sql/select.go b/sql/select.go index 2682a5349..b1f11e0d8 100644 --- a/sql/select.go +++ b/sql/select.go @@ -305,6 +305,15 @@ func (h handlerSelectDistinctFromTable) Apply(stmt *sqlparser.Select, qm QueryMa return nil, errors.New("distinct requires a valid field column") } + var wherePQL string + if stmt.Where != nil { + if wherePQL, err = extractWhere(index, stmt.Where.Expr); err != nil { + return nil, err + } + } else { + wherePQL = All() + } + limit, offset, hasLimit, hasOffset, err := extractLimitOffset(stmt) if err != nil { return nil, errors.Wrap(err, "extracting limit") @@ -315,22 +324,8 @@ func (h handlerSelectDistinctFromTable) Apply(stmt *sqlparser.Select, qm QueryMa return nil, errors.Wrap(err, "extracting order by") } - // Determine the type of the field needing distinct. - // If the pilosa field is type int, handle it as a Distinct() query. - // Otherwise, use Rows() - // TODO: ensure this works for all field types (bool, time, etc). - var qo string - if fieldCol.Field.Type() == pilosa.FieldTypeInt || fieldCol.Field.Type() == pilosa.FieldTypeTimestamp { - qo = Distinct(fieldCol.Field.Index(), fieldCol.Field.Name()) - } else { - if !qm.HasOrderBy() && limit > 0 { - if qo, err = RowsLimit(fieldCol.Field.Name(), int64(limit)); err != nil { - return nil, errors.Wrap(err, "creating Rows query") - } - } else { - qo = Rows(fieldCol.Field.Name()) - } - } + // We use a Distinct call instead of Rows as it supports filtering. + qo := Distinct(fieldCol.Field.Index(), fieldCol.Field.Name(), wherePQL) mr := &MappingResult{ IndexName: indexName, @@ -340,7 +335,7 @@ func (h handlerSelectDistinctFromTable) Apply(stmt *sqlparser.Select, qm QueryMa // Assign headers to the result. mr.addReducer(func(result pproto.ToRowser) pproto.ToRowser { - return AssignHeaders(result, selectFields...) + return StaticHeaders(result, selectFields...) }) if qm.HasOrderBy() { @@ -795,7 +790,7 @@ func (h handlerSelectJoin) Apply(stmt *sqlparser.Select, qm QueryMask, indexFunc // Build the Distinct() portion of the query on the secondary. var distinctQry string if secondaryWhere == "" { - distinctQry = Distinct(secondaryField.Index(), secondaryField.Name()) + distinctQry = Distinct(secondaryField.Index(), secondaryField.Name(), "") } else { distinctQry = RowDistinct(secondaryField.Index(), secondaryField.Name(), secondaryWhere) } From 63c3a8b76192cba013e02dd886766ea6b16e7ca6 Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Tue, 1 Mar 2022 13:51:13 -0600 Subject: [PATCH 30/34] add "needs: []" to go tests race to make it start immediately also move race tests to a special "nonblocking" stage that is after everything else, so they don't block anything else from starting --- .gitlab/.gitlab-ci.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.gitlab/.gitlab-ci.yml b/.gitlab/.gitlab-ci.yml index d2c78d989..31f8de039 100644 --- a/.gitlab/.gitlab-ci.yml +++ b/.gitlab/.gitlab-ci.yml @@ -14,6 +14,7 @@ stages: - gauntlet - performance - post build + - nonblocking smoke build: image: golang:$GOVERSION @@ -84,11 +85,12 @@ run go tests: - aws run go tests race: - stage: test + stage: nonblocking # don't let this job block any other jobs because it takes much longer than the other tests. image: golang:$GOVERSION rules: - if: '$CI_PIPELINE_SOURCE == "push" || $CI_PIPELINE_SOURCE == "schedule" || $CI_PIPELINE_SOURCE == "web"' retry: 1 + needs: [] # don't wait to start running this. script: - echo "Running featurebase race tests..." - go test -race -v -timeout=90m ./... @@ -101,7 +103,7 @@ run go tests shardwidth22: rules: - if: '$CI_PIPELINE_SOURCE == "push" || $CI_PIPELINE_SOURCE == "schedule" || $CI_PIPELINE_SOURCE == "web"' script: - - echo "Running featurebase race tests..." + - echo "Running featurebase shardwidth22 tests..." - go test -timeout=30m -tags=shardwidth22 ./... tags: - aws From 0dc5aed8d77476a4c8b59704352b5de4de07d275 Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Tue, 1 Mar 2022 16:32:53 -0600 Subject: [PATCH 31/34] add "go mod tidy" CI check pulled this from IDK... we just had an issue where we had an unused dep in go.mod. --- .gitlab/.gitlab-ci.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.gitlab/.gitlab-ci.yml b/.gitlab/.gitlab-ci.yml index 31f8de039..a0b2ca8b3 100644 --- a/.gitlab/.gitlab-ci.yml +++ b/.gitlab/.gitlab-ci.yml @@ -36,6 +36,15 @@ golangci-lint: - echo "Checking for issues in new code" - golangci-lint run +go mod tidy: + stage: lint + image: golang:$GOVERSION + rules: + - if: '$CI_COMMIT_TAG == null && ($CI_PIPELINE_SOURCE == "push" || $CI_PIPELINE_SOURCE == "schedule" || $CI_PIPELINE_SOURCE == "web")' + script: + - go mod tidy + - git diff --exit-code -- go.mod go.sum + build lattice: stage: test image: node:14 From 17203e3441ed22bc4ee7eb097076b9ac9d747cca Mon Sep 17 00:00:00 2001 From: reesporte Date: Tue, 1 Mar 2022 10:11:17 -0600 Subject: [PATCH 32/34] remove cardinality calculation from schema/details this is related to work for [fb-1127](https://molecula.atlassian.net/browse/FB-1127) cardinality reporting has caused no shortage of issues such that we recommend disabling them almost everywhere. this commit removes the cardinality calculation for right now, as well as the option to enable/disable schema details. --- api.go | 42 ------------------- api_test.go | 23 ---------- ctl/server.go | 3 -- http_handler.go | 9 +++- .../MoleculaTable/MoleculaTable.tsx | 4 -- server/config.go | 6 --- server/handler_test.go | 40 ++---------------- server/server.go | 1 - 8 files changed, 10 insertions(+), 118 deletions(-) diff --git a/api.go b/api.go index 9e3aa0753..e0c4236f9 100644 --- a/api.go +++ b/api.go @@ -50,8 +50,6 @@ type API struct { importWorkerPoolSize int importWork chan importJob - schemaDetailsOn bool - Serializer Serializer } @@ -72,14 +70,6 @@ func OptAPIServer(s *Server) apiOption { } } -// Used to configure API option: schemaDetailsOn -func OptAPISchemaDetailsOn(isOn bool) apiOption { - return func(a *API) error { - a.schemaDetailsOn = isOn - return nil - } -} - func OptAPIImportWorkerPoolSize(size int) apiOption { return func(a *API) error { a.importWorkerPoolSize = size @@ -1021,38 +1011,6 @@ func (api *API) Schema(ctx context.Context, withViews bool) ([]*IndexInfo, error return api.holder.limitedSchema() } -// SchemaDetails returns information about each index in Pilosa including which -// fields they contain. Additional field information such as cardinality unless -// turned off via the schemaDetailsOn cli option. -func (api *API) SchemaDetails(ctx context.Context) ([]*IndexInfo, error) { - span, _ := tracing.StartSpanFromContext(ctx, "API.Schema") - defer span.Finish() - schema, err := api.holder.Schema() - if err != nil { - return nil, errors.Wrap(err, "getting schema") - } - if !api.schemaDetailsOn { - return schema, nil - } - for _, index := range schema { - for _, field := range index.Fields { - q := fmt.Sprintf("Count(Distinct(field=%s))", field.Name) - req := QueryRequest{Index: index.Name, Query: q} - resp, err := api.query(ctx, &req) - if err != nil { - return schema, errors.Wrapf(err, "querying cardinality (%s/%s)", index.Name, field.Name) - } - if len(resp.Results) == 0 { - continue - } - if card, ok := resp.Results[0].(uint64); ok { - field.Cardinality = &card - } - } - } - return schema, nil -} - // ApplySchema takes the given schema and applies it across the // cluster (if remote is false), or just to this node (if remote is // true). This is designed for the use case of replicating a schema diff --git a/api_test.go b/api_test.go index cd2595064..82ce1af4f 100644 --- a/api_test.go +++ b/api_test.go @@ -956,29 +956,6 @@ func TestAPI_IDAlloc(t *testing.T) { }) } -func TestAPI_SchemaDetailsOff(t *testing.T) { - cluster := test.MustRunCluster(t, 2) - defer cluster.Close() - cmd := cluster.GetNode(0) - err := cmd.API.SetAPIOptions(pilosa.OptAPISchemaDetailsOn(false)) - if err != nil { - t.Fatalf("could not toggle schema details to off: %v", err) - } - schema, err := cmd.API.SchemaDetails(context.Background()) - if err != nil { - t.Fatalf("getting schema: %v", err) - } - - for _, i := range schema { - for _, f := range i.Fields { - if f.Cardinality != nil { - t.Fatalf("expected nil cardinality, got: %v", *f.Cardinality) - } - } - } - -} - type mutexCheckIndex struct { index *pilosa.Index indexName string diff --git a/ctl/server.go b/ctl/server.go index 278005a40..497c0087c 100644 --- a/ctl/server.go +++ b/ctl/server.go @@ -93,9 +93,6 @@ func BuildServerFlags(cmd *cobra.Command, srv *server.Command) { // Future flags. flags.BoolVar(&srv.Config.Future.Rename, "future.rename", false, "Present application name as FeatureBase. Defaults to false, will default to true in an upcoming release.") - // Toggle /schema/details endpoint. - flags.BoolVar(&srv.Config.SchemaDetailsOn, "schema-details-on", true, "Disable /schema/details endpoint") - // OAuth2.0 identity provider configuration flags.BoolVar(&srv.Config.Auth.Enable, "auth.enable", false, "Enable AuthN/AuthZ of featurebase, disabled by default.") flags.StringVar(&srv.Config.Auth.ClientId, "auth.client-id", srv.Config.Auth.ClientId, "Identity Provider's Application/Client ID.") diff --git a/http_handler.go b/http_handler.go index 13ecd1f95..712994fe2 100644 --- a/http_handler.go +++ b/http_handler.go @@ -926,7 +926,12 @@ func (h *Handler) handleGetSchema(w http.ResponseWriter, r *http.Request) { } } -// handleGetSchema handles GET /schema/details requests. +// handleGetSchema handles GET /schema/details requests. This is essentially the +// same thing as a GET /schema request, except WithViews is turned on by default. +// Previously, /schema/details returned the cardinality of each field, but this was +// removed for performance reasons. If, at some point in the future, there is a more +// performant way to get the cardinality of a field, that information would be +// included here. func (h *Handler) handleGetSchemaDetails(w http.ResponseWriter, r *http.Request) { if !validHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) @@ -934,7 +939,7 @@ func (h *Handler) handleGetSchemaDetails(w http.ResponseWriter, r *http.Request) } w.Header().Set("Content-Type", "application/json") - schema, err := h.api.SchemaDetails(r.Context()) + schema, err := h.api.Schema(r.Context(), true) if err != nil { h.logger.Printf("error getting detailed schema: %s", err) return diff --git a/lattice/src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx b/lattice/src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx index bb93b33ce..6c09fdf27 100644 --- a/lattice/src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx +++ b/lattice/src/App/MoleculaTables/MoleculaTable/MoleculaTable.tsx @@ -144,7 +144,6 @@ export const MoleculaTable: FC = ({ Type - Cardinality Options @@ -171,9 +170,6 @@ export const MoleculaTable: FC = ({ {type} {showKeys ? (keys ? '(keys)' : '(ID)') : null} - - {cardinality ? cardinality.toLocaleString() : '-'} -
{map(rest, (value, key) => { diff --git a/server/config.go b/server/config.go index 29038e8b4..15fd7df0f 100644 --- a/server/config.go +++ b/server/config.go @@ -222,9 +222,6 @@ type Config struct { Rename bool `toml:"rename"` } `toml:"future"` - // Toggles /schema/details endpoint. If off, it returns empty. - SchemaDetailsOn bool `toml:"schema-details-on"` - Auth Auth } @@ -390,9 +387,6 @@ func NewConfig() *Config { // Future flags. c.Future.Rename = false - // Schema Details Toggle - c.SchemaDetailsOn = true - return c } diff --git a/server/handler_test.go b/server/handler_test.go index b0c074b63..92d500238 100644 --- a/server/handler_test.go +++ b/server/handler_test.go @@ -302,8 +302,7 @@ func TestHandler_Endpoints(t *testing.T) { } var bodySchema pilosa.Schema - if err := json.Unmarshal(w.Body.Bytes(), - &bodySchema); err != nil { + if err := json.Unmarshal(w.Body.Bytes(), &bodySchema); err != nil { t.Fatalf("unexpected unmarshalling error: %v", err) } // DO NOT COMPARE `CreatedAt` - reset to 0 @@ -316,9 +315,8 @@ func TestHandler_Endpoints(t *testing.T) { // var targetSchema pilosa.Schema - target := fmt.Sprintf(`{"indexes":[{"name":"i0","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"cardinality":0},{"name":"f1","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"cardinality":1,"views":[{"name":"standard"}]}],"shardWidth":%[1]d},{"name":"i1","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"cardinality":1,"views":[{"name":"standard"}]}],"shardWidth":%[1]d},{"name":"i2","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":1000,"keys":false},"cardinality":1,"views":[{"name":"standard"}]},{"name":"f1","options":{"type":"int","base":0,"bitDepth":0,"min":-100,"max":100,"keys":false,"foreignIndex":""},"cardinality":4,"views":[{"name":"bsig_f1"}]},{"name":"f2","options":{"type":"decimal","base":0,"scale":1,"bitDepth":0,"min":-10,"max":10,"keys":false},"cardinality":5,"views":[{"name":"bsig_f2"}]},{"name":"f3","options":{"type":"time","timeQuantum":"YMDH","keys":false,"noStandardView":false},"cardinality":1,"views":[{"name":"standard"}]},{"name":"f4","options":{"type":"mutex","cacheType":"ranked","cacheSize":5000,"keys":false},"cardinality":1,"views":[{"name":"standard"}]},{"name":"f5","options":{"type":"bool"},"cardinality":1,"views":[{"name":"standard"}]}],"shardWidth":%[1]d}]}`, pilosa.ShardWidth) - if err := json.Unmarshal([]byte(target), - &targetSchema); err != nil { + target := fmt.Sprintf(`{"indexes":[{"name":"i0","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false}},{"name":"f1","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"views":[{"name":"standard"}]}],"shardWidth":%[1]d},{"name":"i1","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":50000,"keys":false},"views":[{"name":"standard"}]}],"shardWidth":%[1]d},{"name":"i2","options":{"keys":false,"trackExistence":false},"fields":[{"name":"f0","options":{"type":"set","cacheType":"ranked","cacheSize":1000,"keys":false},"views":[{"name":"standard"}]},{"name":"f1","options":{"type":"int","base":0,"bitDepth":0,"min":-100,"max":100,"keys":false,"foreignIndex":""},"views":[{"name":"bsig_f1"}]},{"name":"f2","options":{"type":"decimal","base":0,"scale":1,"bitDepth":0,"min":-10,"max":10,"keys":false},"views":[{"name":"bsig_f2"}]},{"name":"f3","options":{"type":"time","timeQuantum":"YMDH","keys":false,"noStandardView":false},"views":[{"name":"standard"}]},{"name":"f4","options":{"type":"mutex","cacheType":"ranked","cacheSize":5000,"keys":false},"views":[{"name":"standard"}]},{"name":"f5","options":{"type":"bool"},"views":[{"name":"standard"}]}],"shardWidth":%[1]d}]}`, pilosa.ShardWidth) + if err := json.Unmarshal([]byte(target), &targetSchema); err != nil { t.Fatalf("unexpected unmarshalling error: %v", err) } @@ -327,38 +325,6 @@ func TestHandler_Endpoints(t *testing.T) { } }) - t.Run("SchemaDetailsOff", func(t *testing.T) { - err := cmd.API.SetAPIOptions(pilosa.OptAPISchemaDetailsOn(false)) - if err != nil { - t.Fatalf("setting schema details option") - } - - w := httptest.NewRecorder() - h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/schema/details", nil)) - if w.Code != gohttp.StatusOK { - t.Fatalf("unexpected status code: %d", w.Code) - } - - var bodySchema pilosa.Schema - if err := json.Unmarshal(w.Body.Bytes(), - &bodySchema); err != nil { - t.Fatalf("unexpected unmarshalling error: %v", err) - - } - for _, i := range bodySchema.Indexes { - for _, f := range i.Fields { - if f.Cardinality != nil { - t.Fatalf("expected nil cardinality, got: %v", *f.Cardinality) - } - } - } - - err = cmd.API.SetAPIOptions(pilosa.OptAPISchemaDetailsOn(true)) - if err != nil { - t.Fatalf("could not toggle schema details to on: %v", err) - } - }) - t.Run("Import", func(t *testing.T) { indexInfo, err := cmd.API.Schema(context.Background(), false) if err != nil { diff --git a/server/server.go b/server/server.go index 5d02232d3..5e368cfab 100644 --- a/server/server.go +++ b/server/server.go @@ -509,7 +509,6 @@ func (m *Command) SetupServer() error { m.API, err = pilosa.NewAPI( pilosa.OptAPIServer(m.Server), pilosa.OptAPIImportWorkerPoolSize(m.Config.ImportWorkerPoolSize), - pilosa.OptAPISchemaDetailsOn(m.Config.SchemaDetailsOn), ) if err != nil { return errors.Wrap(err, "new api") From 3fc271ff0760783d0b094290f957c459f1b39f0d Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Mon, 28 Feb 2022 11:28:30 -0600 Subject: [PATCH 33/34] change release format This was in response to some feedback we got about the new release format. Executables were no longer had executable permission due to going through S3 (hence the tarballs), and we wanted a more consistent directory structure in the final release which included the versions of various components. --- .gitlab/.gitlab-ci.yml | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/.gitlab/.gitlab-ci.yml b/.gitlab/.gitlab-ci.yml index a0b2ca8b3..ffe441fbd 100644 --- a/.gitlab/.gitlab-ci.yml +++ b/.gitlab/.gitlab-ci.yml @@ -517,18 +517,21 @@ s3 dump tag: - aws configure set aws_secret_access_key $AWS_FBCI_SECRET_ACCESS_KEY - aws configure set region "us-east-2" - aws configure set aws_profile $PROFILE - - aws s3 cp featurebase_linux_amd64 s3://${LOCATION}/${CI_COMMIT_TAG}/featurebase_linux_amd64 - - aws s3 cp roaring-migrate_linux_amd64 s3://${LOCATION}/${CI_COMMIT_TAG}/roaring-migrate_linux_amd64 - - aws s3 cp featurebase_linux_arm64 s3://${LOCATION}/${CI_COMMIT_TAG}/featurebase_linux_arm64 - - aws s3 cp roaring-migrate_linux_arm64 s3://${LOCATION}/${CI_COMMIT_TAG}/roaring-migrate_linux_arm64 - - aws s3 cp featurebase_darwin_amd64 s3://${LOCATION}/${CI_COMMIT_TAG}/featurebase_darwin_amd64 - - aws s3 cp roaring-migrate_darwin_amd64 s3://${LOCATION}/${CI_COMMIT_TAG}/roaring-migrate_darwin_amd64 - - aws s3 cp featurebase_darwin_arm64 s3://${LOCATION}/${CI_COMMIT_TAG}/featurebase_darwin_arm64 - - aws s3 cp roaring-migrate_darwin_arm64 s3://${LOCATION}/${CI_COMMIT_TAG}/roaring-migrate_darwin_arm64 - - aws s3 cp NOTICE s3://${LOCATION}/${CI_COMMIT_TAG}/NOTICE - - aws s3 cp install/featurebase.debian.service s3://${LOCATION}/${CI_COMMIT_TAG}/featurebase.debian.service - - aws s3 cp install/featurebase.redhat.service s3://${LOCATION}/${CI_COMMIT_TAG}/featurebase.redhat.service - - aws s3 cp install/featurebase.conf s3://${LOCATION}/${CI_COMMIT_TAG}/featurebase.conf + - | + for goos in "darwin" "linux"; do + for goarch in "amd64" "arm64"; do + dir=featurebase-${CI_COMMIT_TAG}-${goos}-${goarch} + echo "Directory ${dir}" + mkdir $dir + mv featurebase_${goos}_${goarch} ${dir}/featurebase + mv roaring-migrate_${goos}_${goarch} ${dir}/roaring-migrate + cp NOTICE install/featurebase.conf install/featurebase.*.service ${dir}/ + tar cvzf ${dir}.tar.gz ${dir} + aws s3 cp ${dir} s3://${LOCATION}/${CI_COMMIT_TAG}/${dir}/ --recursive + aws s3 cp ${dir}.tar.gz s3://${LOCATION}/${CI_COMMIT_TAG}/ + done + done + needs: - job: build for darwin amd64 - job: build for darwin arm64 From d9ad819fe995aef53db5e912a2d69bbb024e39bb Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Thu, 3 Mar 2022 12:35:54 -0700 Subject: [PATCH 34/34] Revert auto-quoting in Web UI --- lattice/src/App/Query/QueryContainer.tsx | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/lattice/src/App/Query/QueryContainer.tsx b/lattice/src/App/Query/QueryContainer.tsx index a26190eb0..45401a7ca 100644 --- a/lattice/src/App/Query/QueryContainer.tsx +++ b/lattice/src/App/Query/QueryContainer.tsx @@ -119,19 +119,7 @@ export const QueryContainer: FC<{}> = () => { setLoading(false); } } else { - let queryArr = query.split(' '); - queryArr.forEach((word, idx) => { - if (word.includes('-')) { - let wordArr = word.split('.'); - wordArr.forEach((section, idx) => { - if (section.includes('-') && !word.includes('`')) { - wordArr[idx] = `\`${wordArr[idx]}\``; - } - }); - queryArr[idx] = wordArr.join('.'); - } - }); - querySQL(queryArr.join(' '), handleQueryMessages, handleQueryEnd); + querySQL(query, handleQueryMessages, handleQueryEnd); } } };