From e58d40718209fbce63ae2e665fcadbccab52373e Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Thu, 7 Jun 2018 22:50:21 -0500 Subject: [PATCH] unexport (most) View methods --- cluster.go | 2 +- field.go | 32 ++++++------ holder.go | 8 +-- holder_test.go | 13 ++--- test/holder.go | 18 ------- view.go | 68 ++++++++++--------------- view_internal_test.go | 68 +++++++++++++++++++++++++ view_test.go | 115 ------------------------------------------ 8 files changed, 122 insertions(+), 202 deletions(-) create mode 100644 view_internal_test.go delete mode 100644 view_test.go diff --git a/cluster.go b/cluster.go index 9723e5bce..9cf54df08 100644 --- a/cluster.go +++ b/cluster.go @@ -629,7 +629,7 @@ func (c *Cluster) fragsByHost(idx *Index) fragsByHost { for _, field := range idx.Fields() { for _, view := range field.Views() { - fieldViews.addView(field.Name(), view.Name()) + fieldViews.addView(field.Name(), view.name) } } diff --git a/field.go b/field.go index 8d35de8fa..8c7683cd1 100644 --- a/field.go +++ b/field.go @@ -137,7 +137,7 @@ func (f *Field) MaxSlice() uint64 { var max uint64 for _, view := range f.views { - if viewMaxSlice := view.MaxSlice(); viewMaxSlice > max { + if viewMaxSlice := view.calculateMaxSlice(); viewMaxSlice > max { max = viewMaxSlice } } @@ -249,11 +249,11 @@ func (f *Field) openViews() error { name := filepath.Base(fi.Name()) view := f.newView(f.ViewPath(name), name) - if err := view.Open(); err != nil { - return fmt.Errorf("opening view: view=%s, err=%s", view.Name(), err) + if err := view.open(); err != nil { + return fmt.Errorf("opening view: view=%s, err=%s", view.name, err) } view.RowAttrStore = f.rowAttrStore - f.views[view.Name()] = view + f.views[view.name] = view } return nil @@ -369,7 +369,7 @@ func (f *Field) Close() error { // Close all views. for _, view := range f.views { - if err := view.Close(); err != nil { + if err := view.close(); err != nil { return err } } @@ -447,9 +447,9 @@ func (f *Field) deleteBSIGroupAndView(name string) error { if view := f.views[viewName]; view != nil { delete(f.views, viewName) - if err := view.Close(); err != nil { + if err := view.close(); err != nil { return errors.Wrap(err, "closing view") - } else if err := os.RemoveAll(view.Path()); err != nil { + } else if err := os.RemoveAll(view.path); err != nil { return errors.Wrap(err, "deleting directory") } } @@ -538,7 +538,7 @@ func (f *Field) viewNames() []string { // RecalculateCaches recalculates caches on every view in the field. func (f *Field) RecalculateCaches() { for _, view := range f.Views() { - view.RecalculateCaches() + view.recalculateCaches() } } @@ -579,11 +579,11 @@ func (f *Field) createViewIfNotExistsBase(name string) (*View, bool, error) { view := f.newView(f.ViewPath(name), name) - if err := view.Open(); err != nil { + if err := view.open(); err != nil { return nil, false, errors.Wrap(err, "opening view") } view.RowAttrStore = f.rowAttrStore - f.views[view.Name()] = view + f.views[view.name] = view return view, true, nil } @@ -606,12 +606,12 @@ func (f *Field) DeleteView(name string) error { } // Close data files before deletion. - if err := view.Close(); err != nil { + if err := view.close(); err != nil { return errors.Wrap(err, "closing view") } // Delete view directory. - if err := os.RemoveAll(view.Path()); err != nil { + if err := os.RemoveAll(view.path); err != nil { return errors.Wrap(err, "deleting directory") } @@ -656,7 +656,7 @@ func (f *Field) SetBit(name string, rowID, colID uint64, t *time.Time) (changed } // Set non-time bit. - if v, err := view.SetBit(rowID, colID); err != nil { + if v, err := view.setBit(rowID, colID); err != nil { return changed, errors.Wrap(err, "setting on view") } else if v { changed = v @@ -674,7 +674,7 @@ func (f *Field) SetBit(name string, rowID, colID uint64, t *time.Time) (changed return changed, errors.Wrapf(err, "creating view %s", subname) } - if c, err := view.SetBit(rowID, colID); err != nil { + if c, err := view.setBit(rowID, colID); err != nil { return changed, errors.Wrapf(err, "setting on view %s", subname) } else if c { changed = true @@ -698,7 +698,7 @@ func (f *Field) ClearBit(name string, rowID, colID uint64, t *time.Time) (change } // Clear non-time bit. - if v, err := view.ClearBit(rowID, colID); err != nil { + if v, err := view.clearBit(rowID, colID); err != nil { return changed, errors.Wrap(err, "clearing on view") } else if v { changed = v @@ -716,7 +716,7 @@ func (f *Field) ClearBit(name string, rowID, colID uint64, t *time.Time) (change return changed, errors.Wrapf(err, "creating view %s", subname) } - if c, err := view.ClearBit(rowID, colID); err != nil { + if c, err := view.clearBit(rowID, colID); err != nil { return changed, errors.Wrapf(err, "clearing on view %s", subname) } else if c { changed = true diff --git a/holder.go b/holder.go index 89bd508ac..c7ae15e0d 100644 --- a/holder.go +++ b/holder.go @@ -217,7 +217,7 @@ func (h *Holder) Schema() []*IndexInfo { for _, field := range index.Fields() { fi := &FieldInfo{Name: field.Name(), Options: field.Options()} for _, view := range field.Views() { - fi.Views = append(fi.Views, &ViewInfo{Name: view.Name()}) + fi.Views = append(fi.Views, &ViewInfo{Name: view.name}) } sort.Sort(viewInfoSlice(fi.Views)) di.Fields = append(di.Fields, fi) @@ -437,7 +437,7 @@ func (h *Holder) flushCaches() { for _, index := range h.Indexes() { for _, field := range index.Fields() { for _, view := range field.Views() { - for _, fragment := range view.Fragments() { + for _, fragment := range view.allFragments() { select { case <-h.closing: return @@ -808,14 +808,14 @@ func (c *HolderCleaner) CleanHolder() error { // Get the fragments registered in memory. for _, field := range index.Fields() { for _, view := range field.Views() { - for _, fragment := range view.Fragments() { + for _, fragment := range view.allFragments() { fragSlice := fragment.slice // Ignore fragments that should be present. if uint64InSlice(fragSlice, containedSlices) { continue } // Delete fragment. - if err := view.DeleteFragment(fragSlice); err != nil { + if err := view.deleteFragment(fragSlice); err != nil { return errors.Wrap(err, "deleting fragment") } } diff --git a/holder_test.go b/holder_test.go index 9739de65d..b5a494028 100644 --- a/holder_test.go +++ b/holder_test.go @@ -210,9 +210,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if field, err := idx.CreateField("bar", pilosa.FieldOptions{}); err != nil { t.Fatal(err) - } else if view, err := field.CreateViewIfNotExists(pilosa.ViewStandard); err != nil { - t.Fatal(err) - } else if _, err := view.SetBit(0, 0); err != nil { + } else if _, err := field.SetBit(pilosa.ViewStandard, 0, 0, nil); err != nil { t.Fatal(err) } else if err := h.Holder.Close(); err != nil { t.Fatal(err) @@ -233,9 +231,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if field, err := idx.CreateField("bar", pilosa.FieldOptions{}); err != nil { t.Fatal(err) - } else if view, err := field.CreateViewIfNotExists(pilosa.ViewStandard); err != nil { - t.Fatal(err) - } else if _, err := view.SetBit(0, 0); err != nil { + } else if _, err := field.SetBit(pilosa.ViewStandard, 0, 0, nil); err != nil { t.Fatal(err) } else if err := h.Holder.Close(); err != nil { t.Fatal(err) @@ -261,7 +257,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if view, err := field.CreateViewIfNotExists(pilosa.ViewStandard); err != nil { t.Fatal(err) - } else if _, err := view.SetBit(0, 0); err != nil { + } else if _, err := field.SetBit(pilosa.ViewStandard, 0, 0, nil); err != nil { t.Fatal(err) } else if err := view.Fragment(0).FlushCache(); err != nil { t.Fatal(err) @@ -400,7 +396,8 @@ func TestHolderSyncer_SyncHolder(t *testing.T) { hldr0.SetBit("i", "f0", 9, SliceWidth+5) - hldr0.MustCreateFragmentIfNotExists("y", "z", pilosa.ViewStandard, 0) + // Set a bit to create the fragment. + hldr0.SetBit("y", "z", 0, 0) // Set data on the remote holder. hldr1.SetBit("i", "f", 0, 4000) diff --git a/test/holder.go b/test/holder.go index f49613fb0..4484850fd 100644 --- a/test/holder.go +++ b/test/holder.go @@ -89,24 +89,6 @@ func (h *Holder) MustCreateFieldIfNotExists(index, field string) *Field { return f } -// MustCreateFragmentIfNotExists returns a given fragment. Panic on error. -func (h *Holder) MustCreateFragmentIfNotExists(index, field, view string, slice uint64) *Fragment { - idx := h.MustCreateIndexIfNotExists(index, pilosa.IndexOptions{}) - f, err := idx.CreateFieldIfNotExists(field, pilosa.FieldOptions{}) - if err != nil { - panic(err) - } - v, err := f.CreateViewIfNotExists(view) - if err != nil { - panic(err) - } - frag, err := v.CreateFragmentIfNotExists(slice) - if err != nil { - panic(err) - } - return &Fragment{Fragment: frag} -} - // MustCreateRankedFragmentIfNotExists returns a given fragment with a ranked cache. Panic on error. func (h *Holder) MustCreateRankedFragmentIfNotExists(index, field, view string, slice uint64) *Fragment { idx := h.MustCreateIndexIfNotExists(index, pilosa.IndexOptions{}) diff --git a/view.go b/view.go index 0049edaa1..a7aaf0cb8 100644 --- a/view.go +++ b/view.go @@ -82,20 +82,8 @@ func NewView(path, index, field, name string, cacheSize uint32) *View { } } -// Name returns the name the view was initialized with. -func (v *View) Name() string { return v.name } - -// Index returns the index name the view was initialized with. -func (v *View) Index() string { return v.index } - -// Field returns the field name the view was initialized with. -func (v *View) Field() string { return v.field } - -// Path returns the path the view was initialized with. -func (v *View) Path() string { return v.path } - -// Open opens and initializes the view. -func (v *View) Open() error { +// open opens and initializes the view. +func (v *View) open() error { // Never keep a cache for field views. if strings.HasPrefix(v.name, viewBSIGroupPrefix) { @@ -116,7 +104,7 @@ func (v *View) Open() error { return nil }(); err != nil { - v.Close() + v.close() return err } @@ -149,7 +137,7 @@ func (v *View) openFragments() error { continue } - frag := v.newFragment(v.FragmentPath(slice), slice) + frag := v.newFragment(v.fragmentPath(slice), slice) if err := frag.Open(); err != nil { return fmt.Errorf("open fragment: slice=%d, err=%s", frag.slice, err) } @@ -160,8 +148,8 @@ func (v *View) openFragments() error { return nil } -// Close closes the view and its fragments. -func (v *View) Close() error { +// close closes the view and its fragments. +func (v *View) close() error { v.mu.Lock() defer v.mu.Unlock() @@ -176,8 +164,8 @@ func (v *View) Close() error { return nil } -// MaxSlice returns the max slice in the view. -func (v *View) MaxSlice() uint64 { +// calculateMaxSlice returns the max slice in the view. +func (v *View) calculateMaxSlice() uint64 { v.mu.RLock() defer v.mu.RUnlock() @@ -191,8 +179,8 @@ func (v *View) MaxSlice() uint64 { return max } -// FragmentPath returns the path to a fragment in the view. -func (v *View) FragmentPath(slice uint64) string { +// fragmentPath returns the path to a fragment in the view. +func (v *View) fragmentPath(slice uint64) string { return filepath.Join(v.path, "fragments", strconv.FormatUint(slice, 10)) } @@ -205,8 +193,8 @@ func (v *View) Fragment(slice uint64) *Fragment { func (v *View) fragment(slice uint64) *Fragment { return v.fragments[slice] } -// Fragments returns a list of all fragments in the view. -func (v *View) Fragments() []*Fragment { +// allFragments returns a list of all fragments in the view. +func (v *View) allFragments() []*Fragment { v.mu.Lock() defer v.mu.Unlock() @@ -217,9 +205,9 @@ func (v *View) Fragments() []*Fragment { return other } -// RecalculateCaches recalculates the cache on every fragment in the view. -func (v *View) RecalculateCaches() { - for _, fragment := range v.Fragments() { +// recalculateCaches recalculates the cache on every fragment in the view. +func (v *View) recalculateCaches() { + for _, fragment := range v.allFragments() { fragment.RecalculateCache() } } @@ -238,7 +226,7 @@ func (v *View) createFragmentIfNotExists(slice uint64) (*Fragment, error) { } // Initialize and open fragment. - frag := v.newFragment(v.FragmentPath(slice), slice) + frag := v.newFragment(v.fragmentPath(slice), slice) if err := frag.Open(); err != nil { return nil, errors.Wrap(err, "opening fragment") } @@ -273,8 +261,8 @@ func (v *View) newFragment(path string, slice uint64) *Fragment { return frag } -// DeleteFragment removes the fragment from the view. -func (v *View) DeleteFragment(slice uint64) error { +// deleteFragment removes the fragment from the view. +func (v *View) deleteFragment(slice uint64) error { fragment := v.fragments[slice] if fragment == nil { @@ -306,7 +294,7 @@ func (v *View) DeleteFragment(slice uint64) error { // row returns a row for a slice of the view. func (v *View) row(rowID uint64) *Row { row := NewRow() - for _, frag := range v.Fragments() { + for _, frag := range v.allFragments() { fr := frag.row(rowID) if fr == nil { continue @@ -317,8 +305,8 @@ func (v *View) row(rowID uint64) *Row { } -// SetBit sets a bit within the view. -func (v *View) SetBit(rowID, columnID uint64) (changed bool, err error) { +// setBit sets a bit within the view. +func (v *View) setBit(rowID, columnID uint64) (changed bool, err error) { slice := columnID / SliceWidth frag, err := v.CreateFragmentIfNotExists(slice) if err != nil { @@ -327,8 +315,8 @@ func (v *View) SetBit(rowID, columnID uint64) (changed bool, err error) { return frag.setBit(rowID, columnID) } -// ClearBit clears a bit within the view. -func (v *View) ClearBit(rowID, columnID uint64) (changed bool, err error) { +// clearBit clears a bit within the view. +func (v *View) clearBit(rowID, columnID uint64) (changed bool, err error) { slice := columnID / SliceWidth frag, err := v.CreateFragmentIfNotExists(slice) if err != nil { @@ -359,7 +347,7 @@ func (v *View) setValue(columnID uint64, bitDepth uint, value uint64) (changed b // sum returns the sum & count of a field. func (v *View) sum(filter *Row, bitDepth uint) (sum, count uint64, err error) { - for _, f := range v.Fragments() { + for _, f := range v.allFragments() { fsum, fcount, err := f.sum(filter, bitDepth) if err != nil { return sum, count, err @@ -373,7 +361,7 @@ func (v *View) sum(filter *Row, bitDepth uint) (sum, count uint64, err error) { // min returns the min and count of a field. func (v *View) min(filter *Row, bitDepth uint) (min, count uint64, err error) { var minHasValue bool - for _, f := range v.Fragments() { + for _, f := range v.allFragments() { fmin, fcount, err := f.min(filter, bitDepth) if err != nil { return min, count, err @@ -400,7 +388,7 @@ func (v *View) min(filter *Row, bitDepth uint) (min, count uint64, err error) { // max returns the max and count of a field. func (v *View) max(filter *Row, bitDepth uint) (max, count uint64, err error) { - for _, f := range v.Fragments() { + for _, f := range v.allFragments() { fmax, fcount, err := f.max(filter, bitDepth) if err != nil { return max, count, err @@ -416,7 +404,7 @@ func (v *View) max(filter *Row, bitDepth uint) (max, count uint64, err error) { // rangeOp returns rows with a field value encoding matching the predicate. func (v *View) rangeOp(op pql.Token, bitDepth uint, predicate uint64) (*Row, error) { r := NewRow() - for _, frag := range v.Fragments() { + for _, frag := range v.allFragments() { other, err := frag.rangeOp(op, bitDepth, predicate) if err != nil { return nil, err @@ -430,7 +418,7 @@ func (v *View) rangeOp(op pql.Token, bitDepth uint, predicate uint64) (*Row, err // value between predicateMin and predicateMax. func (v *View) rangeBetween(bitDepth uint, predicateMin, predicateMax uint64) (*Row, error) { r := NewRow() - for _, frag := range v.Fragments() { + for _, frag := range v.allFragments() { other, err := frag.rangeBetween(bitDepth, predicateMin, predicateMax) if err != nil { return nil, err diff --git a/view_internal_test.go b/view_internal_test.go new file mode 100644 index 000000000..d0e8bfdd1 --- /dev/null +++ b/view_internal_test.go @@ -0,0 +1,68 @@ +// Copyright 2017 Pilosa Corp. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pilosa + +import ( + "io/ioutil" + "testing" +) + +// mustOpenView returns a new instance of View with a temporary path. +func mustOpenView(index, field, name string) *View { + path, err := ioutil.TempDir("", "pilosa-view-") + if err != nil { + panic(err) + } + + v := NewView(path, index, field, name, DefaultCacheSize) + if err := v.open(); err != nil { + panic(err) + } + v.RowAttrStore = newMemAttrStore() + return v +} + +// Ensure view can open and retrieve a fragment. +func TestView_DeleteFragment(t *testing.T) { + v := mustOpenView("i", "f", "v") + defer v.close() + + slice := uint64(9) + + // Create fragment. + fragment, err := v.CreateFragmentIfNotExists(slice) + if err != nil { + t.Fatal(err) + } else if fragment == nil { + t.Fatal("expected fragment") + } + + err = v.deleteFragment(slice) + if err != nil { + t.Fatal(err) + } + + if v.Fragment(slice) != nil { + t.Fatal("fragment still exists in view") + } + + // Recreate fragment with same slice, verify that the old fragment was not reused. + fragment2, err := v.CreateFragmentIfNotExists(slice) + if err != nil { + t.Fatal(err) + } else if fragment == fragment2 { + t.Fatal("failed to create new fragment") + } +} diff --git a/view_test.go b/view_test.go deleted file mode 100644 index 04921786d..000000000 --- a/view_test.go +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright 2017 Pilosa Corp. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package pilosa_test - -import ( - "io/ioutil" - "os" - "testing" - - "github.com/pilosa/pilosa" - "github.com/pilosa/pilosa/test" -) - -// View is a test wrapper for pilosa.View. -type View struct { - *pilosa.View - RowAttrStore pilosa.AttrStore -} - -// NewView returns a new instance of View with a temporary path. -func NewView(index, field, name string) *View { - path, err := ioutil.TempDir("", "pilosa-view-") - if err != nil { - panic(err) - } - - v := &View{ - View: pilosa.NewView(path, index, field, name, pilosa.DefaultCacheSize), - RowAttrStore: test.MustOpenAttrStore(), - } - v.View.RowAttrStore = v.RowAttrStore - return v -} - -// MustOpenView creates and opens an view at a temporary path. Panic on error. -func MustOpenView(index, field, name string) *View { - v := NewView(index, field, name) - if err := v.Open(); err != nil { - panic(err) - } - return v -} - -// Close closes the view and removes all underlying data. -func (v *View) Close() error { - defer os.Remove(v.Path()) - defer v.RowAttrStore.Close() - return v.View.Close() -} - -// Reopen closes the view and reopens it as a new instance. -func (v *View) Reopen() error { - path := v.Path() - if err := v.View.Close(); err != nil { - return err - } - - v.View = pilosa.NewView(path, v.Index(), v.Field(), v.Name(), pilosa.DefaultCacheSize) - v.View.RowAttrStore = v.RowAttrStore - return v.Open() -} - -// MustClearColumns clears columns on a row. Panic on error. -func (v *View) MustClearBits(rowID uint64, columnIDs ...uint64) { - for _, columnID := range columnIDs { - if _, err := v.ClearBit(rowID, columnID); err != nil { - panic(err) - } - } -} - -// Ensure view can open and retrieve a fragment. -func TestView_DeleteFragment(t *testing.T) { - v := MustOpenView("i", "f", "v") - defer v.Close() - - slice := uint64(9) - - // Create fragment. - fragment, err := v.CreateFragmentIfNotExists(slice) - if err != nil { - t.Fatal(err) - } else if fragment == nil { - t.Fatal("expected fragment") - } - - err = v.DeleteFragment(slice) - if err != nil { - t.Fatal(err) - } - - if v.Fragment(slice) != nil { - t.Fatal("fragment still exists in view") - } - - // Recreate fragment with same slice, verify that the old fragment was not reused. - fragment2, err := v.CreateFragmentIfNotExists(slice) - if err != nil { - t.Fatal(err) - } else if fragment == fragment2 { - t.Fatal("failed to create new fragment") - } -}