From 1e816b401cc65b99b3d8bfa004a20256ad0e7c97 Mon Sep 17 00:00:00 2001 From: Travis Date: Fri, 27 Jan 2017 13:16:27 -0600 Subject: [PATCH 01/35] Adds Todd's performance improvements: - Ingore asserts in fragment container. - Only log queries that take longer than 90 seconds. TODO: - address the TODOs that make the asserts configurable. --- fragment.go | 16 +++++++++++----- handler.go | 5 ++++- roaring/roaring.go | 24 ++++++++++++++---------- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/fragment.go b/fragment.go index 1181f1739..aaef9323a 100644 --- a/fragment.go +++ b/fragment.go @@ -350,6 +350,11 @@ func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, bool error) return false, err } + // Don't update the cache if nothing changed. + if !changed { + return changed, nil + } + // Invalidate block checksum. delete(f.checksums, int(bitmapID/HashBlockSize)) @@ -389,6 +394,11 @@ func (f *Fragment) clearBit(bitmapID, profileID uint64) (bool, error) { return false, err } + // Don't update the cache if nothing changed. + if !changed { + return changed, nil + } + // Invalidate block checksum. delete(f.checksums, int(bitmapID/HashBlockSize)) @@ -838,7 +848,6 @@ func (f *Fragment) Import(bitmapIDs, profileIDs []uint64) error { // Process every bit. // If an error occurs then reopen the storage. lastID := uint64(0) - bmCounter := 0 if err := func() error { set := make(map[uint64]struct{}) for i := range bitmapIDs { @@ -851,7 +860,7 @@ func (f *Fragment) Import(bitmapIDs, profileIDs []uint64) error { } // Write to storage. - changed, err := f.storage.Add(pos) + _, err = f.storage.Add(pos) if err != nil { return err } @@ -863,9 +872,6 @@ func (f *Fragment) Import(bitmapIDs, profileIDs []uint64) error { lastID = bitmapID set[bitmapID] = struct{}{} } - if changed { - bmCounter += 1 - } // Invalidate block checksum. delete(f.checksums, int(bitmapID/HashBlockSize)) diff --git a/handler.go b/handler.go index 175161384..9a704e6c8 100644 --- a/handler.go +++ b/handler.go @@ -202,7 +202,10 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { http.NotFound(w, r) } - h.logger().Printf("%s %s %.03fs", r.Method, r.URL.String(), time.Since(t).Seconds()) + dif := time.Since(t).Seconds() + if dif > 90 { + h.logger().Printf("%s %s %.03fs", r.Method, r.URL.String(), dif) + } } // handleGetSchema handles GET /schema requests. diff --git a/roaring/roaring.go b/roaring/roaring.go index 44b9c9341..efbf48374 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -460,8 +460,9 @@ func (b *Bitmap) WriteTo(w io.Writer) (n int64, err error) { c := b.containers[i] // Verify container count before writing. - count := c.count() - assert(c.count() == c.n, "cannot write container count, mismatch: count=%d, n=%d", count, c.n) + // TODO: instead of commenting this out, we need to make it a configuration option + //count := c.count() + //assert(c.count() == c.n, "cannot write container count, mismatch: count=%d, n=%d", count, c.n) binary.LittleEndian.PutUint64(buf[headerSize+i*12:], uint64(key)) binary.LittleEndian.PutUint32(buf[headerSize+i*12+8:], uint32(c.n-1)) @@ -532,9 +533,10 @@ func (b *Bitmap) UnmarshalBinary(data []byte) error { c := b.containers[i] if c.n <= ArrayMaxSize { c.array = (*[0xFFFFFFF]uint32)(unsafe.Pointer(&data[offset]))[:c.n] - for _, v := range c.array { - assert(lowbits(uint64(v)) == v, "array value out of range: %d", v) - } + // TODO: instead of commenting this out, we need to make it a configuration option + //for _, v := range c.array { + // assert(lowbits(uint64(v)) == v, "array value out of range: %d", v) + //} opsOffset = int(offset) + len(c.array)*4 } else { c.bitmap = (*[0xFFFFFFF]uint64)(unsafe.Pointer(&data[offset]))[:bitmapN] @@ -542,8 +544,9 @@ func (b *Bitmap) UnmarshalBinary(data []byte) error { } // Verify container count on load. - count := c.count() - assert(c.count() == c.n, "container count mismatch: count=%d, n=%d", count, c.n) + // TODO: instead of commenting this out, we need to make it a configuration option + //count := c.count() + //assert(c.count() == c.n, "container count mismatch: count=%d, n=%d", count, c.n) } // Read ops log until the end of the file. @@ -1074,9 +1077,10 @@ func (c *container) arrayWriteTo(w io.Writer) (n int64, err error) { } // Verify all elements are valid. - for _, v := range c.array { - assert(lowbits(uint64(v)) == v, "cannot write array value out of range: %d", v) - } + // TODO: instead of commenting this out, we need to make it a configuration option + //for _, v := range c.array { + // assert(lowbits(uint64(v)) == v, "cannot write array value out of range: %d", v) + //} nn, err := w.Write((*[0xFFFFFFF]byte)(unsafe.Pointer(&c.array[0]))[:4*c.n]) return int64(nn), err From e3f4eab5ac05007fead5d4e7316fe75efd3d83e4 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Fri, 27 Jan 2017 18:42:47 -0500 Subject: [PATCH 02/35] removed maxslice from setbit path --- executor.go | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/executor.go b/executor.go index 3a8152ff5..f97c35c79 100644 --- a/executor.go +++ b/executor.go @@ -52,13 +52,15 @@ func (e *Executor) Execute(ctx context.Context, db string, q *pql.Query, slices // If slices aren't specified, then include all of them. if len(slices) == 0 { - // Round up the number of slices. - maxSlice := e.Index.DB(db).MaxSlice() + if needsSlices(q.Calls){ + // Round up the number of slices. +maxSlice := e.Index.DB(db).MaxSlice() - // Generate a slices of all slices. - slices = make([]uint64, maxSlice+1) - for i := range slices { - slices[i] = uint64(i) + // Generate a slices of all slices. + slices = make([]uint64, maxSlice+1) + for i := range slices { + slices[i] = uint64(i) + } } } @@ -876,3 +878,21 @@ func hasOnlySetBitmapAttrs(calls pql.Calls) bool { } return true } + +func needsSlices(calls pql.Calls) bool { + if len(calls) == 0 { + return false + } + + for _, call := range calls { + if _, ok := call.(pql.BitmapCall); !ok { + return true + }else if _, ok := call.(*pql.Count); !ok { + return true + }else if _, ok := call.(*pql.TopN); !ok { + return true + } + + } + return false +} From 7105c89398904a7a429fab1c9190eeceabb2e413 Mon Sep 17 00:00:00 2001 From: Travis Date: Fri, 27 Jan 2017 17:48:44 -0600 Subject: [PATCH 03/35] run gofmt on the previous commit --- executor.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/executor.go b/executor.go index f97c35c79..6f213bafa 100644 --- a/executor.go +++ b/executor.go @@ -52,15 +52,15 @@ func (e *Executor) Execute(ctx context.Context, db string, q *pql.Query, slices // If slices aren't specified, then include all of them. if len(slices) == 0 { - if needsSlices(q.Calls){ + if needsSlices(q.Calls) { // Round up the number of slices. -maxSlice := e.Index.DB(db).MaxSlice() + maxSlice := e.Index.DB(db).MaxSlice() - // Generate a slices of all slices. - slices = make([]uint64, maxSlice+1) - for i := range slices { - slices[i] = uint64(i) - } + // Generate a slices of all slices. + slices = make([]uint64, maxSlice+1) + for i := range slices { + slices[i] = uint64(i) + } } } @@ -880,19 +880,19 @@ func hasOnlySetBitmapAttrs(calls pql.Calls) bool { } func needsSlices(calls pql.Calls) bool { - if len(calls) == 0 { - return false - } + if len(calls) == 0 { + return false + } for _, call := range calls { if _, ok := call.(pql.BitmapCall); !ok { return true - }else if _, ok := call.(*pql.Count); !ok { + } else if _, ok := call.(*pql.Count); !ok { return true - }else if _, ok := call.(*pql.TopN); !ok { + } else if _, ok := call.(*pql.TopN); !ok { return true } } - return false + return false } From 61da4a1001355875382817cb94df3cdd0bf882db Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Wed, 1 Feb 2017 13:40:12 -0500 Subject: [PATCH 04/35] limit the number of connections to single host --- cmd/pilosa/main.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/pilosa/main.go b/cmd/pilosa/main.go index 24d2a9fec..8513a5ce3 100644 --- a/cmd/pilosa/main.go +++ b/cmd/pilosa/main.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "math/rand" + "net/http" "os" "os/signal" "path/filepath" @@ -34,6 +35,7 @@ const ( ) func main() { + http.DefaultTransport.(*http.Transport).MaxIdleConnsPerHost = 64 m := NewMain() fmt.Fprintf(m.Stderr, "Pilosa %s\n", Build) From a35dd5e5861db6d9059dffa4963b7cd673692ebd Mon Sep 17 00:00:00 2001 From: Travis Date: Wed, 1 Feb 2017 12:57:06 -0600 Subject: [PATCH 05/35] add comments to the MaxIdleConnsPerHost tweak --- cmd/pilosa/main.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/pilosa/main.go b/cmd/pilosa/main.go index 8513a5ce3..4d19ef808 100644 --- a/cmd/pilosa/main.go +++ b/cmd/pilosa/main.go @@ -35,7 +35,10 @@ const ( ) func main() { + // Limit the number of connections that a server can make to a single node + // in order to prevent a cluster storm. http.DefaultTransport.(*http.Transport).MaxIdleConnsPerHost = 64 + m := NewMain() fmt.Fprintf(m.Stderr, "Pilosa %s\n", Build) From 631b3bc44c326cc39ea5aeb026e01dc68be39257 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Wed, 1 Feb 2017 16:29:48 -0500 Subject: [PATCH 06/35] added handling for float attribute --- attr.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/attr.go b/attr.go index 1425676cf..c85f77cad 100644 --- a/attr.go +++ b/attr.go @@ -265,6 +265,8 @@ func txUpdateAttrs(tx *bolt.Tx, id uint64, m map[string]interface{}) (map[string attr[k] = uint64(v) case uint: attr[k] = uint64(v) + case float64: + attr[k] = uint64(v) case int64: attr[k] = uint64(v) case string, uint64, bool: From e62bd5fad56f98a5b154b68b7c4839ec189b524b Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Thu, 16 Feb 2017 15:43:27 -0500 Subject: [PATCH 07/35] WIP snapshot optimization --- roaring/roaring.go | 48 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index efbf48374..d714577da 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -444,17 +444,30 @@ func (b *Bitmap) removeEmptyContainers() { i++ } } +func (b *Bitmap) countEmptyContainers() int { + result := 0 + for i := 0; i < len(b.containers); { + c := b.containers[i] + + if c.n == 0 { + result++ + } + i++ + } + return result +} // WriteTo writes b to w. func (b *Bitmap) WriteTo(w io.Writer) (n int64, err error) { // Remove empty containers before persisting. - b.removeEmptyContainers() + //b.removeEmptyContainers() + containerCount := len(b.keys) - b.countEmptyContainers() // Build header before writing individual container blocks. - buf := make([]byte, headerSize+(len(b.keys)*(4+8+4))) + buf := make([]byte, headerSize+(containerCount*(4+8+4))) binary.LittleEndian.PutUint32(buf[0:], cookie) - binary.LittleEndian.PutUint32(buf[4:], uint32(len(b.keys))) - + binary.LittleEndian.PutUint32(buf[4:], uint32(containerCount)) + empty:=0 // Encode keys and cardinality. for i, key := range b.keys { c := b.containers[i] @@ -463,15 +476,24 @@ func (b *Bitmap) WriteTo(w io.Writer) (n int64, err error) { // TODO: instead of commenting this out, we need to make it a configuration option //count := c.count() //assert(c.count() == c.n, "cannot write container count, mismatch: count=%d, n=%d", count, c.n) - - binary.LittleEndian.PutUint64(buf[headerSize+i*12:], uint64(key)) - binary.LittleEndian.PutUint32(buf[headerSize+i*12+8:], uint32(c.n-1)) + if c.n > 0 { + binary.LittleEndian.PutUint64(buf[headerSize+(i-empty)*12:], uint64(key)) + binary.LittleEndian.PutUint32(buf[headerSize+(i-empty)*12+8:], uint32(c.n-1)) + }else{ + empty++ + } } // Write the offset for each container block. offset := uint32(len(buf)) + empty=0 for i, c := range b.containers { - binary.LittleEndian.PutUint32(buf[headerSize+(len(b.keys)*12)+(i*4):], uint32(offset)) + + if c.n > 0 { + binary.LittleEndian.PutUint32(buf[headerSize+(containerCount*12)+((i-empty)*4):], uint32(offset)) + }else{ + empty++ + } offset += uint32(c.size()) } @@ -484,10 +506,12 @@ func (b *Bitmap) WriteTo(w io.Writer) (n int64, err error) { // Write each container block. for _, c := range b.containers { - nn, err := c.WriteTo(w) - n += nn - if err != nil { - return n, err + if c.n > 0 { + nn, err := c.WriteTo(w) + n += nn + if err != nil { + return n, err + } } } From 9a506fa05477c4e227b739726ab6f731e63dfab7 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Thu, 16 Feb 2017 15:44:32 -0500 Subject: [PATCH 08/35] added max slice to schema --- db.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/db.go b/db.go index 2a4f1d6e0..60a018de4 100644 --- a/db.go +++ b/db.go @@ -426,8 +426,9 @@ func (p dbSlice) Less(i, j int) bool { return p[i].Name() < p[j].Name() } // DBInfo represents schema information for a database. type DBInfo struct { - Name string `json:"name"` - Frames []*FrameInfo `json:"frames"` + Name string `json:"name"` + Frames []*FrameInfo `json:"frames"` + MaxSlice uint64 } type dbInfoSlice []*DBInfo From 667d56eedf0b51e7c0e7541a17055ce50e3b7f7a Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Thu, 16 Feb 2017 15:54:47 -0500 Subject: [PATCH 09/35] added simple cache to fragment --- fragment.go | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/fragment.go b/fragment.go index aaef9323a..88b39663c 100644 --- a/fragment.go +++ b/fragment.go @@ -28,7 +28,8 @@ import ( const ( // SliceWidth is the number of profile IDs in a slice. - SliceWidth = 1048576 + //SliceWidth = 1048576 + SliceWidth = 262144 // SnapshotExt is the file extension used for an in-process snapshot. SnapshotExt = ".snapshotting" @@ -49,8 +50,25 @@ const ( const ( // DefaultFragmentMaxOpN is the default value for Fragment.MaxOpN. - DefaultFragmentMaxOpN = 1000 + //TODO CHANGING FOR TEST TO 10x + DefaultFragmentMaxOpN = 2000 ) +type BitmapCacher interface { + Fetch(id uint64)(*Bitmap,bool) + Add(id uint64, b *Bitmap) +} + + +type Simple struct{ + cache map[uint64]*Bitmap +} +func (s *Simple)Fetch(id uint64)(*Bitmap,bool){ + m,ok:=s.cache[id] + return m,ok +} +func (s *Simple)Add(id uint64,p*Bitmap){ + s.cache[id]=p +} // Fragment represents the intersection of a frame and slice in a database. type Fragment struct { @@ -87,6 +105,8 @@ type Fragment struct { BitmapAttrStore *AttrStore stats StatsClient + turbo BitmapCacher + } // NewFragment returns a new instance of Fragment. @@ -203,6 +223,7 @@ func (f *Fragment) openStorage() error { // Attach the file to the bitmap to act as a write-ahead log. f.storage.OpWriter = f.file + f.turbo = &Simple{make(map[uint64]*Bitmap)} return nil @@ -308,7 +329,12 @@ func (f *Fragment) Bitmap(bitmapID uint64) *Bitmap { return f.bitmap(bitmapID) } + func (f *Fragment) bitmap(bitmapID uint64) *Bitmap { + r,ok:=f.turbo.Fetch(bitmapID) + if ok && r != nil{ + return r + } // Only use a subset of the containers. // NOTE: The start & end ranges must be divisible by data := f.storage.OffsetRange(f.slice*SliceWidth, bitmapID*SliceWidth, (bitmapID+1)*SliceWidth) @@ -325,6 +351,7 @@ func (f *Fragment) bitmap(bitmapID uint64) *Bitmap { // Update cache. f.cache.Add(bitmapID, bm.Count()) + f.turbo.Add(bitmapID, bm) return bm } @@ -918,10 +945,15 @@ func (f *Fragment) Snapshot() error { defer f.mu.Unlock() return f.snapshot() } +func track(start time.Time, name string, logger *log.Logger) { + elapsed := time.Since(start) + logger.Printf("%s took %s", name, elapsed) +} func (f *Fragment) snapshot() error { logger := f.logger() logger.Printf("fragment: snapshotting %s/%s/%d", f.db, f.frame, f.slice) + defer track(time.Now(), fmt.Sprintf("fragment: snapshot complete %s/%s/%d", f.db, f.frame, f.slice), logger) // Create a temporary file to snapshot to. snapshotPath := f.path + SnapshotExt From 028539f10da8d27e0aa81d27e79214322c357544 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Fri, 17 Feb 2017 15:18:38 -0500 Subject: [PATCH 10/35] top bug in cache refresh --- cache.go | 16 +++++++++++----- fragment.go | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/cache.go b/cache.go index a1cba3dec..fb5f4defd 100644 --- a/cache.go +++ b/cache.go @@ -22,6 +22,7 @@ type Cache interface { // Updates the cache, if necessary. Invalidate() + Refresh() // Returns an ordered list of the top ranked bitmaps. Top() []BitmapPair @@ -86,6 +87,8 @@ func (c *LRUCache) Top() []BitmapPair { } func (c *LRUCache) onEvicted(key lru.Key, _ interface{}) { delete(c.counts, key.(uint64)) } +func (c *LRUCache) Refresh() { +} // Ensure LRUCache implements Cache. var _ Cache = &LRUCache{} @@ -117,17 +120,17 @@ func (c *RankCache) Add(bitmapID uint64, n uint64) { return } - // Add to cache. c.entries[bitmapID] = n + // If size is larger than the threshold then trim it. if len(c.entries) > c.ThresholdLength { c.update() - for id, n := range c.entries { - if n <= c.ThresholdValue { - delete(c.entries, id) + for id, n := range c.entries { + if n <= c.ThresholdValue { + delete(c.entries, id) + } } - } } } @@ -154,6 +157,9 @@ func (c *RankCache) Invalidate() { c.update() } } +func (c *RankCache) Refresh() { + c.update() +} // update reorders the entries by rank. func (c *RankCache) update() { diff --git a/fragment.go b/fragment.go index 88b39663c..51cb1dc61 100644 --- a/fragment.go +++ b/fragment.go @@ -909,7 +909,7 @@ func (f *Fragment) Import(bitmapIDs, profileIDs []uint64) error { f.cache.Add(bitmapID, f.bitmap(bitmapID).Count()) } - f.cache.Invalidate() + f.cache.Refresh() return nil }(); err != nil { _ = f.closeStorage() From d66e368843807d2307e90ac3e3c1ad17dba4f1fa Mon Sep 17 00:00:00 2001 From: Travis Date: Mon, 20 Feb 2017 18:02:24 -0600 Subject: [PATCH 11/35] handle `unionArrayBitmap` width of bitmaps correctly --- roaring/roaring.go | 1 + 1 file changed, 1 insertion(+) diff --git a/roaring/roaring.go b/roaring/roaring.go index efbf48374..c3e50a940 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1368,6 +1368,7 @@ func unionArrayBitmap(a, b *container) *container { break } else if i >= len(a.array) { output.add(vb) + continue } else if eof { output.add(a.array[i]) i++ From 43ede4d9c8efe07e05acbc557d1fa61d5abebb5c Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Tue, 21 Feb 2017 09:28:01 -0600 Subject: [PATCH 12/35] WIP increase caching and topn adjustments --- cache.go | 42 ++++++++++++++-------------- fragment.go | 69 ++++++++++++++++++++++++++++------------------ roaring/roaring.go | 13 +++++---- 3 files changed, 71 insertions(+), 53 deletions(-) diff --git a/cache.go b/cache.go index fb5f4defd..c6fc48ec6 100644 --- a/cache.go +++ b/cache.go @@ -14,6 +14,7 @@ import ( // Cache represents a cache for bitmap counts. type Cache interface { Add(bitmapID uint64, n uint64) + BulkAdd(bitmapID uint64, n uint64) Get(bitmapID uint64) uint64 Len() int @@ -22,7 +23,6 @@ type Cache interface { // Updates the cache, if necessary. Invalidate() - Refresh() // Returns an ordered list of the top ranked bitmaps. Top() []BitmapPair @@ -44,6 +44,10 @@ func NewLRUCache(maxEntries int) *LRUCache { return c } +func (c *LRUCache) BulkAdd(bitmapID, n uint64) { + c.Add(bitmapID, n) +} + // Add adds a bitmap to the cache. func (c *LRUCache) Add(bitmapID, n uint64) { c.cache.Add(bitmapID, n) @@ -87,7 +91,7 @@ func (c *LRUCache) Top() []BitmapPair { } func (c *LRUCache) onEvicted(key lru.Key, _ interface{}) { delete(c.counts, key.(uint64)) } -func (c *LRUCache) Refresh() { +func (c *LRUCache) Refresh() { } // Ensure LRUCache implements Cache. @@ -122,18 +126,26 @@ func (c *RankCache) Add(bitmapID uint64, n uint64) { c.entries[bitmapID] = n - + c.Invalidate() // If size is larger than the threshold then trim it. if len(c.entries) > c.ThresholdLength { - c.update() - for id, n := range c.entries { - if n <= c.ThresholdValue { - delete(c.entries, id) - } + for id, n := range c.entries { + if n <= c.ThresholdValue { + delete(c.entries, id) } + } } } +// Add adds a bitmap to the cache unsorted you should Invalidate after completion +func (c *RankCache) BulkAdd(bitmapID uint64, n uint64) { + if n < c.ThresholdValue { + return + } + + c.entries[bitmapID] = n +} + // Get returns a bitmap with a given id. func (c *RankCache) Get(bitmapID uint64) uint64 { return c.entries[bitmapID] } @@ -150,19 +162,9 @@ func (c *RankCache) BitmapIDs() []uint64 { return a } -// Invalidate reorders the entries, if necessary. -func (c *RankCache) Invalidate() { - // Update if there aren't many items or it hasn't been updated recently. - if len(c.rankings) < 50 || (c.updateN > 0 && time.Since(c.updateTime) > 5*time.Minute) { - c.update() - } -} -func (c *RankCache) Refresh() { - c.update() -} - // update reorders the entries by rank. -func (c *RankCache) update() { +func (c *RankCache) Invalidate() { + //fmt.Println("RankCache Update") // Convert cache to a sorted list. rankings := make([]BitmapPair, 0, len(c.entries)) for id, n := range c.entries { diff --git a/fragment.go b/fragment.go index 51cb1dc61..f16bf6ddb 100644 --- a/fragment.go +++ b/fragment.go @@ -53,21 +53,22 @@ const ( //TODO CHANGING FOR TEST TO 10x DefaultFragmentMaxOpN = 2000 ) + type BitmapCacher interface { - Fetch(id uint64)(*Bitmap,bool) + Fetch(id uint64) (*Bitmap, bool) Add(id uint64, b *Bitmap) } +type Simple struct { + cache map[uint64]*Bitmap +} -type Simple struct{ - cache map[uint64]*Bitmap +func (s *Simple) Fetch(id uint64) (*Bitmap, bool) { + m, ok := s.cache[id] + return m, ok } -func (s *Simple)Fetch(id uint64)(*Bitmap,bool){ - m,ok:=s.cache[id] - return m,ok -} -func (s *Simple)Add(id uint64,p*Bitmap){ - s.cache[id]=p +func (s *Simple) Add(id uint64, p *Bitmap) { + s.cache[id] = p } // Fragment represents the intersection of a frame and slice in a database. @@ -105,8 +106,7 @@ type Fragment struct { BitmapAttrStore *AttrStore stats StatsClient - turbo BitmapCacher - + turbo BitmapCacher } // NewFragment returns a new instance of Fragment. @@ -173,6 +173,7 @@ func (f *Fragment) Open() error { // openStorage opens the storage bitmap. func (f *Fragment) openStorage() error { + //f.logger().Printf("Open Storage %s/%s/%d", f.db, f.frame, f.slice) // Create a roaring bitmap to serve as storage for the slice. f.storage = roaring.NewBitmap() @@ -260,9 +261,11 @@ func (f *Fragment) openCache() error { // Read in all bitmaps by ID. // This will cause them to be added to the cache. for _, bitmapID := range pb.BitmapIDs { - n := f.storage.CountRange(bitmapID*SliceWidth, (bitmapID+1)*SliceWidth) - f.cache.Add(bitmapID, n) + //n := f.storage.CountRange(bitmapID*SliceWidth, (bitmapID+1)*SliceWidth) + n := f.bitmap(bitmapID, false).Count() + f.cache.BulkAdd(bitmapID, n) } + f.cache.Invalidate() return nil } @@ -326,15 +329,14 @@ func (f *Fragment) logger() *log.Logger { return log.New(f.LogOutput, "", log.Ls func (f *Fragment) Bitmap(bitmapID uint64) *Bitmap { f.mu.Lock() defer f.mu.Unlock() - return f.bitmap(bitmapID) + return f.bitmap(bitmapID, false) } - -func (f *Fragment) bitmap(bitmapID uint64) *Bitmap { - r,ok:=f.turbo.Fetch(bitmapID) - if ok && r != nil{ - return r - } +func (f *Fragment) bitmap(bitmapID uint64, updateCache bool) *Bitmap { + r, ok := f.turbo.Fetch(bitmapID) + if ok && r != nil { + return r + } // Only use a subset of the containers. // NOTE: The start & end ranges must be divisible by data := f.storage.OffsetRange(f.slice*SliceWidth, bitmapID*SliceWidth, (bitmapID+1)*SliceWidth) @@ -349,9 +351,11 @@ func (f *Fragment) bitmap(bitmapID uint64) *Bitmap { } bm.InvalidateCount() - // Update cache. - f.cache.Add(bitmapID, bm.Count()) - f.turbo.Add(bitmapID, bm) + if updateCache { + // Update cache. + f.cache.Add(bitmapID, bm.Count()) + f.turbo.Add(bitmapID, bm) + } return bm } @@ -391,7 +395,7 @@ func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, bool error) } // Update the cache. - if f.bitmap(bitmapID).SetBit(profileID) { + if f.bitmap(bitmapID, true).SetBit(profileID) { changed = true } @@ -435,7 +439,7 @@ func (f *Fragment) clearBit(bitmapID, profileID uint64) (bool, error) { } // Update the cache. - if f.bitmap(bitmapID).ClearBit(profileID) { + if f.bitmap(bitmapID, true).ClearBit(profileID) { return true, nil } @@ -570,7 +574,16 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { return results, nil } +func debugDumpPairs(pairs []BitmapPair) { + fmt.Println("=====Start") + for i, pair := range pairs { + fmt.Println(i, pair.ID, pair.Count) + } + fmt.Println("=====Stop") +} + func (f *Fragment) topBitmapPairs(bitmapIDs []uint64) []BitmapPair { + //fmt.Println("DEBUG topBitmapPairs") // If no specific bitmaps are requested, retrieve top bitmaps. if len(bitmapIDs) == 0 { f.mu.Lock() @@ -597,6 +610,8 @@ func (f *Fragment) topBitmapPairs(bitmapIDs []uint64) []BitmapPair { Count: f.Bitmap(bitmapID).Count(), } } + sort.Sort(BitmapPairs(pairs)) + //debugDumpPairs(pairs) return pairs } @@ -906,10 +921,10 @@ func (f *Fragment) Import(bitmapIDs, profileIDs []uint64) error { // Update cache counts for all bitmaps. for bitmapID := range set { - f.cache.Add(bitmapID, f.bitmap(bitmapID).Count()) + f.cache.BulkAdd(bitmapID, f.bitmap(bitmapID, false).Count()) } - f.cache.Refresh() + f.cache.Invalidate() return nil }(); err != nil { _ = f.closeStorage() diff --git a/roaring/roaring.go b/roaring/roaring.go index d714577da..58b4d1e19 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -467,7 +467,7 @@ func (b *Bitmap) WriteTo(w io.Writer) (n int64, err error) { buf := make([]byte, headerSize+(containerCount*(4+8+4))) binary.LittleEndian.PutUint32(buf[0:], cookie) binary.LittleEndian.PutUint32(buf[4:], uint32(containerCount)) - empty:=0 + empty := 0 // Encode keys and cardinality. for i, key := range b.keys { c := b.containers[i] @@ -479,20 +479,20 @@ func (b *Bitmap) WriteTo(w io.Writer) (n int64, err error) { if c.n > 0 { binary.LittleEndian.PutUint64(buf[headerSize+(i-empty)*12:], uint64(key)) binary.LittleEndian.PutUint32(buf[headerSize+(i-empty)*12+8:], uint32(c.n-1)) - }else{ + } else { empty++ } } // Write the offset for each container block. offset := uint32(len(buf)) - empty=0 + empty = 0 for i, c := range b.containers { if c.n > 0 { - binary.LittleEndian.PutUint32(buf[headerSize+(containerCount*12)+((i-empty)*4):], uint32(offset)) - }else{ - empty++ + binary.LittleEndian.PutUint32(buf[headerSize+(containerCount*12)+((i-empty)*4):], uint32(offset)) + } else { + empty++ } offset += uint32(c.size()) } @@ -1392,6 +1392,7 @@ func unionArrayBitmap(a, b *container) *container { break } else if i >= len(a.array) { output.add(vb) + continue } else if eof { output.add(a.array[i]) i++ From 774dc14412c4a7205f5448d9cd6a32d6c09dd746 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Wed, 22 Feb 2017 18:11:06 -0600 Subject: [PATCH 13/35] handle empty Union/Intersect --- executor.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/executor.go b/executor.go index 6f213bafa..bfe1cccf1 100644 --- a/executor.go +++ b/executor.go @@ -281,7 +281,7 @@ func (e *Executor) executeBitmapSlice(ctx context.Context, db string, c *pql.Bit // executeIntersectSlice executes a intersect() call for a local slice. func (e *Executor) executeIntersectSlice(ctx context.Context, db string, c *pql.Intersect, slice uint64) (*Bitmap, error) { - var other *Bitmap + other := &Bitmap{} for i, input := range c.Inputs { bm, err := e.executeBitmapCallSlice(ctx, db, input, slice) if err != nil { @@ -331,7 +331,7 @@ func (e *Executor) executeRangeSlice(ctx context.Context, db string, c *pql.Rang // executeUnionSlice executes a union() call for a local slice. func (e *Executor) executeUnionSlice(ctx context.Context, db string, c *pql.Union, slice uint64) (*Bitmap, error) { - var other *Bitmap + other := &Bitmap{} for i, input := range c.Inputs { bm, err := e.executeBitmapCallSlice(ctx, db, input, slice) if err != nil { From ff52ffa0473a02e68433d11c7f7ac038886d7a6f Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Thu, 23 Feb 2017 15:08:18 -0600 Subject: [PATCH 14/35] WIP TopN optimization --- cache.go | 18 ++++++++++++++++++ fragment.go | 47 +++++++++++++++++++++-------------------------- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/cache.go b/cache.go index c6fc48ec6..2f4761872 100644 --- a/cache.go +++ b/cache.go @@ -241,6 +241,24 @@ func (p Pairs) Swap(i, j int) { p[i], p[j] = p[j], p[i] } func (p Pairs) Len() int { return len(p) } func (p Pairs) Less(i, j int) bool { return p[i].Count > p[j].Count } +type PairHeap struct { + Pairs +} + +func (h *Pairs) Push(x interface{}) { + // Push and Pop use pointer receivers because they modify the slice's length, + // not just its contents. + *h = append(*h, x.(Pair)) +} + +func (h *Pairs) Pop() interface{} { + old := *h + n := len(old) + x := old[n-1] + *h = old[0 : n-1] + return x +} + // Add merges other into p and returns a new slice. func (p Pairs) Add(other []Pair) []Pair { // Create lookup of key/counts. diff --git a/fragment.go b/fragment.go index f16bf6ddb..e7d6086d1 100644 --- a/fragment.go +++ b/fragment.go @@ -4,6 +4,7 @@ import ( "archive/tar" "bufio" "bytes" + "container/heap" "context" "crypto/sha1" "encoding/binary" @@ -28,7 +29,7 @@ import ( const ( // SliceWidth is the number of profile IDs in a slice. - //SliceWidth = 1048576 + // SliceWidth = 1048576 SliceWidth = 262144 // SnapshotExt is the file extension used for an in-process snapshot. @@ -173,7 +174,6 @@ func (f *Fragment) Open() error { // openStorage opens the storage bitmap. func (f *Fragment) openStorage() error { - //f.logger().Printf("Open Storage %s/%s/%d", f.db, f.frame, f.slice) // Create a roaring bitmap to serve as storage for the slice. f.storage = roaring.NewBitmap() @@ -494,7 +494,8 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { } // Iterate over rankings and add to results until we have enough. - results := make([]Pair, 0, opt.N) + //results := make(PairHeap, 0, opt.N) + results := &PairHeap{} for _, pair := range pairs { bitmapID, n := pair.ID, pair.Count @@ -518,7 +519,7 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { } // The initial n pairs should simply be added to the results. - if opt.N == 0 || len(results) < opt.N { + if opt.N == 0 || results.Len() < opt.N { // Calculate count and append. count := n if opt.Src != nil { @@ -527,23 +528,26 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { if count == 0 { continue } - results = append(results, Pair{Key: bitmapID, Count: count}) + //results = append(results, Pair{Key: bitmapID, Count: count}) + heap.Push(results, Pair{Key: bitmapID, Count: count}) // If we reach the requested number of pairs and we are not computing // intersections then simply exit. If we are intersecting then sort // and then only keep pairs that are higher than the lowest count. - if opt.N > 0 && len(results) == opt.N { + if opt.N > 0 && results.Len() == opt.N { if opt.Src == nil { break } - sort.Sort(Pairs(results)) + // sort.Sort(Pairs(results)) } continue } // Retrieve the lowest count we have. // If it's too low then don't try finding anymore pairs. - threshold := results[len(results)-1].Count + //threshold := results[len(results)-1].Count + + threshold := results.Pairs[0].Count if threshold < MinThreshold { break } @@ -556,34 +560,25 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { // Calculate the intersecting bit count and skip if it's below our // last bitmap in our current result set. + count := opt.Src.IntersectionCount(f.Bitmap(bitmapID)) if count < threshold { continue } - // Swap out the last pair for this new count. - results[len(results)-1] = Pair{Key: bitmapID, Count: count} - - // If it's count is also higher than the second to last item then resort. - if len(results) >= 2 && count > results[len(results)-2].Count { - sort.Sort(Pairs(results)) - } + heap.Push(results, Pair{Key: bitmapID, Count: count}) } - - sort.Sort(Pairs(results)) - return results, nil -} - -func debugDumpPairs(pairs []BitmapPair) { - fmt.Println("=====Start") - for i, pair := range pairs { - fmt.Println(i, pair.ID, pair.Count) + r := make(Pairs, results.Len(), results.Len()) + x := results.Len() + i := 1 + for results.Len() > 0 { + r[x-i] = heap.Pop(results).(Pair) + i++ } - fmt.Println("=====Stop") + return r, nil } func (f *Fragment) topBitmapPairs(bitmapIDs []uint64) []BitmapPair { - //fmt.Println("DEBUG topBitmapPairs") // If no specific bitmaps are requested, retrieve top bitmaps. if len(bitmapIDs) == 0 { f.mu.Lock() From b4d6181285e8808511361694343f26082b72cdda Mon Sep 17 00:00:00 2001 From: Travis Date: Fri, 24 Feb 2017 12:57:41 -0600 Subject: [PATCH 15/35] no longer need to implement `Refresh()` since that was removed` --- cache.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cache.go b/cache.go index 2f4761872..af2284fc7 100644 --- a/cache.go +++ b/cache.go @@ -91,8 +91,6 @@ func (c *LRUCache) Top() []BitmapPair { } func (c *LRUCache) onEvicted(key lru.Key, _ interface{}) { delete(c.counts, key.(uint64)) } -func (c *LRUCache) Refresh() { -} // Ensure LRUCache implements Cache. var _ Cache = &LRUCache{} From 846b528c529565b33a81faddaf4d0f8a9d530df9 Mon Sep 17 00:00:00 2001 From: Travis Date: Fri, 24 Feb 2017 13:18:29 -0600 Subject: [PATCH 16/35] change the names of `Simple`, `turbo` to `SimpleCache`, `bitmapCache` --- fragment.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/fragment.go b/fragment.go index e7d6086d1..d6eead246 100644 --- a/fragment.go +++ b/fragment.go @@ -29,8 +29,7 @@ import ( const ( // SliceWidth is the number of profile IDs in a slice. - // SliceWidth = 1048576 - SliceWidth = 262144 + SliceWidth = 1048576 // SnapshotExt is the file extension used for an in-process snapshot. SnapshotExt = ".snapshotting" @@ -51,24 +50,28 @@ const ( const ( // DefaultFragmentMaxOpN is the default value for Fragment.MaxOpN. - //TODO CHANGING FOR TEST TO 10x DefaultFragmentMaxOpN = 2000 ) +// BitmapCacher implements SimpleCache +// it is meant to be a short-lived cache for cases where writes are continuing to access +// the same bit withing a short time frame (i.e. good for write-heavy loads) +// A read-heavy use case would cause the cache to get bigger, potentially causing the +// node to run out of memory. type BitmapCacher interface { Fetch(id uint64) (*Bitmap, bool) Add(id uint64, b *Bitmap) } -type Simple struct { +type SimpleCache struct { cache map[uint64]*Bitmap } -func (s *Simple) Fetch(id uint64) (*Bitmap, bool) { +func (s *SimpleCache) Fetch(id uint64) (*Bitmap, bool) { m, ok := s.cache[id] return m, ok } -func (s *Simple) Add(id uint64, p *Bitmap) { +func (s *SimpleCache) Add(id uint64, p *Bitmap) { s.cache[id] = p } @@ -106,8 +109,8 @@ type Fragment struct { // This is set by the parent frame unless overridden for testing. BitmapAttrStore *AttrStore - stats StatsClient - turbo BitmapCacher + stats StatsClient + bitmapCache BitmapCacher } // NewFragment returns a new instance of Fragment. @@ -224,7 +227,7 @@ func (f *Fragment) openStorage() error { // Attach the file to the bitmap to act as a write-ahead log. f.storage.OpWriter = f.file - f.turbo = &Simple{make(map[uint64]*Bitmap)} + f.bitmapCache = &SimpleCache{make(map[uint64]*Bitmap)} return nil @@ -333,7 +336,7 @@ func (f *Fragment) Bitmap(bitmapID uint64) *Bitmap { } func (f *Fragment) bitmap(bitmapID uint64, updateCache bool) *Bitmap { - r, ok := f.turbo.Fetch(bitmapID) + r, ok := f.bitmapCache.Fetch(bitmapID) if ok && r != nil { return r } @@ -354,7 +357,7 @@ func (f *Fragment) bitmap(bitmapID uint64, updateCache bool) *Bitmap { if updateCache { // Update cache. f.cache.Add(bitmapID, bm.Count()) - f.turbo.Add(bitmapID, bm) + f.bitmapCache.Add(bitmapID, bm) } return bm From de1a779d41b042bdfe00c1d8896fa60ddd614de1 Mon Sep 17 00:00:00 2001 From: Travis Date: Fri, 24 Feb 2017 13:20:04 -0600 Subject: [PATCH 17/35] remove unused `MaxSlice` from `DB` --- db.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/db.go b/db.go index 60a018de4..2a4f1d6e0 100644 --- a/db.go +++ b/db.go @@ -426,9 +426,8 @@ func (p dbSlice) Less(i, j int) bool { return p[i].Name() < p[j].Name() } // DBInfo represents schema information for a database. type DBInfo struct { - Name string `json:"name"` - Frames []*FrameInfo `json:"frames"` - MaxSlice uint64 + Name string `json:"name"` + Frames []*FrameInfo `json:"frames"` } type dbInfoSlice []*DBInfo From b6907c4e19558899654326fb5ea7e37c397fa7d3 Mon Sep 17 00:00:00 2001 From: Travis Date: Fri, 24 Feb 2017 13:29:14 -0600 Subject: [PATCH 18/35] add TODO to make `MaxIdleConnsPerHost` configurable --- cmd/pilosa/main.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/pilosa/main.go b/cmd/pilosa/main.go index 6fff5db6b..82eb9d7a7 100644 --- a/cmd/pilosa/main.go +++ b/cmd/pilosa/main.go @@ -43,6 +43,7 @@ const ( func main() { // Limit the number of connections that a server can make to a single node // in order to prevent a cluster storm. + // TODO: make this configurable http.DefaultTransport.(*http.Transport).MaxIdleConnsPerHost = 64 m := NewMain() From 22e9c1cafaa85582bb040df77ecf5dbf2498da8c Mon Sep 17 00:00:00 2001 From: Travis Date: Mon, 27 Feb 2017 11:04:37 -0600 Subject: [PATCH 19/35] modify `needsSlices()` to use `call.Name` --- executor.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/executor.go b/executor.go index 59e5fab5e..5f6b766c9 100644 --- a/executor.go +++ b/executor.go @@ -995,20 +995,20 @@ func hasOnlySetBitmapAttrs(calls []*pql.Call) bool { return true } -func needsSlices(calls pql.Calls) bool { +func needsSlices(calls []*pql.Call) bool { if len(calls) == 0 { return false } - for _, call := range calls { - if _, ok := call.(pql.BitmapCall); !ok { + switch call.Name { + case "ClearBit", "Profile", "SetBit", "SetBitmapAttrs", "SetProfileAttrs": + continue + case "Count", "TopN": return true - } else if _, ok := call.(*pql.Count); !ok { - return true - } else if _, ok := call.(*pql.TopN); !ok { + // default catches Bitmap calls + default: return true } - } return false } From c36828863856b5b80161a1d4358dabfa04ef217d Mon Sep 17 00:00:00 2001 From: Travis Date: Mon, 27 Feb 2017 11:54:59 -0600 Subject: [PATCH 20/35] fix `BulkAdd` comment --- cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cache.go b/cache.go index af2284fc7..86cab6b5a 100644 --- a/cache.go +++ b/cache.go @@ -135,7 +135,7 @@ func (c *RankCache) Add(bitmapID uint64, n uint64) { } } -// Add adds a bitmap to the cache unsorted you should Invalidate after completion +// BulkAdd adds a bitmap to the cache unsorted. You should Invalidate after completion. func (c *RankCache) BulkAdd(bitmapID uint64, n uint64) { if n < c.ThresholdValue { return From ab52a803b849bc43efd13d3afc9304314a4eb64a Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Mon, 27 Feb 2017 12:42:36 -0600 Subject: [PATCH 21/35] rank cache update after count --- fragment.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/fragment.go b/fragment.go index d6eead246..8337dbb96 100644 --- a/fragment.go +++ b/fragment.go @@ -398,9 +398,10 @@ func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, bool error) } // Update the cache. - if f.bitmap(bitmapID, true).SetBit(profileID) { - changed = true - } + bm := f.bitmap(bitmapID, true) + bm.SetBit(profileID) + bm.InvalidateCount() //maybe a perf opportunity? + f.cache.Add(bitmapID, bm.Count()) f.stats.Count("setN", 1) @@ -442,9 +443,10 @@ func (f *Fragment) clearBit(bitmapID, profileID uint64) (bool, error) { } // Update the cache. - if f.bitmap(bitmapID, true).ClearBit(profileID) { - return true, nil - } + bm := f.bitmap(bitmapID, true) + bm.ClearBit(profileID) + bm.InvalidateCount() //maybe a perf opportunity? + f.cache.Add(bitmapID, bm.Count()) f.stats.Count("clearN", 1) From fd800e130cc7c82cd603c8bfe0a21e33a7e13638 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Mon, 27 Feb 2017 12:43:31 -0600 Subject: [PATCH 22/35] heap sort order backwards --- cache.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cache.go b/cache.go index 86cab6b5a..8eab72368 100644 --- a/cache.go +++ b/cache.go @@ -243,6 +243,8 @@ type PairHeap struct { Pairs } +func (p PairHeap) Less(i, j int) bool { return p.Pairs[i].Count < p.Pairs[j].Count } + func (h *Pairs) Push(x interface{}) { // Push and Pop use pointer receivers because they modify the slice's length, // not just its contents. From 3b309d0d5140f12ada16a1a37bd8f39a2f35482b Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Mon, 27 Feb 2017 18:25:45 -0600 Subject: [PATCH 23/35] WIP TopN accuracy --- executor.go | 13 ++++++++++--- fragment.go | 25 ++++++++++++++----------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/executor.go b/executor.go index 5f6b766c9..c681495f7 100644 --- a/executor.go +++ b/executor.go @@ -168,6 +168,7 @@ func (e *Executor) executeBitmapCallSlice(ctx context.Context, db string, c *pql // requeries to retrieve the full counts for each of the top results. func (e *Executor) executeTopN(ctx context.Context, db string, c *pql.Call, slices []uint64, opt *ExecOptions) ([]Pair, error) { bitmapIDs, _ := c.Args["ids"].([]uint64) + n := c.Args["n"].(uint64) // Execute original query. pairs, err := e.executeTopNSlices(ctx, db, c, slices, opt) @@ -180,16 +181,21 @@ func (e *Executor) executeTopN(ctx context.Context, db string, c *pql.Call, slic if len(pairs) == 0 || len(bitmapIDs) > 0 || opt.Remote { return pairs, nil } - // Only the original caller should refetch the full counts. other := c.Clone() - other.Args["n"] = 0 + //other.Args["n"] = 0 + other.Args["n"] = len(bitmapIDs) * 2 ids := Pairs(pairs).Keys() sort.Sort(uint64Slice(ids)) other.Args["ids"] = ids - return e.executeTopNSlices(ctx, db, other, slices, opt) + trimedlist, x := e.executeTopNSlices(ctx, db, other, slices, opt) + if x != nil { + return nil, x + } + trimedlist = trimedlist[0:n] + return trimedlist, nil } func (e *Executor) executeTopNSlices(ctx context.Context, db string, c *pql.Call, slices []uint64, opt *ExecOptions) ([]Pair, error) { @@ -896,6 +902,7 @@ func (e *Executor) mapper(ctx context.Context, ch chan mapResponse, nodes []*Nod if n.Host == e.Host { resp.result, resp.err = e.mapperLocal(ctx, nodeSlices, mapFn, reduceFn) } else if !opt.Remote { + results, err := e.exec(ctx, n, db, &pql.Query{Calls: []*pql.Call{c}}, nodeSlices, opt) if len(results) > 0 { resp.result = results[0] diff --git a/fragment.go b/fragment.go index 8337dbb96..493640ef0 100644 --- a/fragment.go +++ b/fragment.go @@ -553,9 +553,9 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { //threshold := results[len(results)-1].Count threshold := results.Pairs[0].Count - if threshold < MinThreshold { - break - } + //if threshold < MinThreshold { + //break + //} // If the bitmap doesn't have enough bits set before the intersection // then we can assume that any remaing bitmaps also have a count too low. @@ -593,21 +593,24 @@ func (f *Fragment) topBitmapPairs(bitmapIDs []uint64) []BitmapPair { } // Otherwise retrieve specific bitmaps. - pairs := make([]BitmapPair, len(bitmapIDs)) - for i, bitmapID := range bitmapIDs { + pairs := make([]BitmapPair, 0, len(bitmapIDs)) + for _, bitmapID := range bitmapIDs { // Look up cache first, if available. if n := f.cache.Get(bitmapID); n > 0 { - pairs[i] = BitmapPair{ + pairs = append(pairs, BitmapPair{ ID: bitmapID, Count: n, - } + }) continue } - // Otherwise load from storage. - pairs[i] = BitmapPair{ - ID: bitmapID, - Count: f.Bitmap(bitmapID).Count(), + bm := f.Bitmap(bitmapID) + if bm.Count() > 0 { + // Otherwise load from storage. + pairs = append(pairs, BitmapPair{ + ID: bitmapID, + Count: bm.Count(), + }) } } sort.Sort(BitmapPairs(pairs)) From 5ef4d6fa6bd156a37576e228e3784e2b9153bb0f Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Mon, 27 Feb 2017 19:34:33 -0600 Subject: [PATCH 24/35] adjusted first phase topn to collect all slices id's --- executor.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/executor.go b/executor.go index c681495f7..72df37c80 100644 --- a/executor.go +++ b/executor.go @@ -199,7 +199,7 @@ func (e *Executor) executeTopN(ctx context.Context, db string, c *pql.Call, slic } func (e *Executor) executeTopNSlices(ctx context.Context, db string, c *pql.Call, slices []uint64, opt *ExecOptions) ([]Pair, error) { - n, _ := c.Args["n"].(uint64) + //n, _ := c.Args["n"].(uint64) // Execute calls in bulk on each remote node and merge. mapFn := func(slice uint64) (interface{}, error) { @@ -222,9 +222,9 @@ func (e *Executor) executeTopNSlices(ctx context.Context, db string, c *pql.Call sort.Sort(Pairs(results)) // Only keep the top n after sorting. - if n > 0 && len(results) > int(n) { - results = results[0:n] - } + // if n > 0 && len(results) > int(n) { + // results = results[0:n] + //} return results, nil } From 62a59f2bd1ba760885ed08e1339432fbb676aefe Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Tue, 28 Feb 2017 14:01:30 -0600 Subject: [PATCH 25/35] incorrect handling of large topns --- executor.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/executor.go b/executor.go index 72df37c80..f1f1adb3c 100644 --- a/executor.go +++ b/executor.go @@ -194,7 +194,10 @@ func (e *Executor) executeTopN(ctx context.Context, db string, c *pql.Call, slic if x != nil { return nil, x } - trimedlist = trimedlist[0:n] + + if int(n) < len(trimedlist) { + trimedlist = trimedlist[0:n] + } return trimedlist, nil } From 527ff5682f8175d0d46c92dbad8c3cfde5e78591 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Wed, 1 Mar 2017 15:14:00 -0600 Subject: [PATCH 26/35] cache performance enhancement --- bitmap.go | 23 ++++++++++++++++++++++- cache.go | 47 ++++++++++++++++++++++++++++++++++++----------- fragment.go | 22 +++++++++++++++++----- 3 files changed, 75 insertions(+), 17 deletions(-) diff --git a/bitmap.go b/bitmap.go index 55664b24b..d30a9192a 100644 --- a/bitmap.go +++ b/bitmap.go @@ -16,6 +16,8 @@ type Bitmap struct { // Attributes associated with the bitmap. Attrs map[string]interface{} + + cacheoveride uint64 } // NewBitmap returns a new instance of Bitmap. @@ -174,6 +176,26 @@ func (b *Bitmap) InvalidateCount() { } } +//increment the bitmap cached counter, note this is an optimization that assumes that the caller is aware the size increased +func (b *Bitmap) IncrementCount(i uint64) { + seg := b.segment(i / SliceWidth) + if seg != nil { + seg.n++ + } +} +func (b *Bitmap) DecrementCount(i uint64) { + seg := b.segment(i / SliceWidth) + if seg != nil { + if seg.n > 0 { + seg.n-- + } + } +} +func (b *Bitmap) SetCount(i uint64, count uint64) { + seg := b.segment(i / SliceWidth) + seg.n = count +} + // Count returns the number of set bits in the bitmap. func (b *Bitmap) Count() uint64 { var n uint64 @@ -312,7 +334,6 @@ func (s *BitmapSegment) Difference(other *BitmapSegment) *BitmapSegment { // SetBit sets the i-th bit of the bitmap. func (s *BitmapSegment) SetBit(i uint64) (changed bool) { s.ensureWritable() - changed, _ = s.data.Add(i) if changed { s.n++ diff --git a/cache.go b/cache.go index 8eab72368..4345c93fd 100644 --- a/cache.go +++ b/cache.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "sort" + "sync" "time" "github.com/golang/groupcache/lru" @@ -97,6 +98,7 @@ var _ Cache = &LRUCache{} // RankCache represents a cache with sorted entries. type RankCache struct { + mu sync.Mutex entries map[uint64]uint64 rankings []BitmapPair // cached, ordered list @@ -117,6 +119,8 @@ func NewRankCache() *RankCache { // Add adds a bitmap to the cache. func (c *RankCache) Add(bitmapID uint64, n uint64) { + c.mu.Lock() + defer c.mu.Unlock() // Ignore if the bit count on the bitmap is below the threshold. if n < c.ThresholdValue { return @@ -124,19 +128,13 @@ func (c *RankCache) Add(bitmapID uint64, n uint64) { c.entries[bitmapID] = n - c.Invalidate() - // If size is larger than the threshold then trim it. - if len(c.entries) > c.ThresholdLength { - for id, n := range c.entries { - if n <= c.ThresholdValue { - delete(c.entries, id) - } - } - } + c.invalidate() } // BulkAdd adds a bitmap to the cache unsorted. You should Invalidate after completion. func (c *RankCache) BulkAdd(bitmapID uint64, n uint64) { + c.mu.Lock() + defer c.mu.Unlock() if n < c.ThresholdValue { return } @@ -145,13 +143,23 @@ func (c *RankCache) BulkAdd(bitmapID uint64, n uint64) { } // Get returns a bitmap with a given id. -func (c *RankCache) Get(bitmapID uint64) uint64 { return c.entries[bitmapID] } +func (c *RankCache) Get(bitmapID uint64) uint64 { + c.mu.Lock() + defer c.mu.Unlock() + return c.entries[bitmapID] +} // Len returns the number of items in the cache. -func (c *RankCache) Len() int { return len(c.entries) } +func (c *RankCache) Len() int { + c.mu.Lock() + defer c.mu.Unlock() + return len(c.entries) +} // BitmapIDs returns a list of all bitmap IDs in the cache. func (c *RankCache) BitmapIDs() []uint64 { + c.mu.Lock() + defer c.mu.Unlock() a := make([]uint64, 0, len(c.entries)) for id := range c.entries { a = append(a, id) @@ -162,6 +170,15 @@ func (c *RankCache) BitmapIDs() []uint64 { // update reorders the entries by rank. func (c *RankCache) Invalidate() { + c.mu.Lock() + defer c.mu.Unlock() + c.invalidate() + +} +func (c *RankCache) invalidate() { + if time.Now().Sub(c.updateTime).Seconds() < 10 { + return + } //fmt.Println("RankCache Update") // Convert cache to a sorted list. rankings := make([]BitmapPair, 0, len(c.entries)) @@ -183,6 +200,14 @@ func (c *RankCache) Invalidate() { // Reset counters. c.updateTime, c.updateN = time.Now(), 0 + // If size is larger than the threshold then trim it. + if len(c.entries) > c.ThresholdLength { + for id, n := range c.entries { + if n <= c.ThresholdValue { + delete(c.entries, id) + } + } + } } // Top returns an ordered list of bitmaps. diff --git a/fragment.go b/fragment.go index 493640ef0..5516e05ec 100644 --- a/fragment.go +++ b/fragment.go @@ -351,8 +351,10 @@ func (f *Fragment) bitmap(bitmapID uint64, updateCache bool) *Bitmap { slice: f.slice, writable: false, }}, + cacheoveride: 0, } bm.InvalidateCount() + bm.cacheoveride = bm.Count() if updateCache { // Update cache. @@ -380,6 +382,7 @@ func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, bool error) } // Write to storage. + if changed, err = f.storage.Add(pos); err != nil { return false, err } @@ -399,8 +402,13 @@ func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, bool error) // Update the cache. bm := f.bitmap(bitmapID, true) - bm.SetBit(profileID) - bm.InvalidateCount() //maybe a perf opportunity? + if bm.cacheoveride > 0 { + bm.SetCount(profileID, bm.cacheoveride) + bm.cacheoveride = 0 + } else { + bm.IncrementCount(profileID) + } + f.cache.Add(bitmapID, bm.Count()) f.stats.Count("setN", 1) @@ -443,9 +451,13 @@ func (f *Fragment) clearBit(bitmapID, profileID uint64) (bool, error) { } // Update the cache. - bm := f.bitmap(bitmapID, true) - bm.ClearBit(profileID) - bm.InvalidateCount() //maybe a perf opportunity? + bm := f.bitmap(bitmapID, false) + if bm.cacheoveride > 0 { + bm.SetCount(profileID, bm.cacheoveride) + bm.cacheoveride = 0 + } else { + bm.DecrementCount(profileID) + } f.cache.Add(bitmapID, bm.Count()) f.stats.Count("clearN", 1) From 26041563fa6985d616c7f184e9afd343d8fb6da2 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Thu, 2 Mar 2017 10:36:35 -0600 Subject: [PATCH 27/35] fix for failed test TestMain_FrameRestore --- fragment.go | 1 + 1 file changed, 1 insertion(+) diff --git a/fragment.go b/fragment.go index 5516e05ec..ac4d63926 100644 --- a/fragment.go +++ b/fragment.go @@ -408,6 +408,7 @@ func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, bool error) } else { bm.IncrementCount(profileID) } + bm.SetBit(profileID) f.cache.Add(bitmapID, bm.Count()) From 9237e7b6995618af7f6aa818707e88ab4208038b Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Mon, 27 Feb 2017 12:42:36 -0600 Subject: [PATCH 28/35] rank cache update after count heap sort order backwards WIP TopN accuracy adjusted first phase topn to collect all slices id's incorrect handling of large topns cache performance enhancement fix for failed test TestMain_FrameRestore remove unused code and fix some variable names --- bitmap.go | 23 ++++++++++++++++++++++- cache.go | 49 ++++++++++++++++++++++++++++++++++++++----------- executor.go | 24 ++++++++++++++---------- fragment.go | 47 ++++++++++++++++++++++++++++++----------------- 4 files changed, 104 insertions(+), 39 deletions(-) diff --git a/bitmap.go b/bitmap.go index 55664b24b..d30a9192a 100644 --- a/bitmap.go +++ b/bitmap.go @@ -16,6 +16,8 @@ type Bitmap struct { // Attributes associated with the bitmap. Attrs map[string]interface{} + + cacheoveride uint64 } // NewBitmap returns a new instance of Bitmap. @@ -174,6 +176,26 @@ func (b *Bitmap) InvalidateCount() { } } +//increment the bitmap cached counter, note this is an optimization that assumes that the caller is aware the size increased +func (b *Bitmap) IncrementCount(i uint64) { + seg := b.segment(i / SliceWidth) + if seg != nil { + seg.n++ + } +} +func (b *Bitmap) DecrementCount(i uint64) { + seg := b.segment(i / SliceWidth) + if seg != nil { + if seg.n > 0 { + seg.n-- + } + } +} +func (b *Bitmap) SetCount(i uint64, count uint64) { + seg := b.segment(i / SliceWidth) + seg.n = count +} + // Count returns the number of set bits in the bitmap. func (b *Bitmap) Count() uint64 { var n uint64 @@ -312,7 +334,6 @@ func (s *BitmapSegment) Difference(other *BitmapSegment) *BitmapSegment { // SetBit sets the i-th bit of the bitmap. func (s *BitmapSegment) SetBit(i uint64) (changed bool) { s.ensureWritable() - changed, _ = s.data.Add(i) if changed { s.n++ diff --git a/cache.go b/cache.go index 86cab6b5a..4345c93fd 100644 --- a/cache.go +++ b/cache.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "sort" + "sync" "time" "github.com/golang/groupcache/lru" @@ -97,6 +98,7 @@ var _ Cache = &LRUCache{} // RankCache represents a cache with sorted entries. type RankCache struct { + mu sync.Mutex entries map[uint64]uint64 rankings []BitmapPair // cached, ordered list @@ -117,6 +119,8 @@ func NewRankCache() *RankCache { // Add adds a bitmap to the cache. func (c *RankCache) Add(bitmapID uint64, n uint64) { + c.mu.Lock() + defer c.mu.Unlock() // Ignore if the bit count on the bitmap is below the threshold. if n < c.ThresholdValue { return @@ -124,19 +128,13 @@ func (c *RankCache) Add(bitmapID uint64, n uint64) { c.entries[bitmapID] = n - c.Invalidate() - // If size is larger than the threshold then trim it. - if len(c.entries) > c.ThresholdLength { - for id, n := range c.entries { - if n <= c.ThresholdValue { - delete(c.entries, id) - } - } - } + c.invalidate() } // BulkAdd adds a bitmap to the cache unsorted. You should Invalidate after completion. func (c *RankCache) BulkAdd(bitmapID uint64, n uint64) { + c.mu.Lock() + defer c.mu.Unlock() if n < c.ThresholdValue { return } @@ -145,13 +143,23 @@ func (c *RankCache) BulkAdd(bitmapID uint64, n uint64) { } // Get returns a bitmap with a given id. -func (c *RankCache) Get(bitmapID uint64) uint64 { return c.entries[bitmapID] } +func (c *RankCache) Get(bitmapID uint64) uint64 { + c.mu.Lock() + defer c.mu.Unlock() + return c.entries[bitmapID] +} // Len returns the number of items in the cache. -func (c *RankCache) Len() int { return len(c.entries) } +func (c *RankCache) Len() int { + c.mu.Lock() + defer c.mu.Unlock() + return len(c.entries) +} // BitmapIDs returns a list of all bitmap IDs in the cache. func (c *RankCache) BitmapIDs() []uint64 { + c.mu.Lock() + defer c.mu.Unlock() a := make([]uint64, 0, len(c.entries)) for id := range c.entries { a = append(a, id) @@ -162,6 +170,15 @@ func (c *RankCache) BitmapIDs() []uint64 { // update reorders the entries by rank. func (c *RankCache) Invalidate() { + c.mu.Lock() + defer c.mu.Unlock() + c.invalidate() + +} +func (c *RankCache) invalidate() { + if time.Now().Sub(c.updateTime).Seconds() < 10 { + return + } //fmt.Println("RankCache Update") // Convert cache to a sorted list. rankings := make([]BitmapPair, 0, len(c.entries)) @@ -183,6 +200,14 @@ func (c *RankCache) Invalidate() { // Reset counters. c.updateTime, c.updateN = time.Now(), 0 + // If size is larger than the threshold then trim it. + if len(c.entries) > c.ThresholdLength { + for id, n := range c.entries { + if n <= c.ThresholdValue { + delete(c.entries, id) + } + } + } } // Top returns an ordered list of bitmaps. @@ -243,6 +268,8 @@ type PairHeap struct { Pairs } +func (p PairHeap) Less(i, j int) bool { return p.Pairs[i].Count < p.Pairs[j].Count } + func (h *Pairs) Push(x interface{}) { // Push and Pop use pointer receivers because they modify the slice's length, // not just its contents. diff --git a/executor.go b/executor.go index 5f6b766c9..4a2d954c9 100644 --- a/executor.go +++ b/executor.go @@ -168,6 +168,7 @@ func (e *Executor) executeBitmapCallSlice(ctx context.Context, db string, c *pql // requeries to retrieve the full counts for each of the top results. func (e *Executor) executeTopN(ctx context.Context, db string, c *pql.Call, slices []uint64, opt *ExecOptions) ([]Pair, error) { bitmapIDs, _ := c.Args["ids"].([]uint64) + n := c.Args["n"].(uint64) // Execute original query. pairs, err := e.executeTopNSlices(ctx, db, c, slices, opt) @@ -180,21 +181,28 @@ func (e *Executor) executeTopN(ctx context.Context, db string, c *pql.Call, slic if len(pairs) == 0 || len(bitmapIDs) > 0 || opt.Remote { return pairs, nil } - // Only the original caller should refetch the full counts. other := c.Clone() - other.Args["n"] = 0 + + // Double the size of n for other calls in order to... + other.Args["n"] = len(bitmapIDs) * 2 ids := Pairs(pairs).Keys() sort.Sort(uint64Slice(ids)) other.Args["ids"] = ids - return e.executeTopNSlices(ctx, db, other, slices, opt) + trimmedList, err := e.executeTopNSlices(ctx, db, other, slices, opt) + if err != nil { + return nil, err + } + + if int(n) < len(trimmedList) { + trimmedList = trimmedList[0:n] + } + return trimmedList, nil } func (e *Executor) executeTopNSlices(ctx context.Context, db string, c *pql.Call, slices []uint64, opt *ExecOptions) ([]Pair, error) { - n, _ := c.Args["n"].(uint64) - // Execute calls in bulk on each remote node and merge. mapFn := func(slice uint64) (interface{}, error) { return e.executeTopNSlice(ctx, db, c, slice) @@ -215,11 +223,6 @@ func (e *Executor) executeTopNSlices(ctx context.Context, db string, c *pql.Call // Sort final merged results. sort.Sort(Pairs(results)) - // Only keep the top n after sorting. - if n > 0 && len(results) > int(n) { - results = results[0:n] - } - return results, nil } @@ -896,6 +899,7 @@ func (e *Executor) mapper(ctx context.Context, ch chan mapResponse, nodes []*Nod if n.Host == e.Host { resp.result, resp.err = e.mapperLocal(ctx, nodeSlices, mapFn, reduceFn) } else if !opt.Remote { + results, err := e.exec(ctx, n, db, &pql.Query{Calls: []*pql.Call{c}}, nodeSlices, opt) if len(results) > 0 { resp.result = results[0] diff --git a/fragment.go b/fragment.go index d6eead246..e203dd711 100644 --- a/fragment.go +++ b/fragment.go @@ -351,8 +351,10 @@ func (f *Fragment) bitmap(bitmapID uint64, updateCache bool) *Bitmap { slice: f.slice, writable: false, }}, + cacheoveride: 0, } bm.InvalidateCount() + bm.cacheoveride = bm.Count() if updateCache { // Update cache. @@ -380,6 +382,7 @@ func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, bool error) } // Write to storage. + if changed, err = f.storage.Add(pos); err != nil { return false, err } @@ -398,9 +401,16 @@ func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, bool error) } // Update the cache. - if f.bitmap(bitmapID, true).SetBit(profileID) { - changed = true + bm := f.bitmap(bitmapID, true) + if bm.cacheoveride > 0 { + bm.SetCount(profileID, bm.cacheoveride) + bm.cacheoveride = 0 + } else { + bm.IncrementCount(profileID) } + bm.SetBit(profileID) + + f.cache.Add(bitmapID, bm.Count()) f.stats.Count("setN", 1) @@ -442,9 +452,14 @@ func (f *Fragment) clearBit(bitmapID, profileID uint64) (bool, error) { } // Update the cache. - if f.bitmap(bitmapID, true).ClearBit(profileID) { - return true, nil + bm := f.bitmap(bitmapID, false) + if bm.cacheoveride > 0 { + bm.SetCount(profileID, bm.cacheoveride) + bm.cacheoveride = 0 + } else { + bm.DecrementCount(profileID) } + f.cache.Add(bitmapID, bm.Count()) f.stats.Count("clearN", 1) @@ -548,12 +563,7 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { // Retrieve the lowest count we have. // If it's too low then don't try finding anymore pairs. - //threshold := results[len(results)-1].Count - threshold := results.Pairs[0].Count - if threshold < MinThreshold { - break - } // If the bitmap doesn't have enough bits set before the intersection // then we can assume that any remaing bitmaps also have a count too low. @@ -591,21 +601,24 @@ func (f *Fragment) topBitmapPairs(bitmapIDs []uint64) []BitmapPair { } // Otherwise retrieve specific bitmaps. - pairs := make([]BitmapPair, len(bitmapIDs)) - for i, bitmapID := range bitmapIDs { + pairs := make([]BitmapPair, 0, len(bitmapIDs)) + for _, bitmapID := range bitmapIDs { // Look up cache first, if available. if n := f.cache.Get(bitmapID); n > 0 { - pairs[i] = BitmapPair{ + pairs = append(pairs, BitmapPair{ ID: bitmapID, Count: n, - } + }) continue } - // Otherwise load from storage. - pairs[i] = BitmapPair{ - ID: bitmapID, - Count: f.Bitmap(bitmapID).Count(), + bm := f.Bitmap(bitmapID) + if bm.Count() > 0 { + // Otherwise load from storage. + pairs = append(pairs, BitmapPair{ + ID: bitmapID, + Count: bm.Count(), + }) } } sort.Sort(BitmapPairs(pairs)) From def03f3a45dead9baea89b69008b065961c7f7dc Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Thu, 2 Mar 2017 15:07:10 -0600 Subject: [PATCH 29/35] more TopN tests --- executor_test.go | 80 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/executor_test.go b/executor_test.go index 2cea37d3d..d6c9d0e91 100644 --- a/executor_test.go +++ b/executor_test.go @@ -215,6 +215,40 @@ func TestExecutor_Execute_TopN_fill(t *testing.T) { } } +// Ensure +func TestExecutor_Execute_TopN_fill_small(t *testing.T) { + idx := MustOpenIndex() + defer idx.Close() + + idx.MustCreateFragmentIfNotExists("d", "f", 0).SetBit(0, 0) + idx.MustCreateFragmentIfNotExists("d", "f", 1).SetBit(0, SliceWidth) + idx.MustCreateFragmentIfNotExists("d", "f", 2).SetBit(0, 2*SliceWidth) + idx.MustCreateFragmentIfNotExists("d", "f", 3).SetBit(0, 3*SliceWidth) + idx.MustCreateFragmentIfNotExists("d", "f", 4).SetBit(0, 4*SliceWidth) + + idx.MustCreateFragmentIfNotExists("d", "f", 0).SetBit(1, 0) + idx.MustCreateFragmentIfNotExists("d", "f", 0).SetBit(1, 1) + + idx.MustCreateFragmentIfNotExists("d", "f", 1).SetBit(2, SliceWidth) + idx.MustCreateFragmentIfNotExists("d", "f", 1).SetBit(2, SliceWidth+1) + + idx.MustCreateFragmentIfNotExists("d", "f", 2).SetBit(3, 2*SliceWidth) + idx.MustCreateFragmentIfNotExists("d", "f", 2).SetBit(3, 2*SliceWidth+1) + + idx.MustCreateFragmentIfNotExists("d", "f", 3).SetBit(4, 3*SliceWidth) + idx.MustCreateFragmentIfNotExists("d", "f", 3).SetBit(4, 3*SliceWidth+1) + + // Execute query. + e := NewExecutor(idx.Index, NewCluster(1)) + if result, err := e.Execute(context.Background(), "d", MustParse(`TopN(frame=f, n=1)`), nil, nil); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(result, []interface{}{[]pilosa.Pair{ + {Key: 0, Count: 5}, + }}) { + t.Fatalf("unexpected result: %s", spew.Sdump(result)) + } +} + // Ensure a TopN() query with a source bitmap can be executed. func TestExecutor_Execute_TopN_Src(t *testing.T) { idx := MustOpenIndex() @@ -248,6 +282,52 @@ func TestExecutor_Execute_TopN_Src(t *testing.T) { } } +//Ensure TopN handles Attribute filters +func TestExecutor_Execute_TopN_Attr(t *testing.T) { + // + idx := MustOpenIndex() + defer idx.Close() + idx.MustCreateFragmentIfNotExists("d", "f", 0).SetBit(0, 0) + idx.MustCreateFragmentIfNotExists("d", "f", 0).SetBit(0, 1) + idx.MustCreateFragmentIfNotExists("d", "f", 1).SetBit(10, SliceWidth) + + if err := idx.Frame("d", "f").BitmapAttrStore().SetAttrs(10, map[string]interface{}{"category": uint64(123)}); err != nil { + t.Fatal(err) + } + e := NewExecutor(idx.Index, NewCluster(1)) + if result, err := e.Execute(context.Background(), "d", MustParse(`TopN(frame="f", n=1, field="category", filters=[123])`), nil, nil); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(result, []interface{}{[]pilosa.Pair{ + {Key: 10, Count: 1}, + }}) { + t.Fatalf("unexpected result: %s", spew.Sdump(result)) + } + +} + +//Ensure TopN handles Attribute filters with source bitmap +func TestExecutor_Execute_TopN_Attr_Src(t *testing.T) { + // + idx := MustOpenIndex() + defer idx.Close() + idx.MustCreateFragmentIfNotExists("d", "f", 0).SetBit(0, 0) + idx.MustCreateFragmentIfNotExists("d", "f", 0).SetBit(0, 1) + idx.MustCreateFragmentIfNotExists("d", "f", 1).SetBit(10, SliceWidth) + + if err := idx.Frame("d", "f").BitmapAttrStore().SetAttrs(10, map[string]interface{}{"category": uint64(123)}); err != nil { + t.Fatal(err) + } + e := NewExecutor(idx.Index, NewCluster(1)) + if result, err := e.Execute(context.Background(), "d", MustParse(`TopN(Bitmap(id=10,frame=f),frame="f", n=1, field="category", filters=[123])`), nil, nil); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(result, []interface{}{[]pilosa.Pair{ + {Key: 10, Count: 1}, + }}) { + t.Fatalf("unexpected result: %s", spew.Sdump(result)) + } + +} + // Ensure a range query can be executed. func TestExecutor_Execute_Range(t *testing.T) { idx := MustOpenIndex() From cbac9ec88bf80a864f95bb19115329b7ca469c39 Mon Sep 17 00:00:00 2001 From: Travis Date: Thu, 2 Mar 2017 15:17:21 -0600 Subject: [PATCH 30/35] move BitmapCache into cache.go. adjust some variable names for clarity --- bitmap.go | 2 +- cache.go | 35 +++++++++++++++++++++++++----- executor.go | 1 + fragment.go | 62 ++++++++++++++++------------------------------------- 4 files changed, 51 insertions(+), 49 deletions(-) diff --git a/bitmap.go b/bitmap.go index d30a9192a..651950e45 100644 --- a/bitmap.go +++ b/bitmap.go @@ -17,7 +17,7 @@ type Bitmap struct { // Attributes associated with the bitmap. Attrs map[string]interface{} - cacheoveride uint64 + adjustedCount uint64 } // NewBitmap returns a new instance of Bitmap. diff --git a/cache.go b/cache.go index 4345c93fd..184310361 100644 --- a/cache.go +++ b/cache.go @@ -176,16 +176,17 @@ func (c *RankCache) Invalidate() { } func (c *RankCache) invalidate() { + // Don't invalidate more than once every X seconds. + // TODO: consider making this configurable. if time.Now().Sub(c.updateTime).Seconds() < 10 { return } - //fmt.Println("RankCache Update") // Convert cache to a sorted list. rankings := make([]BitmapPair, 0, len(c.entries)) - for id, n := range c.entries { + for id, cnt := range c.entries { rankings = append(rankings, BitmapPair{ ID: id, - Count: n, + Count: cnt, }) } sort.Sort(BitmapPairs(rankings)) @@ -200,10 +201,11 @@ func (c *RankCache) invalidate() { // Reset counters. c.updateTime, c.updateN = time.Now(), 0 + // If size is larger than the threshold then trim it. if len(c.entries) > c.ThresholdLength { - for id, n := range c.entries { - if n <= c.ThresholdValue { + for id, cnt := range c.entries { + if cnt <= c.ThresholdValue { delete(c.entries, id) } } @@ -378,3 +380,26 @@ func (p uint64Slice) merge(other []uint64) []uint64 { return ret } + +// BitmapCache provides an interface for caching full bitmaps. +type BitmapCache interface { + Fetch(id uint64) (*Bitmap, bool) + Add(id uint64, b *Bitmap) +} + +// SimpleCache implements BitmapCache +// it is meant to be a short-lived cache for cases where writes are continuing to access +// the same bit within a short time frame (i.e. good for write-heavy loads) +// A read-heavy use case would cause the cache to get bigger, potentially causing the +// node to run out of memory. +type SimpleCache struct { + cache map[uint64]*Bitmap +} + +func (s *SimpleCache) Fetch(id uint64) (*Bitmap, bool) { + return s.cache[id] +} + +func (s *SimpleCache) Add(id uint64, b *Bitmap) { + s.cache[id] = b +} diff --git a/executor.go b/executor.go index 4a2d954c9..26b2f4747 100644 --- a/executor.go +++ b/executor.go @@ -185,6 +185,7 @@ func (e *Executor) executeTopN(ctx context.Context, db string, c *pql.Call, slic other := c.Clone() // Double the size of n for other calls in order to... + // TODO: travis review other.Args["n"] = len(bitmapIDs) * 2 ids := Pairs(pairs).Keys() diff --git a/fragment.go b/fragment.go index e203dd711..8f8698192 100644 --- a/fragment.go +++ b/fragment.go @@ -53,28 +53,6 @@ const ( DefaultFragmentMaxOpN = 2000 ) -// BitmapCacher implements SimpleCache -// it is meant to be a short-lived cache for cases where writes are continuing to access -// the same bit withing a short time frame (i.e. good for write-heavy loads) -// A read-heavy use case would cause the cache to get bigger, potentially causing the -// node to run out of memory. -type BitmapCacher interface { - Fetch(id uint64) (*Bitmap, bool) - Add(id uint64, b *Bitmap) -} - -type SimpleCache struct { - cache map[uint64]*Bitmap -} - -func (s *SimpleCache) Fetch(id uint64) (*Bitmap, bool) { - m, ok := s.cache[id] - return m, ok -} -func (s *SimpleCache) Add(id uint64, p *Bitmap) { - s.cache[id] = p -} - // Fragment represents the intersection of a frame and slice in a database. type Fragment struct { mu sync.Mutex @@ -91,9 +69,12 @@ type Fragment struct { storageData []byte opN int // number of ops since snapshot - // Bitmap cache. + // Cache for bitmap counts. cache Cache + // Cache containing full bitmaps (not just counts). + bitmapCache BitmapCache + // Cached checksums for each block. checksums map[int][]byte @@ -109,8 +90,7 @@ type Fragment struct { // This is set by the parent frame unless overridden for testing. BitmapAttrStore *AttrStore - stats StatsClient - bitmapCache BitmapCacher + stats StatsClient } // NewFragment returns a new instance of Fragment. @@ -351,10 +331,10 @@ func (f *Fragment) bitmap(bitmapID uint64, updateCache bool) *Bitmap { slice: f.slice, writable: false, }}, - cacheoveride: 0, + adjustedCount: 0, } bm.InvalidateCount() - bm.cacheoveride = bm.Count() + bm.adjustedCount = bm.Count() if updateCache { // Update cache. @@ -382,7 +362,6 @@ func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, bool error) } // Write to storage. - if changed, err = f.storage.Add(pos); err != nil { return false, err } @@ -400,18 +379,17 @@ func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, bool error) return false, err } - // Update the cache. + // If adjustedCount is set, then apply that value to bitmap.n instead. bm := f.bitmap(bitmapID, true) - if bm.cacheoveride > 0 { - bm.SetCount(profileID, bm.cacheoveride) - bm.cacheoveride = 0 + if bm.adjustedCount > 0 { + bm.SetCount(profileID, bm.adjustedCount) + bm.adjustedCount = 0 } else { bm.IncrementCount(profileID) + f.cache.Add(bitmapID, bm.Count()) } bm.SetBit(profileID) - f.cache.Add(bitmapID, bm.Count()) - f.stats.Count("setN", 1) return changed, nil @@ -451,15 +429,16 @@ func (f *Fragment) clearBit(bitmapID, profileID uint64) (bool, error) { return false, err } - // Update the cache. - bm := f.bitmap(bitmapID, false) - if bm.cacheoveride > 0 { - bm.SetCount(profileID, bm.cacheoveride) - bm.cacheoveride = 0 + // If adjustedCount is set, then apply that value to bitmap.n instead. + bm := f.bitmap(bitmapID, true) + if bm.adjustedCount > 0 { + bm.SetCount(profileID, bm.adjustedCount) + bm.adjustedCount = 0 } else { bm.DecrementCount(profileID) + f.cache.Add(bitmapID, bm.Count()) } - f.cache.Add(bitmapID, bm.Count()) + bm.ClearBit(profileID) f.stats.Count("clearN", 1) @@ -546,7 +525,6 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { if count == 0 { continue } - //results = append(results, Pair{Key: bitmapID, Count: count}) heap.Push(results, Pair{Key: bitmapID, Count: count}) // If we reach the requested number of pairs and we are not computing @@ -556,7 +534,6 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { if opt.Src == nil { break } - // sort.Sort(Pairs(results)) } continue } @@ -622,7 +599,6 @@ func (f *Fragment) topBitmapPairs(bitmapIDs []uint64) []BitmapPair { } } sort.Sort(BitmapPairs(pairs)) - //debugDumpPairs(pairs) return pairs } From c013661880d0682521813f9374e16e8704636e7e Mon Sep 17 00:00:00 2001 From: Travis Date: Thu, 2 Mar 2017 16:00:34 -0600 Subject: [PATCH 31/35] bug fix in SimpleCache.Fetch (really just reverting my change) --- cache.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cache.go b/cache.go index 184310361..7c2e96f2d 100644 --- a/cache.go +++ b/cache.go @@ -397,7 +397,8 @@ type SimpleCache struct { } func (s *SimpleCache) Fetch(id uint64) (*Bitmap, bool) { - return s.cache[id] + m, ok := s.cache[id] + return m, ok } func (s *SimpleCache) Add(id uint64, b *Bitmap) { From c3ed12c20ebe09c12efc431bfcd9666f1c588305 Mon Sep 17 00:00:00 2001 From: Travis Date: Mon, 6 Mar 2017 11:06:22 -0600 Subject: [PATCH 32/35] Refactor fragment.bitmap() so that it leverages `bitmapCache` and so that it's no longer reponsible for updating the count cache. This commit also helps SetBit/ClearBit performance by allowing them to work against data from `bitmapCache` instead of loading bitmaps from fragment.storage every time. --- bitmap.go | 2 -- fragment.go | 72 ++++++++++++++++++++++------------------------ roaring/roaring.go | 2 +- 3 files changed, 35 insertions(+), 41 deletions(-) diff --git a/bitmap.go b/bitmap.go index 651950e45..3a2e4c96a 100644 --- a/bitmap.go +++ b/bitmap.go @@ -16,8 +16,6 @@ type Bitmap struct { // Attributes associated with the bitmap. Attrs map[string]interface{} - - adjustedCount uint64 } // NewBitmap returns a new instance of Bitmap. diff --git a/fragment.go b/fragment.go index 8f8698192..51259585a 100644 --- a/fragment.go +++ b/fragment.go @@ -245,7 +245,7 @@ func (f *Fragment) openCache() error { // This will cause them to be added to the cache. for _, bitmapID := range pb.BitmapIDs { //n := f.storage.CountRange(bitmapID*SliceWidth, (bitmapID+1)*SliceWidth) - n := f.bitmap(bitmapID, false).Count() + n := f.bitmap(bitmapID, true, false).Count() f.cache.BulkAdd(bitmapID, n) } f.cache.Invalidate() @@ -312,33 +312,35 @@ func (f *Fragment) logger() *log.Logger { return log.New(f.LogOutput, "", log.Ls func (f *Fragment) Bitmap(bitmapID uint64) *Bitmap { f.mu.Lock() defer f.mu.Unlock() - return f.bitmap(bitmapID, false) + return f.bitmap(bitmapID, true, true) } -func (f *Fragment) bitmap(bitmapID uint64, updateCache bool) *Bitmap { - r, ok := f.bitmapCache.Fetch(bitmapID) - if ok && r != nil { - return r +func (f *Fragment) bitmap(bitmapID uint64, checkBitmapCache bool, updateBitmapCache bool) *Bitmap { + + if checkBitmapCache { + r, ok := f.bitmapCache.Fetch(bitmapID) + if ok && r != nil { + return r + } } + // Only use a subset of the containers. // NOTE: The start & end ranges must be divisible by data := f.storage.OffsetRange(f.slice*SliceWidth, bitmapID*SliceWidth, (bitmapID+1)*SliceWidth) // Reference bitmap subrange in storage. + // We Clone() data because otherwise bm will contains pointers to containers in storage. + // This causes unexpected results when we cache the bitmap and try to use it later. bm := &Bitmap{ segments: []BitmapSegment{{ - data: *data, + data: *data.Clone(), slice: f.slice, writable: false, }}, - adjustedCount: 0, } bm.InvalidateCount() - bm.adjustedCount = bm.Count() - if updateCache { - // Update cache. - f.cache.Add(bitmapID, bm.Count()) + if updateBitmapCache { f.bitmapCache.Add(bitmapID, bm) } @@ -353,9 +355,9 @@ func (f *Fragment) SetBit(bitmapID, profileID uint64) (changed bool, err error) return f.setBit(bitmapID, profileID) } -func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, bool error) { - // Determine the position of the bit in the storage. +func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, err error) { changed = false + // Determine the position of the bit in the storage. pos, err := f.pos(bitmapID, profileID) if err != nil { return false, err @@ -374,22 +376,18 @@ func (f *Fragment) setBit(bitmapID, profileID uint64) (changed bool, bool error) // Invalidate block checksum. delete(f.checksums, int(bitmapID/HashBlockSize)) - // If the number of operations exceeds the limit then snapshot. + // Increment number of operations until snapshot is required. if err := f.incrementOpN(); err != nil { return false, err } - // If adjustedCount is set, then apply that value to bitmap.n instead. - bm := f.bitmap(bitmapID, true) - if bm.adjustedCount > 0 { - bm.SetCount(profileID, bm.adjustedCount) - bm.adjustedCount = 0 - } else { - bm.IncrementCount(profileID) - f.cache.Add(bitmapID, bm.Count()) - } + // Get the bitmap from bitmapCache or fragment.storage. + bm := f.bitmap(bitmapID, true, true) bm.SetBit(profileID) + // Update the cache. + f.cache.Add(bitmapID, bm.Count()) + f.stats.Count("setN", 1) return changed, nil @@ -403,7 +401,8 @@ func (f *Fragment) ClearBit(bitmapID, profileID uint64) (bool, error) { return f.clearBit(bitmapID, profileID) } -func (f *Fragment) clearBit(bitmapID, profileID uint64) (bool, error) { +func (f *Fragment) clearBit(bitmapID, profileID uint64) (changed bool, err error) { + changed = false // Determine the position of the bit in the storage. pos, err := f.pos(bitmapID, profileID) if err != nil { @@ -411,8 +410,7 @@ func (f *Fragment) clearBit(bitmapID, profileID uint64) (bool, error) { } // Write to storage. - changed, err := f.storage.Remove(pos) - if err != nil { + if changed, err = f.storage.Remove(pos); err != nil { return false, err } @@ -429,17 +427,13 @@ func (f *Fragment) clearBit(bitmapID, profileID uint64) (bool, error) { return false, err } - // If adjustedCount is set, then apply that value to bitmap.n instead. - bm := f.bitmap(bitmapID, true) - if bm.adjustedCount > 0 { - bm.SetCount(profileID, bm.adjustedCount) - bm.adjustedCount = 0 - } else { - bm.DecrementCount(profileID) - f.cache.Add(bitmapID, bm.Count()) - } + // Get the bitmap from bitmapCache or fragment.storage. + bm := f.bitmap(bitmapID, true, true) bm.ClearBit(profileID) + // Update the cache. + f.cache.Add(bitmapID, bm.Count()) + f.stats.Count("clearN", 1) return changed, nil @@ -908,7 +902,10 @@ func (f *Fragment) Import(bitmapIDs, profileIDs []uint64) error { // Update cache counts for all bitmaps. for bitmapID := range set { - f.cache.BulkAdd(bitmapID, f.bitmap(bitmapID, false).Count()) + // Import should ALWAYS have bitmap() load a new bm from fragment.storage + // because the bitmap that's in bitmapCache hasn't been updated with + // this import's data. + f.cache.BulkAdd(bitmapID, f.bitmap(bitmapID, false, false).Count()) } f.cache.Invalidate() @@ -1256,7 +1253,6 @@ func (s *FragmentSyncer) SyncFragment() error { // Determine replica set. nodes := s.Cluster.FragmentNodes(s.Fragment.DB(), s.Fragment.Slice()) if len(nodes) == 1 { - //fmt.Println("no place to replicate", s.Fragment.DB(), s.Fragment.Frame(), s.Fragment.Slice()) return nil } diff --git a/roaring/roaring.go b/roaring/roaring.go index 58b4d1e19..e74739311 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1084,7 +1084,7 @@ func (c *container) clone() *container { copy(other.bitmap, c.bitmap) } - return c + return other } // WriteTo writes c to w. From 6216c4fb8a03a450277dfce9976bb2f4b475a91b Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Mon, 6 Mar 2017 15:34:57 -0600 Subject: [PATCH 33/35] don't panic if 'n' not supplied to TopN --- executor.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/executor.go b/executor.go index 26b2f4747..728e22cee 100644 --- a/executor.go +++ b/executor.go @@ -168,7 +168,10 @@ func (e *Executor) executeBitmapCallSlice(ctx context.Context, db string, c *pql // requeries to retrieve the full counts for each of the top results. func (e *Executor) executeTopN(ctx context.Context, db string, c *pql.Call, slices []uint64, opt *ExecOptions) ([]Pair, error) { bitmapIDs, _ := c.Args["ids"].([]uint64) - n := c.Args["n"].(uint64) + var n uint64 + if nval, ok := c.Args["n"]; ok { + n = nval.(uint64) + } // Execute original query. pairs, err := e.executeTopNSlices(ctx, db, c, slices, opt) @@ -197,7 +200,7 @@ func (e *Executor) executeTopN(ctx context.Context, db string, c *pql.Call, slic return nil, err } - if int(n) < len(trimmedList) { + if n != 0 && int(n) < len(trimmedList) { trimmedList = trimmedList[0:n] } return trimmedList, nil From 03e1f4f09bb54f0262b333cddf5365a7b96680f1 Mon Sep 17 00:00:00 2001 From: Travis Date: Tue, 7 Mar 2017 08:57:03 -0600 Subject: [PATCH 34/35] turn on bitmapCache support in openCache() --- fragment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fragment.go b/fragment.go index fd73ff343..18306bc5d 100644 --- a/fragment.go +++ b/fragment.go @@ -245,7 +245,7 @@ func (f *Fragment) openCache() error { // This will cause them to be added to the cache. for _, bitmapID := range pb.BitmapIDs { //n := f.storage.CountRange(bitmapID*SliceWidth, (bitmapID+1)*SliceWidth) - n := f.bitmap(bitmapID, true, false).Count() + n := f.bitmap(bitmapID, true, true).Count() f.cache.BulkAdd(bitmapID, n) } f.cache.Invalidate() From b6430bcc0917a92cbf74cbdae732b01b06859c74 Mon Sep 17 00:00:00 2001 From: Travis Date: Tue, 7 Mar 2017 15:25:36 -0600 Subject: [PATCH 35/35] remove code that is no longer used: - `cacheoveride` - `Bitmap.SetCount()` --- bitmap.go | 6 ------ fragment.go | 2 -- 2 files changed, 8 deletions(-) diff --git a/bitmap.go b/bitmap.go index d30a9192a..2e27ded7a 100644 --- a/bitmap.go +++ b/bitmap.go @@ -16,8 +16,6 @@ type Bitmap struct { // Attributes associated with the bitmap. Attrs map[string]interface{} - - cacheoveride uint64 } // NewBitmap returns a new instance of Bitmap. @@ -191,10 +189,6 @@ func (b *Bitmap) DecrementCount(i uint64) { } } } -func (b *Bitmap) SetCount(i uint64, count uint64) { - seg := b.segment(i / SliceWidth) - seg.n = count -} // Count returns the number of set bits in the bitmap. func (b *Bitmap) Count() uint64 { diff --git a/fragment.go b/fragment.go index 18306bc5d..e70dab80c 100644 --- a/fragment.go +++ b/fragment.go @@ -337,10 +337,8 @@ func (f *Fragment) bitmap(bitmapID uint64, checkBitmapCache bool, updateBitmapCa slice: f.slice, writable: false, }}, - cacheoveride: 0, } bm.InvalidateCount() - bm.cacheoveride = bm.Count() if updateBitmapCache { f.bitmapCache.Add(bitmapID, bm)