From f68362c40c967f2b71ae1e8034ec007175b3a450 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Thu, 20 Jul 2017 16:11:13 -0500 Subject: [PATCH 1/5] implement nopcache --- cache.go | 34 +++++++++++++++++++++++++++++++++- fragment.go | 2 ++ frame.go | 3 ++- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/cache.go b/cache.go index c32881a29..e5f1163a0 100644 --- a/cache.go +++ b/cache.go @@ -54,7 +54,7 @@ type Cache interface { SetStats(s StatsClient) } -// LRUCache represents a least recently used Cache implemenation. +// LRUCache represents a least recently used Cache implementation. type LRUCache struct { cache *lru.Cache counts map[uint64]uint64 @@ -483,3 +483,35 @@ func (s *SimpleCache) Fetch(id uint64) (*Bitmap, bool) { func (s *SimpleCache) Add(id uint64, b *Bitmap) { s.cache[id] = b } + +type NopCache struct { + stats StatsClient +} + +// NopCache implement Cache interface, returns no cache for cache type None +var _ Cache = &NopCache{} + +// NewNopeCache returns a new instance of NopCache. +func NewNopCache() *NopCache { + c := &NopCache{ + stats: NopStatsClient, + } + return c +} + +func (c *NopCache) Add(id uint64, n uint64) {} +func (c *NopCache) BulkAdd(id uint64, n uint64) {} +func (c *NopCache) Get(id uint64) uint64 { return 0 } +func (c *NopCache) IDs() []uint64 { return make([]uint64, 0, 0) } + +func (c *NopCache) Invalidate() {} +func (c *NopCache) Len() int { return 0 } +func (c *NopCache) Recalculate() { +} +func (c *NopCache) SetStats(s StatsClient) { + c.stats = s +} + +func (c *NopCache) Top() []BitmapPair { + return []BitmapPair{} +} diff --git a/fragment.go b/fragment.go index 64c31434b..fdfcddd86 100644 --- a/fragment.go +++ b/fragment.go @@ -249,6 +249,8 @@ func (f *Fragment) openCache() error { f.cache = NewRankCache(f.CacheSize) case CacheTypeLRU: f.cache = NewLRUCache(f.CacheSize) + case CacheTypeNone: + f.cache = NewNopCache() default: return ErrInvalidCacheType } diff --git a/frame.go b/frame.go index 42990aeb6..d295ff01c 100644 --- a/frame.go +++ b/frame.go @@ -905,12 +905,13 @@ func (p importBitSet) Less(i, j int) bool { return p.rowIDs[i] < p.rowIDs[j] } const ( CacheTypeLRU = "lru" CacheTypeRanked = "ranked" + CacheTypeNone = "none" ) // IsValidCacheType returns true if v is a valid cache type. func IsValidCacheType(v string) bool { switch v { - case CacheTypeLRU, CacheTypeRanked: + case CacheTypeLRU, CacheTypeRanked, CacheTypeNone: return true default: return false From 2cfcf497e110aa8e660015cbd3ae1bd86c551f59 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Fri, 21 Jul 2017 00:16:37 -0500 Subject: [PATCH 2/5] add tests for none cache --- cache.go | 6 +++--- fragment.go | 4 ++++ fragment_test.go | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/cache.go b/cache.go index e5f1163a0..d6d4174cd 100644 --- a/cache.go +++ b/cache.go @@ -501,11 +501,11 @@ func NewNopCache() *NopCache { func (c *NopCache) Add(id uint64, n uint64) {} func (c *NopCache) BulkAdd(id uint64, n uint64) {} -func (c *NopCache) Get(id uint64) uint64 { return 0 } -func (c *NopCache) IDs() []uint64 { return make([]uint64, 0, 0) } +func (c *NopCache) Get(id uint64) uint64 { return 0 } +func (c *NopCache) IDs() []uint64 { return make([]uint64, 0, 0) } func (c *NopCache) Invalidate() {} -func (c *NopCache) Len() int { return 0 } +func (c *NopCache) Len() int { return 0 } func (c *NopCache) Recalculate() { } func (c *NopCache) SetStats(s StatsClient) { diff --git a/fragment.go b/fragment.go index fdfcddd86..16ec5869f 100644 --- a/fragment.go +++ b/fragment.go @@ -704,6 +704,10 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { } func (f *Fragment) topBitmapPairs(rowIDs []uint64) []BitmapPair { + // Don't retrieve from storage if CacheTypeNone + if f.CacheType == CacheTypeNone { + return f.cache.Top() + } // If no specific rows are requested, retrieve top rows. if len(rowIDs) == 0 { f.mu.Lock() diff --git a/fragment_test.go b/fragment_test.go index 01f4a46bc..5a58acd60 100644 --- a/fragment_test.go +++ b/fragment_test.go @@ -415,6 +415,24 @@ func TestFragment_TopN_IDs(t *testing.T) { } } +// Ensure a fragment can return top rows when specified by ID. +func TestFragment_TopN_NopCache(t *testing.T) { + f := test.MustOpenFragment("i", "f", pilosa.ViewStandard, 0, pilosa.CacheTypeNone) + defer f.Close() + + // Set bits on various rows. + f.MustSetBits(100, 1, 2, 3) + f.MustSetBits(101, 4, 5, 6, 7) + f.MustSetBits(102, 8, 9, 10, 11, 12) + + // Retrieve top rows. + if pairs, err := f.Top(pilosa.TopOptions{RowIDs: []uint64{100, 101, 200}}); err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(pairs, []pilosa.Pair{}) { + t.Fatalf("unexpected pairs: %s", spew.Sdump(pairs)) + } +} + // Ensure the fragment cache limit works func TestFragment_TopN_CacheSize(t *testing.T) { slice := uint64(0) From 4aa487daacfbffaa391a762ec60549e966183c1c Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Fri, 21 Jul 2017 13:28:20 -0500 Subject: [PATCH 3/5] check range with CacheTypeNone --- index.go | 3 +-- index_test.go | 12 ++++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/index.go b/index.go index a5cdd1911..04903cad5 100644 --- a/index.go +++ b/index.go @@ -394,10 +394,9 @@ func (i *Index) createFrame(name string, opt FrameOptions) (*Frame, error) { if opt.RangeEnabled { if opt.InverseEnabled { return nil, ErrInverseRangeNotAllowed - } else if opt.CacheType != "" && opt.CacheType != CacheTypeLRU { + } else if opt.CacheType != "" && opt.CacheType != CacheTypeNone { return nil, ErrRangeCacheNotAllowed } - opt.CacheSize = 0 } else { if len(opt.Fields) > 0 { return nil, ErrFrameFieldsNotAllowed diff --git a/index_test.go b/index_test.go index 2c23f7a40..d3e03ebc7 100644 --- a/index_test.go +++ b/index_test.go @@ -149,6 +149,18 @@ func TestIndex_CreateFrame(t *testing.T) { } }) + t.Run("RangeEnabledWithCacheTypeNone", func(t *testing.T) { + index := test.MustOpenIndex() + defer index.Close() + if _, err := index.CreateFrame("f", pilosa.FrameOptions{ + RangeEnabled: true, + CacheType: pilosa.CacheTypeNone, + CacheSize: uint32(5), + }); err != nil { + t.Fatal(err) + } + }) + t.Run("ErrFrameFieldsNotAllowed", func(t *testing.T) { index := test.MustOpenIndex() defer index.Close() From aa73aec6c9bef7e13ad72de823a7e9d9dd77fb09 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Fri, 21 Jul 2017 13:31:31 -0500 Subject: [PATCH 4/5] TopN NopCache comment --- fragment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fragment_test.go b/fragment_test.go index 5a58acd60..982d4a066 100644 --- a/fragment_test.go +++ b/fragment_test.go @@ -415,7 +415,7 @@ func TestFragment_TopN_IDs(t *testing.T) { } } -// Ensure a fragment can return top rows when specified by ID. +// Ensure a fragment return none if CacheTypeNone is set func TestFragment_TopN_NopCache(t *testing.T) { f := test.MustOpenFragment("i", "f", pilosa.ViewStandard, 0, pilosa.CacheTypeNone) defer f.Close() From 326a16209421adb94b99278b3d9cc548296a1e6e Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Mon, 24 Jul 2017 22:09:31 -0500 Subject: [PATCH 5/5] fixed review --- cache.go | 8 ++++---- fragment.go | 2 +- index.go | 3 --- index_test.go | 2 +- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/cache.go b/cache.go index d6d4174cd..242e5a6ab 100644 --- a/cache.go +++ b/cache.go @@ -484,19 +484,19 @@ func (s *SimpleCache) Add(id uint64, b *Bitmap) { s.cache[id] = b } +// NopCache represents a no-op Cache implementation. type NopCache struct { stats StatsClient } -// NopCache implement Cache interface, returns no cache for cache type None +// Ensure NopCache implements Cache. var _ Cache = &NopCache{} -// NewNopeCache returns a new instance of NopCache. +// NewNopCache returns a new instance of NopCache. func NewNopCache() *NopCache { - c := &NopCache{ + return &NopCache{ stats: NopStatsClient, } - return c } func (c *NopCache) Add(id uint64, n uint64) {} diff --git a/fragment.go b/fragment.go index 16ec5869f..b20dfd9c0 100644 --- a/fragment.go +++ b/fragment.go @@ -704,7 +704,7 @@ func (f *Fragment) Top(opt TopOptions) ([]Pair, error) { } func (f *Fragment) topBitmapPairs(rowIDs []uint64) []BitmapPair { - // Don't retrieve from storage if CacheTypeNone + // Don't retrieve from storage if CacheTypeNone. if f.CacheType == CacheTypeNone { return f.cache.Top() } diff --git a/index.go b/index.go index 04903cad5..a07d28dfe 100644 --- a/index.go +++ b/index.go @@ -388,9 +388,6 @@ func (i *Index) createFrame(name string, opt FrameOptions) (*Frame, error) { } // Validate mutually exclusive options if ranges are enabled. - // - // NOTE(https://github.com/pilosa/pilosa/issues/399): - // Cache type should be validated as "none" once it is allowed. if opt.RangeEnabled { if opt.InverseEnabled { return nil, ErrInverseRangeNotAllowed diff --git a/index_test.go b/index_test.go index d3e03ebc7..0d17a72af 100644 --- a/index_test.go +++ b/index_test.go @@ -155,7 +155,7 @@ func TestIndex_CreateFrame(t *testing.T) { if _, err := index.CreateFrame("f", pilosa.FrameOptions{ RangeEnabled: true, CacheType: pilosa.CacheTypeNone, - CacheSize: uint32(5), + CacheSize: uint32(5), }); err != nil { t.Fatal(err) }