From 2dfa689d71d2d06b7f81a6e96bf433fea2fe276b Mon Sep 17 00:00:00 2001 From: Michael Baird Date: Tue, 22 Aug 2017 14:52:07 -0500 Subject: [PATCH] Fragment.Close now returns an error. This required changing the Holder test utility Reopen, since in those cases we needed to close the file before making permission changes that Close() was failing silently on. --- fragment.go | 2 ++ frame.go | 10 +++++++--- holder.go | 4 +++- holder_test.go | 25 ++++++++++++++++++++++++- index.go | 4 +++- test/holder.go | 6 +++--- view.go | 4 +++- 7 files changed, 45 insertions(+), 10 deletions(-) diff --git a/fragment.go b/fragment.go index a517cbd57..bd3ebdad2 100644 --- a/fragment.go +++ b/fragment.go @@ -293,11 +293,13 @@ func (f *Fragment) close() error { // Flush cache if closing gracefully. if err := f.flushCache(); err != nil { f.logger().Printf("fragment: error flushing cache on close: err=%s, path=%s", err, f.path) + return err } // Close underlying storage. if err := f.closeStorage(); err != nil { f.logger().Printf("fragment: error closing storage: err=%s, path=%s", err, f.path) + return err } // Remove checksums. diff --git a/frame.go b/frame.go index 0b393a5c7..ad44cfb56 100644 --- a/frame.go +++ b/frame.go @@ -401,7 +401,9 @@ func (f *Frame) Close() error { // Close all views. for _, view := range f.views { - _ = view.Close() + if err := view.Close(); err != nil { + return err + } } f.views = make(map[string]*View) @@ -537,8 +539,10 @@ func (f *Frame) deleteView(name string) error { return ErrInvalidView } - // TODO capture errors lower down in this method - _ = view.Close() + // Close data files before deletion + if err := view.Close(); err != nil { + return err + } // Delete view directory. if err := os.RemoveAll(view.Path()); err != nil { diff --git a/holder.go b/holder.go index f7c363524..4270c2594 100644 --- a/holder.go +++ b/holder.go @@ -133,7 +133,9 @@ func (h *Holder) Close() error { h.wg.Wait() for _, index := range h.indexes { - index.Close() + if err := index.Close(); err != nil { + return err + } } return nil } diff --git a/holder_test.go b/holder_test.go index 108349bb2..b3947abb8 100644 --- a/holder_test.go +++ b/holder_test.go @@ -34,8 +34,9 @@ func TestHolder_Open(t *testing.T) { if err := os.Mkdir(h.IndexPath("!"), 0777); err != nil { t.Fatal(err) + } else if err := h.Holder.Close(); err != nil { + t.Fatal(err) } - if err := h.Reopen(); err != nil { t.Fatal(err) } else if logOutput := h.LogOutput.String(); !strings.Contains(logOutput, `ERROR opening index: !`) { @@ -49,6 +50,8 @@ func TestHolder_Open(t *testing.T) { if _, err := h.CreateIndex("test", pilosa.IndexOptions{}); err != nil { t.Fatal(err) + } else if err := h.Holder.Close(); err != nil { + t.Fatal(err) } else if err := os.Chmod(h.IndexPath("test"), 0000); err != nil { t.Fatal(err) } @@ -64,6 +67,8 @@ func TestHolder_Open(t *testing.T) { if _, err := h.CreateIndex("test", pilosa.IndexOptions{}); err != nil { t.Fatal(err) + } else if err := h.Holder.Close(); err != nil { + t.Fatal(err) } else if err := os.Truncate(filepath.Join(h.IndexPath("test"), ".meta"), 2); err != nil { t.Fatal(err) } @@ -78,6 +83,8 @@ func TestHolder_Open(t *testing.T) { if _, err := h.CreateIndex("test", pilosa.IndexOptions{}); err != nil { t.Fatal(err) + } else if err := h.Holder.Close(); err != nil { + t.Fatal(err) } else if err := os.Truncate(filepath.Join(h.IndexPath("test"), ".data"), 2); err != nil { t.Fatal(err) } @@ -95,6 +102,8 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if _, err := idx.CreateFrame("bar", pilosa.FrameOptions{}); err != nil { t.Fatal(err) + } else if err := h.Holder.Close(); err != nil { + t.Fatal(err) } else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar"), 0000); err != nil { t.Fatal(err) } @@ -112,6 +121,8 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if _, err := idx.CreateFrame("bar", pilosa.FrameOptions{}); err != nil { t.Fatal(err) + } else if err := h.Holder.Close(); err != nil { + t.Fatal(err) } else if err := os.Truncate(filepath.Join(h.Path, "foo", "bar", ".meta"), 2); err != nil { t.Fatal(err) } @@ -128,6 +139,8 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if _, err := idx.CreateFrame("bar", pilosa.FrameOptions{}); err != nil { t.Fatal(err) + } else if err := h.Holder.Close(); err != nil { + t.Fatal(err) } else if err := os.Truncate(filepath.Join(h.Path, "foo", "bar", ".data"), 2); err != nil { t.Fatal(err) } @@ -147,6 +160,8 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if _, err := frame.CreateViewIfNotExists(pilosa.ViewStandard); err != nil { t.Fatal(err) + } else if err := h.Holder.Close(); err != nil { + t.Fatal(err) } else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard"), 0000); err != nil { t.Fatal(err) } @@ -166,6 +181,8 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if _, err := frame.CreateViewIfNotExists(pilosa.ViewStandard); err != nil { t.Fatal(err) + } else if err := h.Holder.Close(); err != nil { + t.Fatal(err) } else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments"), 0000); err != nil { t.Fatal(err) } @@ -188,6 +205,8 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if _, err := view.SetBit(0, 0); err != nil { t.Fatal(err) + } else if err := h.Holder.Close(); err != nil { + t.Fatal(err) } else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments", "0"), 0000); err != nil { t.Fatal(err) } @@ -209,6 +228,8 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if _, err := view.SetBit(0, 0); err != nil { t.Fatal(err) + } else if err := h.Holder.Close(); err != nil { + t.Fatal(err) } else if err := os.Truncate(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments", "0"), 2); err != nil { t.Fatal(err) } @@ -232,6 +253,8 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if err := view.Fragment(0).FlushCache(); err != nil { t.Fatal(err) + } else if err := h.Holder.Close(); err != nil { + t.Fatal(err) } else if err := os.Chmod(filepath.Join(h.Path, "foo", "bar", "views", "standard", "fragments", "0.cache"), 0000); err != nil { t.Fatal(err) } diff --git a/index.go b/index.go index 8f1846c5f..345215690 100644 --- a/index.go +++ b/index.go @@ -248,7 +248,9 @@ func (i *Index) Close() error { // Close all frames. for _, f := range i.frames { - f.Close() + if err := f.Close(); err != nil { + return err + } } i.frames = make(map[string]*Frame) diff --git a/test/holder.go b/test/holder.go index 6bb346ec1..099b9cb7b 100644 --- a/test/holder.go +++ b/test/holder.go @@ -45,9 +45,9 @@ func (h *Holder) Close() error { // Reopen closes the holder and instantiates and opens a new holder. func (h *Holder) Reopen() error { - if err := h.Holder.Close(); err != nil { - return err - } + // if err := h.Holder.Close(); err != nil { + // return err + // } path, logOutput := h.Path, h.Holder.LogOutput h.Holder = pilosa.NewHolder() diff --git a/view.go b/view.go index 31a7a53c0..9fa5fcc8d 100644 --- a/view.go +++ b/view.go @@ -162,7 +162,9 @@ func (v *View) Close() error { // Close all fragments. for _, frag := range v.fragments { - _ = frag.Close() + if err := frag.Close(); err != nil { + return err + } } v.fragments = make(map[uint64]*Fragment)