From b8b2b5880406b97c7b107fd91848050eda3e3612 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Mon, 26 Mar 2018 10:59:48 -0500 Subject: [PATCH 01/58] add benchmark for import and snapshot --- fragment_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/fragment_test.go b/fragment_test.go index 8a54d3b07..392607bcc 100644 --- a/fragment_test.go +++ b/fragment_test.go @@ -1098,3 +1098,68 @@ func BenchmarkFragment_Snapshot(b *testing.B) { } } } +func BenchmarkFragment_FullSnapshot(b *testing.B) { + f := test.MustOpenFragment("i", "f", pilosa.ViewStandard, 0, "") + defer f.Close() + // Generate some intersecting data. + maxX := 1048576 / 2 + sz := maxX + rows := make([]uint64, sz, sz) + cols := make([]uint64, sz, sz) + + max := 0 + for row := 0; row < 100; row++ { + val := 1 + i := 0 + for col := 0; col < SliceWidth/2; col++ { + rows[i] = uint64(row) + cols[i] = uint64(val) + val += 2 + i++ + } + if err := f.Import(rows, cols); err != nil { + b.Fatalf("Error Building Sample: %s", err) + } + if row > max { + max = row + } + } + + b.ResetTimer() + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + if err := f.Snapshot(); err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkFragment_Import(b *testing.B) { + f := test.MustOpenFragment("i", "f", pilosa.ViewStandard, 0, "") + defer f.Close() + maxX := 1048576 * 5 * 2 + sz := maxX + rows := make([]uint64, sz, sz) + cols := make([]uint64, sz, sz) + i := 0 + for row := 0; row < 100; row++ { + val := 1 + for col := 0; col < SliceWidth/2; col++ { + rows[i] = uint64(row) + cols[i] = uint64(val) + val += 2 + i++ + } + if i == maxX { + break + } + } + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + if err := f.Import(rows, cols); err != nil { + b.Fatalf("Error Building Sample: %s", err) + } + } +} From f2729b90d7ba5e033a8e2aa13ac8804bf0d73379 Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Mon, 23 Apr 2018 10:43:06 -0500 Subject: [PATCH 02/58] vendor github.com/golang/groupcache/lru. rebuild Gopkg.lock --- Gopkg.lock | 58 +++++++++++++++++------ NOTICE | 36 +++++++++++++++ cache.go | 2 +- lru/lru.go | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 214 insertions(+), 15 deletions(-) create mode 100644 NOTICE create mode 100644 lru/lru.go diff --git a/Gopkg.lock b/Gopkg.lock index af6174c08..f29660b22 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -57,7 +57,10 @@ [[projects]] name = "github.com/go-ole/go-ole" - packages = [".","oleutil"] + packages = [ + ".", + "oleutil" + ] revision = "0e87ea779d9deb219633b828a023b32e1244dd57" version = "v1.2.0" @@ -67,12 +70,6 @@ revision = "100ba4e885062801d56799d78530b73b178a78f3" version = "v0.4" -[[projects]] - branch = "master" - name = "github.com/golang/groupcache" - packages = ["lru"] - revision = "84a468cf14b4376def5d68c722b139b881c450a4" - [[projects]] branch = "master" name = "github.com/golang/protobuf" @@ -130,7 +127,17 @@ [[projects]] branch = "master" name = "github.com/hashicorp/hcl" - packages = [".","hcl/ast","hcl/parser","hcl/scanner","hcl/strconv","hcl/token","json/parser","json/scanner","json/token"] + packages = [ + ".", + "hcl/ast", + "hcl/parser", + "hcl/scanner", + "hcl/strconv", + "hcl/token", + "json/parser", + "json/scanner", + "json/token" + ] revision = "23c074d0eceb2b8a5bfdbb271ab780cde70f05a8" [[projects]] @@ -154,7 +161,10 @@ [[projects]] branch = "master" name = "github.com/miekg/dns" - packages = [".","internal/socket"] + packages = [ + ".", + "internal/socket" + ] revision = "9fc4eb252eedf0ef8adc05169ce35da5e31beaba" [[projects]] @@ -195,7 +205,14 @@ [[projects]] name = "github.com/shirou/gopsutil" - packages = ["cpu","host","internal/common","mem","net","process"] + packages = [ + "cpu", + "host", + "internal/common", + "mem", + "net", + "process" + ] revision = "bfe3c2e8f406bf352bc8df81f98c752224867349" version = "v2.17.11" @@ -208,7 +225,10 @@ [[projects]] branch = "master" name = "github.com/spf13/afero" - packages = [".","mem"] + packages = [ + ".", + "mem" + ] revision = "5660eeed305fe5f69c8fc6cf899132a459a97064" [[projects]] @@ -256,13 +276,23 @@ [[projects]] branch = "master" name = "golang.org/x/sys" - packages = ["unix","windows"] + packages = [ + "unix", + "windows" + ] revision = "1e2299c37cc91a509f1b12369872d27be0ce98a6" [[projects]] branch = "master" name = "golang.org/x/text" - packages = ["internal/gen","internal/triegen","internal/ucd","transform","unicode/cldr","unicode/norm"] + packages = [ + "internal/gen", + "internal/triegen", + "internal/ucd", + "transform", + "unicode/cldr", + "unicode/norm" + ] revision = "88f656faf3f37f690df1a32515b479415e1a6769" [[projects]] @@ -274,6 +304,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "668c03e22d947dd7e24c4c5e07d7850ed0a2ea9a27d2af1fa0d1cff0e3743b0b" + inputs-digest = "61a00007fe7398d958fc8668543d42293f1310f9f9b74c63ff96f5a892eab1e7" solver-name = "gps-cdcl" solver-version = 1 diff --git a/NOTICE b/NOTICE new file mode 100644 index 000000000..7613b11e7 --- /dev/null +++ b/NOTICE @@ -0,0 +1,36 @@ +Software license +================ + +Copyright 2017 Pilosa Corp. + +Licensed under the Apache License, Version 2.0 (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. + + +Third-party software licenses +============================= + +The file /pilosa/lru/lru.go contains a redistribution of lru +(github.com/golang/groupcache/lru); the license follows: + + Copyright 2013 Google Inc. + + 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. diff --git a/cache.go b/cache.go index 9c909ced4..e27d2f93c 100644 --- a/cache.go +++ b/cache.go @@ -22,8 +22,8 @@ import ( "sync" "time" - "github.com/golang/groupcache/lru" "github.com/pilosa/pilosa/internal" + "github.com/pilosa/pilosa/lru" ) const ( diff --git a/lru/lru.go b/lru/lru.go new file mode 100644 index 000000000..532cc45e6 --- /dev/null +++ b/lru/lru.go @@ -0,0 +1,133 @@ +/* +Copyright 2013 Google Inc. + +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 lru implements an LRU cache. +package lru + +import "container/list" + +// Cache is an LRU cache. It is not safe for concurrent access. +type Cache struct { + // MaxEntries is the maximum number of cache entries before + // an item is evicted. Zero means no limit. + MaxEntries int + + // OnEvicted optionally specificies a callback function to be + // executed when an entry is purged from the cache. + OnEvicted func(key Key, value interface{}) + + ll *list.List + cache map[interface{}]*list.Element +} + +// A Key may be any value that is comparable. See http://golang.org/ref/spec#Comparison_operators +type Key interface{} + +type entry struct { + key Key + value interface{} +} + +// New creates a new Cache. +// If maxEntries is zero, the cache has no limit and it's assumed +// that eviction is done by the caller. +func New(maxEntries int) *Cache { + return &Cache{ + MaxEntries: maxEntries, + ll: list.New(), + cache: make(map[interface{}]*list.Element), + } +} + +// Add adds a value to the cache. +func (c *Cache) Add(key Key, value interface{}) { + if c.cache == nil { + c.cache = make(map[interface{}]*list.Element) + c.ll = list.New() + } + if ee, ok := c.cache[key]; ok { + c.ll.MoveToFront(ee) + ee.Value.(*entry).value = value + return + } + ele := c.ll.PushFront(&entry{key, value}) + c.cache[key] = ele + if c.MaxEntries != 0 && c.ll.Len() > c.MaxEntries { + c.RemoveOldest() + } +} + +// Get looks up a key's value from the cache. +func (c *Cache) Get(key Key) (value interface{}, ok bool) { + if c.cache == nil { + return + } + if ele, hit := c.cache[key]; hit { + c.ll.MoveToFront(ele) + return ele.Value.(*entry).value, true + } + return +} + +// Remove removes the provided key from the cache. +func (c *Cache) Remove(key Key) { + if c.cache == nil { + return + } + if ele, hit := c.cache[key]; hit { + c.removeElement(ele) + } +} + +// RemoveOldest removes the oldest item from the cache. +func (c *Cache) RemoveOldest() { + if c.cache == nil { + return + } + ele := c.ll.Back() + if ele != nil { + c.removeElement(ele) + } +} + +func (c *Cache) removeElement(e *list.Element) { + c.ll.Remove(e) + kv := e.Value.(*entry) + delete(c.cache, kv.key) + if c.OnEvicted != nil { + c.OnEvicted(kv.key, kv.value) + } +} + +// Len returns the number of items in the cache. +func (c *Cache) Len() int { + if c.cache == nil { + return 0 + } + return c.ll.Len() +} + +// Clear purges all stored items from the cache. +func (c *Cache) Clear() { + if c.OnEvicted != nil { + for _, e := range c.cache { + kv := e.Value.(*entry) + c.OnEvicted(kv.key, kv.value) + } + } + c.ll = nil + c.cache = nil +} From 7f41c0256ca49ed607608eb01237fda80c359bb9 Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Sun, 29 Apr 2018 16:40:32 -0500 Subject: [PATCH 03/58] avoid creating a slice of nil timestamps on Import() --- frame.go | 6 +++++- index.go | 8 +++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/frame.go b/frame.go index 0c87210bf..3de3f3613 100644 --- a/frame.go +++ b/frame.go @@ -834,7 +834,11 @@ func (f *Frame) Import(rowIDs, columnIDs []uint64, timestamps []*time.Time) erro // Split import data by fragment. dataByFragment := make(map[importKey]importData) for i := range rowIDs { - rowID, columnID, timestamp := rowIDs[i], columnIDs[i], timestamps[i] + rowID, columnID := rowIDs[i], columnIDs[i] + var timestamp *time.Time + if len(timestamps) > i { + timestamp = timestamps[i] + } var standard, inverse []string if timestamp == nil { diff --git a/index.go b/index.go index 00ee98f11..dc676c304 100644 --- a/index.go +++ b/index.go @@ -642,7 +642,8 @@ func (i *Index) openInputDefinitions() error { // InputBits Process the []Bit though the Frame import process func (i *Index) InputBits(frame string, bits []*Bit) error { var rowIDs, columnIDs []uint64 - timestamps := make([]*time.Time, len(bits)) + var timestamps []*time.Time + f := i.Frame(frame) if f == nil { return fmt.Errorf("Frame not found: %s", frame) @@ -657,6 +658,11 @@ func (i *Index) InputBits(frame string, bits []*Bit) error { // Convert timestamps to time.Time. if bit.Timestamp > 0 { + // Don't create a full timestamps slice unless + // at least one bit contains a timestamp. + if len(timestamps) == 0 { + timestamps = make([]*time.Time, len(bits)) + } t := time.Unix(bit.Timestamp, 0) timestamps[i] = &t } From 15f997c1379c0c3290fb7c6904a046a97633889f Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Mon, 30 Apr 2018 16:45:31 +0300 Subject: [PATCH 04/58] Added /info endpoint. Fixes #1232 --- api.go | 11 +++++++++++ handler.go | 8 ++++++++ handler_test.go | 15 +++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/api.go b/api.go index 700b21d25..1f33a6cde 100644 --- a/api.go +++ b/api.go @@ -1121,6 +1121,17 @@ func (api *API) Version() string { return strings.TrimPrefix(Version, "v") } +// Info returns information about this server instance +func (api *API) Info() ServerInfo { + return ServerInfo{ + SliceWidth: SliceWidth, + } +} + +type ServerInfo struct { + SliceWidth uint64 `json:"sliceWidth"` +} + type apiMethod int // API validation constants. diff --git a/handler.go b/handler.go index 7bf11af1b..871ba885a 100644 --- a/handler.go +++ b/handler.go @@ -126,6 +126,7 @@ func NewRouter(handler *Handler) *mux.Router { router.HandleFunc("/schema", handler.handleGetSchema).Methods("GET") router.HandleFunc("/slices/max", handler.handleGetSlicesMax).Methods("GET") // TODO: deprecate, but it's being used by the client (for backups) router.HandleFunc("/status", handler.handleGetStatus).Methods("GET") + router.HandleFunc("/info", handler.handleGetInfo).Methods("GET") router.HandleFunc("/version", handler.handleGetVersion).Methods("GET") router.HandleFunc("/cluster/resize/abort", handler.handlePostClusterResizeAbort).Methods("POST") @@ -252,6 +253,13 @@ func (h *Handler) handleGetStatus(w http.ResponseWriter, r *http.Request) { } } +func (h *Handler) handleGetInfo(w http.ResponseWriter, r *http.Request) { + info := h.API.Info() + if err := json.NewEncoder(w).Encode(info); err != nil { + h.Logger.Printf("write info response error: %s", err) + } +} + type getSchemaResponse struct { Indexes []*IndexInfo `json:"indexes"` } diff --git a/handler_test.go b/handler_test.go index 4187b3303..963c0cccc 100644 --- a/handler_test.go +++ b/handler_test.go @@ -19,6 +19,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "io" "io/ioutil" "net/http" @@ -154,6 +155,20 @@ func TestHandler_Status(t *testing.T) { } } +func TestHandler_Info(t *testing.T) { + s := test.NewServer() + defer s.Close() + h := test.NewHandler() + + w := httptest.NewRecorder() + h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/info", nil)) + if w.Code != http.StatusOK { + t.Fatalf("unexpected status code: %d", w.Code) + } else if body := w.Body.String(); body != fmt.Sprintf("{\"sliceWidth\":%d}\n", SliceWidth) { + t.Fatalf("unexpected body: %s", body) + } +} + // Ensure the handler can abort a cluster resize. func TestHandler_ClusterResizeAbort(t *testing.T) { From 5a56b33826691a027efe266dfd9ee69a32936374 Mon Sep 17 00:00:00 2001 From: Will Fitzgerald Date: Tue, 1 May 2018 05:45:19 -0400 Subject: [PATCH 05/58] fix minor spelling errors --- api.go | 2 +- gossip/gossip.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api.go b/api.go index 700b21d25..02ffa91dc 100644 --- a/api.go +++ b/api.go @@ -788,7 +788,7 @@ func (api *API) DeleteView(ctx context.Context, indexName string, frameName stri // Delete the view. if err := f.DeleteView(viewName); err != nil { - // Ignore this error becuase views do not exist on all nodes due to slice distribution. + // Ignore this error because views do not exist on all nodes due to slice distribution. if err != ErrInvalidView { return err } diff --git a/gossip/gossip.go b/gossip/gossip.go index e4856ec7b..25dd40cc4 100644 --- a/gossip/gossip.go +++ b/gossip/gossip.go @@ -443,7 +443,7 @@ type Transport struct { // NewTransport returns a NetTransport based on the given host and port. // It will dynamically bind to a port if port is 0. -// This is useful for test cases where specifiying a port is not reasonable. +// This is useful for test cases where specifying a port is not reasonable. //func NewTransport(host string, port int) (*memberlist.NetTransport, error) { func NewTransport(host string, port int, logger *log.Logger) (*Transport, error) { // memberlist config From 806437bd23a63183d345b7226477f7ddbd364a09 Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Wed, 2 May 2018 17:06:58 +0300 Subject: [PATCH 06/58] Make sure ~ is expanded in NewServer; BroadcastReceiver uses temp path --- broadcast_test.go | 7 ++++++- server.go | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/broadcast_test.go b/broadcast_test.go index 5654f7c28..970a249cb 100644 --- a/broadcast_test.go +++ b/broadcast_test.go @@ -56,9 +56,14 @@ func testMessageMarshal(t *testing.T, m proto.Message) { // Ensure that BroadcastReceiver can register a BroadcastHandler. func TestBroadcast_BroadcastReceiver(t *testing.T) { + path, err := ioutil.TempDir("", "pilosa-") + if err != nil { + panic(err) + } com := server.NewCommand(bytes.NewBuffer([]byte{}), ioutil.Discard, ioutil.Discard) com.Config.Bind = "localhost:0" - err := com.SetupServer() // this test shouldn't need to import pilosa/server just to set up the Server, but it really shouldn't need to setup the Server at all. The Server should not be the implementation of Broadcast* TODO + com.Config.DataDir = path + err = com.SetupServer() // this test shouldn't need to import pilosa/server just to set up the Server, but it really shouldn't need to setup the Server at all. The Server should not be the implementation of Broadcast* TODO if err != nil { t.Fatalf("setting up server: %v", err) } diff --git a/server.go b/server.go index 06260ae5d..46f8a0dda 100644 --- a/server.go +++ b/server.go @@ -21,6 +21,7 @@ import ( "net/http" "os" "os/exec" + "path/filepath" "runtime" "strconv" "strings" @@ -230,6 +231,11 @@ func NewServer(opts ...ServerOption) (*Server, error) { } } + err := s.expandDataDirName() + if err != nil { + return nil, err + } + s.Holder.Logger = s.logger s.Holder.Stats.SetLogger(s.logger) @@ -260,6 +266,18 @@ func NewServer(opts ...ServerOption) (*Server, error) { return s, nil } +func (s *Server) expandDataDirName() error { + prefix := "~" + string(filepath.Separator) + if strings.HasPrefix(s.Holder.Path, prefix) { + HomeDir := os.Getenv("HOME") + if HomeDir == "" { + return errors.New("data directory not specified and no home dir available") + } + s.Holder.Path = filepath.Join(HomeDir, strings.TrimPrefix(s.Holder.Path, prefix)) + } + return nil +} + // Open opens and initializes the server. func (s *Server) Open() error { s.logger.Printf("open server") From 244c4e894e875ee048c56fc04c7d525a81b0e72a Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Wed, 2 May 2018 17:33:51 +0300 Subject: [PATCH 07/58] Remove ~ expanding code from Command.Start --- server/server.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/server/server.go b/server/server.go index 8fd699ce5..3098cddeb 100644 --- a/server/server.go +++ b/server/server.go @@ -28,9 +28,7 @@ import ( "net/http" "os" "os/signal" - "path/filepath" "strconv" - "strings" "syscall" "time" @@ -93,14 +91,6 @@ func NewCommand(stdin io.Reader, stdout, stderr io.Writer) *Command { // Start starts the pilosa server - it returns once the server is running. func (m *Command) Start() (err error) { defer close(m.Started) - prefix := "~" + string(filepath.Separator) - if strings.HasPrefix(m.Config.DataDir, prefix) { - HomeDir := os.Getenv("HOME") - if HomeDir == "" { - return errors.New("data directory not specified and no home dir available") - } - m.Config.DataDir = filepath.Join(HomeDir, strings.TrimPrefix(m.Config.DataDir, prefix)) - } // SetupServer err = m.SetupServer() From 331a4fc5b8d2818acbfa92bd23e7c673fb8783ad Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Wed, 2 May 2018 12:21:03 -0500 Subject: [PATCH 08/58] add NOTICE to binary distributions --- Dockerfile | 2 ++ Makefile | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index bf076e526..404be937f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -11,6 +11,8 @@ LABEL maintainer "dev@pilosa.com" COPY --from=builder /go/bin/pilosa /pilosa +COPY NOTICE /NOTICE + EXPOSE 10101 VOLUME /data diff --git a/Makefile b/Makefile index cab932fba..e4d120659 100644 --- a/Makefile +++ b/Makefile @@ -42,7 +42,7 @@ build: vendor # Create a single release build under the build directory release-build: vendor $(MAKE) $(if $(DOCKER_BUILD),docker-)build FLAGS="-o build/pilosa-$(VERSION_ID)/pilosa" - cp LICENSE README.md build/pilosa-$(VERSION_ID) + cp NOTICE LICENSE README.md build/pilosa-$(VERSION_ID) tar -cvz -C build -f build/pilosa-$(VERSION_ID).tar.gz pilosa-$(VERSION_ID)/ @echo Created release build: build/pilosa-$(VERSION_ID).tar.gz From 18d7664fc43cf2148a6c6c15557ade6574a289b7 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Wed, 2 May 2018 15:04:12 -0500 Subject: [PATCH 09/58] Rename ID file to ".id" for consistency with existing data files. --- holder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/holder.go b/holder.go index 7649caeba..d0a242ca5 100644 --- a/holder.go +++ b/holder.go @@ -524,7 +524,7 @@ func (h *Holder) setFileLimit() { } func (h *Holder) loadNodeID() (string, error) { - idPath := path.Join(h.Path, "ID") + idPath := path.Join(h.Path, ".id") nodeID := "" h.Logger.Printf("load NodeID: %s", idPath) if err := os.MkdirAll(h.Path, 0777); err != nil { From beaa69a7bd6eaa38e3eb0e6ed4076eb8768fb165 Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Thu, 3 May 2018 00:07:56 +0300 Subject: [PATCH 10/58] Made expandDirName generic. --- server.go | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/server.go b/server.go index 46f8a0dda..86f14c8b3 100644 --- a/server.go +++ b/server.go @@ -231,14 +231,21 @@ func NewServer(opts ...ServerOption) (*Server, error) { } } - err := s.expandDataDirName() + path, err := expandDirName(s.Holder.Path) if err != nil { return nil, err } + s.Holder.Path = path s.Holder.Logger = s.logger s.Holder.Stats.SetLogger(s.logger) + path, err = expandDirName(s.Cluster.Path) + if err != nil { + return nil, err + } + s.Cluster.Path = path + s.Cluster.Logger = s.logger s.Cluster.Holder = s.Holder @@ -266,18 +273,6 @@ func NewServer(opts ...ServerOption) (*Server, error) { return s, nil } -func (s *Server) expandDataDirName() error { - prefix := "~" + string(filepath.Separator) - if strings.HasPrefix(s.Holder.Path, prefix) { - HomeDir := os.Getenv("HOME") - if HomeDir == "" { - return errors.New("data directory not specified and no home dir available") - } - s.Holder.Path = filepath.Join(HomeDir, strings.TrimPrefix(s.Holder.Path, prefix)) - } - return nil -} - // Open opens and initializes the server. func (s *Server) Open() error { s.logger.Printf("open server") @@ -796,3 +791,15 @@ type StatusHandler interface { ClusterStatus() (proto.Message, error) HandleRemoteStatus(proto.Message) error } + +func expandDirName(path string) (string, error) { + prefix := "~" + string(filepath.Separator) + if strings.HasPrefix(path, prefix) { + HomeDir := os.Getenv("HOME") + if HomeDir == "" { + return "", errors.New("data directory not specified and no home dir available") + } + return filepath.Join(HomeDir, strings.TrimPrefix(path, prefix)), nil + } + return path, nil +} From 85f90ac8fcf42a631c5490dba6500e2c37df419e Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Thu, 3 May 2018 16:11:43 -0500 Subject: [PATCH 11/58] Log time/version to startup log (Fixes #391) --- holder.go | 22 ++++++++++++++++++++++ server.go | 7 +++++++ 2 files changed, 29 insertions(+) diff --git a/holder.go b/holder.go index 7649caeba..49037d09f 100644 --- a/holder.go +++ b/holder.go @@ -547,6 +547,28 @@ func (h *Holder) loadNodeID() (string, error) { return nodeID, nil } +// Log startup time and version to $DATA_DIR/.startup.log +func (h *Holder) logStartup() error { + time, err := time.Now().MarshalText() + if err != nil { + return errors.Wrap(err, "creating timestamp") + } + logLine := fmt.Sprintf("%s\t%s\n", time, Version) + + f, err := os.OpenFile(h.Path+"/.startup.log", os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0600) + if err != nil { + return errors.Wrap(err, "opening startup log") + } + + defer f.Close() + + if _, err = f.WriteString(logLine); err != nil { + return errors.Wrap(err, "writing startup log") + } + + return nil +} + // HolderSyncer is an active anti-entropy tool that compares the local holder // with a remote holder based on block checksums and resolves differences. type HolderSyncer struct { diff --git a/server.go b/server.go index 06260ae5d..7617d7e69 100644 --- a/server.go +++ b/server.go @@ -17,6 +17,7 @@ package pilosa import ( "context" "fmt" + "log" "net" "net/http" "os" @@ -268,6 +269,12 @@ func (s *Server) Open() error { return errors.New("Must pass a listener option to NewServer") } + // Log startup + err := s.Holder.logStartup() + if err != nil { + log.Println(errors.Wrap(err, "logging startup")) + } + // Get or create NodeID. // Append the NodeID tag to stats. From e751945bf142cd53c80d2bfa930d00b945647e5f Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Fri, 4 May 2018 11:33:59 -0500 Subject: [PATCH 12/58] Add docs for upgrading Pilosa --- docs/administration.md | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/docs/administration.md b/docs/administration.md index 190b5ca1f..eee32e419 100644 --- a/docs/administration.md +++ b/docs/administration.md @@ -92,7 +92,41 @@ The Pilosa server should support PQL versioning using HTTP headers. On each requ #### Upgrading -When upgrading, upgrade clients first, followed by server for all Minor and Patch level changes. +To upgrade Pilosa: + +1. First, upgrade the [client libraries](../client-libraries/) you are using in your application. Generally, a client version `X` will be compatible with the Pilosa server version `X` and earlier. For example, `python-pilosa 0.9.0` is compatible with both `pilosa 0.8.0` and `pilosa 0.9.0`. +2. Next, download the latest release from our [installation page](/docs/latest/installation/) or from the [release page on Github](https://github.com/pilosa/pilosa/releases). +3. Shut down the Pilosa cluster. A graceful shutdown should ensure that all data files are flushed to disk. +4. Make a backup of the [data directory](../configuration/#data-dir) on each cluster node. +5. Upgrade the Pilosa server binaries and any configuration changes. See the following sections on any version-specific changes you must make. +6. Start Pilosa. It is recommended to start the cluster coordinator node first, followed by any other nodes. + +##### Version 0.9 + +Pilosa v0.9 introduces a few compatibility changes that need to be addressed. + +**Configuration changes**: These changes need to occur during step 5 above: + +1. Cluster-resize capability eliminates the `hosts` setting. Now, cluster membership is determined by `gossip`. This is only a factor if you are running Pilosa as a cluster. +2. Gossip-based cluster membership requires you to set a single cluster node as a [coordinator](../configuration/#cluster-coordinator). Make sure only a single node has the `cluster.coordinator` flag set. +3. `gossip.seed` has been renamed [`gossip.seeds`](../configuration/#gossip-seeds) and takes multiple items. It is recommended that at least two nodes are specified as gossip seeds. + +**Data directory changes**: These changes also need to occur during step 5 above (while the cluster is shut down): + +Pilosa v0.9 adds two new files to the data directory, an `.id` file and a `.topology` file. Due to the way Pilosa internally shards indices, upgrading a Pilosa cluster will result in data loss if an existing cluster is brought up without these files. New clusters will generate them automatically, but you may migrate an existing cluster by using a tool we called `topology-generator`: + +1. Observe the `cluster.hosts` configuration value in Pilosa v0.8. The ordering of the nodes in the config file is significant, as it determines shard (AKA slice) ownership. Pilosa v0.9 uses UUIDs for each node, and the ordering is alphabetical. +2. Install the `topology-generator`: `go get github.com/pilosa/upgrade-utils/v0.9/topology-generator`. +3. Run the `topology-generator`. There are two arguments: the number of nodes and the output directory. For this example, we'll assume a 3-node cluster and place the files in the current working directory: `topology-generator 3 .`. +4. This tool will generate a file, `topology`, and multiple id files, called `nodeX.id`, X being the node index position. +5. Copy the file `topology` into the data directories of every node in the cluster, naming it `.topology` (note the dot), e.g. `cp topology ~/.pilosa/.topology` or `scp topology node1:.pilosa/.topology`. +6. Copy the node ID files into the respective node data directories. For example, `node0.id` will be placed on the first node in the `cluster.hosts` list, with the name `.id`. For example: `scp node0.id node0:.pilosa/.id`. Again, it is very important that the ordering you give the nodes with these IDs matches the ordering you had in your existing `cluster.hosts` setting. + +**Application changes**: + +1. Row and column labels were deprecated in Pilosa v0.8, and removed in Pilosa v0.9. Make sure that your application does not attempt to use a custom row or column label, as they are no longer supported. +2. If your application relies on the implicit creation of [time quantums](../glossary/#time-quantum) by inheriting the time-quantum setting of the index, you must begin explicitly enabling the time quantum per-frame, as index-level time-quantums have been removed. +3. Inverse frames have been deprecated, removed from docs, and will be unsupported in the next release. ### Resizing the Cluster From 0afb95c1711b199501c282e731e82cbe9234fd66 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Fri, 4 May 2018 12:33:20 -0500 Subject: [PATCH 13/58] Minor upgrading docs changes --- docs/administration.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/administration.md b/docs/administration.md index eee32e419..0ecbbed94 100644 --- a/docs/administration.md +++ b/docs/administration.md @@ -96,7 +96,7 @@ To upgrade Pilosa: 1. First, upgrade the [client libraries](../client-libraries/) you are using in your application. Generally, a client version `X` will be compatible with the Pilosa server version `X` and earlier. For example, `python-pilosa 0.9.0` is compatible with both `pilosa 0.8.0` and `pilosa 0.9.0`. 2. Next, download the latest release from our [installation page](/docs/latest/installation/) or from the [release page on Github](https://github.com/pilosa/pilosa/releases). -3. Shut down the Pilosa cluster. A graceful shutdown should ensure that all data files are flushed to disk. +3. Shut down the Pilosa cluster. 4. Make a backup of the [data directory](../configuration/#data-dir) on each cluster node. 5. Upgrade the Pilosa server binaries and any configuration changes. See the following sections on any version-specific changes you must make. 6. Start Pilosa. It is recommended to start the cluster coordinator node first, followed by any other nodes. @@ -105,13 +105,13 @@ To upgrade Pilosa: Pilosa v0.9 introduces a few compatibility changes that need to be addressed. -**Configuration changes**: These changes need to occur during step 5 above: +**Configuration changes**: These changes need to occur before starting Pilosa v0.9: 1. Cluster-resize capability eliminates the `hosts` setting. Now, cluster membership is determined by `gossip`. This is only a factor if you are running Pilosa as a cluster. 2. Gossip-based cluster membership requires you to set a single cluster node as a [coordinator](../configuration/#cluster-coordinator). Make sure only a single node has the `cluster.coordinator` flag set. 3. `gossip.seed` has been renamed [`gossip.seeds`](../configuration/#gossip-seeds) and takes multiple items. It is recommended that at least two nodes are specified as gossip seeds. -**Data directory changes**: These changes also need to occur during step 5 above (while the cluster is shut down): +**Data directory changes**: These changes need to occur while the cluster is shut down, before starting Pilosa v0.9: Pilosa v0.9 adds two new files to the data directory, an `.id` file and a `.topology` file. Due to the way Pilosa internally shards indices, upgrading a Pilosa cluster will result in data loss if an existing cluster is brought up without these files. New clusters will generate them automatically, but you may migrate an existing cluster by using a tool we called `topology-generator`: From 992c3e8bdda48572b0cc3eb67ac56c6a6a7e714b Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Thu, 26 Apr 2018 08:39:56 -0500 Subject: [PATCH 14/58] WIP - adding more locking to Cluster --- api.go | 7 +- cluster.go | 23 ++- cluster_test.go | 170 ++++++++--------- server/cluster_test.go | 4 +- test/pilosa.go | 7 +- utils_test.go | 425 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 538 insertions(+), 98 deletions(-) create mode 100644 utils_test.go diff --git a/api.go b/api.go index 02ffa91dc..9f7356cb8 100644 --- a/api.go +++ b/api.go @@ -1053,8 +1053,8 @@ func (api *API) SetCoordinator(ctx context.Context, id string) (oldNode, newNode return nil, nil, errors.Wrap(err, "validate api method") } - oldNode = api.Cluster.nodeByID(api.Cluster.Coordinator) - newNode = api.Cluster.nodeByID(id) + oldNode = api.Cluster.NodeByID(api.Cluster.Coordinator) + newNode = api.Cluster.NodeByID(id) if newNode == nil { return nil, nil, errors.Wrap(ErrNodeIDNotExists, "getting new node") } @@ -1102,9 +1102,6 @@ func (api *API) ResizeAbort() error { return errors.Wrap(err, "validate api method") } - if !api.Cluster.IsCoordinator() { - return ErrNodeNotCoordinator - } err := api.Cluster.CompleteCurrentJob(ResizeJobStateAborted) return errors.Wrap(err, "complete current job") } diff --git a/cluster.go b/cluster.go index bbf6f799e..c9f5fd9a8 100644 --- a/cluster.go +++ b/cluster.go @@ -292,6 +292,12 @@ func (c *Cluster) CoordinatorNode() *Node { // IsCoordinator is true if this node is the coordinator. func (c *Cluster) IsCoordinator() bool { + c.mu.RLock() + defer c.mu.RUnlock() + return c.isCoordinator() +} + +func (c *Cluster) isCoordinator() bool { return c.Coordinator == c.Node.ID } @@ -300,6 +306,8 @@ func (c *Cluster) IsCoordinator() bool { // will consider itself coordinator and update the other // nodes with its version of Cluster.Status. func (c *Cluster) SetCoordinator(n *Node) error { + c.mu.Lock() + defer c.mu.Unlock() // Verify that the new Coordinator value matches // this node. if c.Node.ID != n.ID { @@ -511,6 +519,12 @@ func (c *Cluster) Status() *internal.ClusterStatus { } } +func (c *Cluster) NodeByID(id string) *Node { + c.mu.RLock() + defer c.mu.RUnlock() + return c.nodeByID(id) +} + // nodeByID returns a node reference by ID. func (c *Cluster) nodeByID(id string) *Node { for _, n := range c.Nodes { @@ -1186,6 +1200,9 @@ func (c *Cluster) generateResizeJobByAction(nodeAction nodeAction) (*ResizeJob, func (c *Cluster) CompleteCurrentJob(state string) error { c.mu.Lock() defer c.mu.Unlock() + if !c.isCoordinator() { + return ErrNodeNotCoordinator + } if c.currentJob == nil { return ErrResizeNotRunning } @@ -1797,9 +1814,11 @@ func (c *Cluster) nodeLeave(node *Node) error { } func (c *Cluster) MergeClusterStatus(cs *internal.ClusterStatus) error { + c.mu.Lock() + defer c.mu.Unlock() c.Logger.Printf("merge cluster status: %v", cs) // Ignore status updates from self (coordinator). - if c.IsCoordinator() { + if c.isCoordinator() { return nil } @@ -1836,7 +1855,7 @@ func (c *Cluster) MergeClusterStatus(cs *internal.ClusterStatus) error { } } - c.SetState(cs.State) + c.setState(cs.State) c.markAsJoined() diff --git a/cluster_test.go b/cluster_test.go index 6c5d41829..b09820ac8 100644 --- a/cluster_test.go +++ b/cluster_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package pilosa_test +package pilosa import ( "bytes" @@ -22,29 +22,27 @@ import ( "testing/quick" "github.com/davecgh/go-spew/spew" - "github.com/pilosa/pilosa" - "github.com/pilosa/pilosa/test" ) // Ensure the cluster can fairly distribute partitions across the nodes. func TestCluster_Owners(t *testing.T) { - c := pilosa.Cluster{ - Nodes: []*pilosa.Node{ - {URI: test.NewURIFromHostPort("serverA", 1000)}, - {URI: test.NewURIFromHostPort("serverB", 1000)}, - {URI: test.NewURIFromHostPort("serverC", 1000)}, + c := Cluster{ + Nodes: []*Node{ + {URI: NewTestURIFromHostPort("serverA", 1000)}, + {URI: NewTestURIFromHostPort("serverB", 1000)}, + {URI: NewTestURIFromHostPort("serverC", 1000)}, }, - Hasher: test.NewModHasher(), + Hasher: NewTestModHasher(), ReplicaN: 2, } // Verify nodes are distributed. - if a := c.PartitionNodes(0); !reflect.DeepEqual(a, []*pilosa.Node{c.Nodes[0], c.Nodes[1]}) { + if a := c.PartitionNodes(0); !reflect.DeepEqual(a, []*Node{c.Nodes[0], c.Nodes[1]}) { t.Fatalf("unexpected owners: %s", spew.Sdump(a)) } // Verify nodes go around the ring. - if a := c.PartitionNodes(2); !reflect.DeepEqual(a, []*pilosa.Node{c.Nodes[2], c.Nodes[0]}) { + if a := c.PartitionNodes(2); !reflect.DeepEqual(a, []*Node{c.Nodes[2], c.Nodes[0]}) { t.Fatalf("unexpected owners: %s", spew.Sdump(a)) } } @@ -52,7 +50,7 @@ func TestCluster_Owners(t *testing.T) { // Ensure the partitioner can assign a fragment to a partition. func TestCluster_Partition(t *testing.T) { if err := quick.Check(func(index string, slice uint64, partitionN int) bool { - c := pilosa.NewCluster() + c := NewCluster() c.PartitionN = partitionN partitionID := c.Partition(index, slice) @@ -85,7 +83,7 @@ func TestHasher(t *testing.T) { {0x0ddc0ffeebadf00d, []int{0, 1, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 15, 15, 15, 15}}, } { for i, v := range tt.bucket { - if got := pilosa.NewHasher().Hash(tt.key, i+1); got != v { + if got := NewHasher().Hash(tt.key, i+1); got != v { t.Errorf("hash(%v,%v)=%v, want %v", tt.key, i+1, got, v) } } @@ -94,8 +92,8 @@ func TestHasher(t *testing.T) { // Ensure OwnsSlices can find the actual slice list for node and index. func TestCluster_OwnsSlices(t *testing.T) { - c := test.NewCluster(5) - slices := c.OwnsSlices("test", 10, test.NewURIFromHostPort("host2", 0)) + c := NewTestCluster(5) + slices := c.OwnsSlices("test", 10, NewTestURIFromHostPort("host2", 0)) if !reflect.DeepEqual(slices, []uint64{0, 3, 6, 10}) { t.Fatalf("unexpected slices for node's index: %v", slices) @@ -104,7 +102,7 @@ func TestCluster_OwnsSlices(t *testing.T) { // Ensure ContainsSlices can find the actual slice list for node and index. func TestCluster_ContainsSlices(t *testing.T) { - c := test.NewCluster(5) + c := NewTestCluster(5) c.ReplicaN = 3 slices := c.ContainsSlices("test", 10, c.Nodes[2]) @@ -114,20 +112,20 @@ func TestCluster_ContainsSlices(t *testing.T) { } func TestCluster_Nodes(t *testing.T) { - uri0 := test.NewURIFromHostPort("node0", 0) - uri1 := test.NewURIFromHostPort("node1", 0) - uri2 := test.NewURIFromHostPort("node2", 0) - uri3 := test.NewURIFromHostPort("node3", 0) + uri0 := NewTestURIFromHostPort("node0", 0) + uri1 := NewTestURIFromHostPort("node1", 0) + uri2 := NewTestURIFromHostPort("node2", 0) + uri3 := NewTestURIFromHostPort("node3", 0) - node0 := &pilosa.Node{ID: "node0", URI: uri0} - node1 := &pilosa.Node{ID: "node1", URI: uri1} - node2 := &pilosa.Node{ID: "node2", URI: uri2} - node3 := &pilosa.Node{ID: "node3", URI: uri3} + node0 := &Node{ID: "node0", URI: uri0} + node1 := &Node{ID: "node1", URI: uri1} + node2 := &Node{ID: "node2", URI: uri2} + node3 := &Node{ID: "node3", URI: uri3} - nodes := []*pilosa.Node{node0, node1, node2} + nodes := []*Node{node0, node1, node2} t.Run("NodeIDs", func(t *testing.T) { - actual := pilosa.Nodes(nodes).IDs() + actual := Nodes(nodes).IDs() expected := []string{node0.ID, node1.ID, node2.ID} if !reflect.DeepEqual(actual, expected) { t.Errorf("expected: %v, but got: %v", expected, actual) @@ -135,24 +133,24 @@ func TestCluster_Nodes(t *testing.T) { }) t.Run("Filter", func(t *testing.T) { - actual := pilosa.Nodes(pilosa.Nodes(nodes).Filter(nodes[1])).URIs() - expected := []pilosa.URI{uri0, uri2} + actual := Nodes(Nodes(nodes).Filter(nodes[1])).URIs() + expected := []URI{uri0, uri2} if !reflect.DeepEqual(actual, expected) { t.Errorf("expected: %v, but got: %v", expected, actual) } }) t.Run("FilterURI", func(t *testing.T) { - actual := pilosa.Nodes(pilosa.Nodes(nodes).FilterURI(uri1)).URIs() - expected := []pilosa.URI{uri0, uri2} + actual := Nodes(Nodes(nodes).FilterURI(uri1)).URIs() + expected := []URI{uri0, uri2} if !reflect.DeepEqual(actual, expected) { t.Errorf("expected: %v, but got: %v", expected, actual) } }) t.Run("Contains", func(t *testing.T) { - actualTrue := pilosa.Nodes(nodes).Contains(node1) - actualFalse := pilosa.Nodes(nodes).Contains(node3) + actualTrue := Nodes(nodes).Contains(node1) + actualFalse := Nodes(nodes).Contains(node3) if !reflect.DeepEqual(actualTrue, true) { t.Errorf("expected: %v, but got: %v", true, actualTrue) } @@ -162,9 +160,9 @@ func TestCluster_Nodes(t *testing.T) { }) t.Run("Clone", func(t *testing.T) { - clone := pilosa.Nodes(nodes).Clone() - actual := pilosa.Nodes(clone).URIs() - expected := []pilosa.URI{uri0, uri1, uri2} + clone := Nodes(nodes).Clone() + actual := Nodes(clone).URIs() + expected := []URI{uri0, uri1, uri2} if !reflect.DeepEqual(actual, expected) { t.Errorf("expected: %v, but got: %v", expected, actual) } @@ -172,16 +170,16 @@ func TestCluster_Nodes(t *testing.T) { } func TestCluster_Coordinator(t *testing.T) { - uri1 := test.NewURIFromHostPort("node1", 0) - uri2 := test.NewURIFromHostPort("node2", 0) + uri1 := NewTestURIFromHostPort("node1", 0) + uri2 := NewTestURIFromHostPort("node2", 0) - node1 := &pilosa.Node{ID: "node1", URI: uri1} - node2 := &pilosa.Node{ID: "node2", URI: uri2} + node1 := &Node{ID: "node1", URI: uri1} + node2 := &Node{ID: "node2", URI: uri2} - c1 := *pilosa.NewCluster() + c1 := *NewCluster() c1.Node = node1 c1.Coordinator = node1.ID - c2 := *pilosa.NewCluster() + c2 := *NewCluster() c2.Node = node2 c2.Coordinator = node1.ID @@ -195,17 +193,17 @@ func TestCluster_Coordinator(t *testing.T) { } func TestCluster_Topology(t *testing.T) { - c1 := test.NewCluster(1) // automatically creates Node{ID: "node0"} + c1 := NewTestCluster(1) // automatically creates Node{ID: "node0"} - uri0 := test.NewURIFromHostPort("host0", 0) - uri1 := test.NewURIFromHostPort("host1", 0) - uri2 := test.NewURIFromHostPort("host2", 0) - invalid := test.NewURIFromHostPort("invalid", 0) + uri0 := NewTestURIFromHostPort("host0", 0) + uri1 := NewTestURIFromHostPort("host1", 0) + uri2 := NewTestURIFromHostPort("host2", 0) + invalid := NewTestURIFromHostPort("invalid", 0) - node0 := &pilosa.Node{ID: "node0", URI: uri0} - node1 := &pilosa.Node{ID: "node1", URI: uri1} - node2 := &pilosa.Node{ID: "node2", URI: uri2} - nodeinvalid := &pilosa.Node{ID: "nodeinvalid", URI: invalid} + node0 := &Node{ID: "node0", URI: uri0} + node1 := &Node{ID: "node1", URI: uri1} + node2 := &Node{ID: "node2", URI: uri2} + nodeinvalid := &Node{ID: "nodeinvalid", URI: invalid} t.Run("AddNode", func(t *testing.T) { err := c1.AddNode(node1) @@ -243,7 +241,7 @@ func TestCluster_Topology(t *testing.T) { func TestCluster_ResizeStates(t *testing.T) { t.Run("Single node, no data", func(t *testing.T) { - tc := test.NewTestCluster(1) + tc := NewClusterCluster(1) // Open TestCluster. if err := tc.Open(); err != nil { @@ -253,11 +251,11 @@ func TestCluster_ResizeStates(t *testing.T) { node := tc.Clusters[0] // Ensure that node comes up in state NORMAL. - if node.State() != pilosa.ClusterStateNormal { - t.Errorf("expected state: %v, but got: %v", pilosa.ClusterStateNormal, node.State()) + if node.State() != ClusterStateNormal { + t.Errorf("expected state: %v, but got: %v", ClusterStateNormal, node.State()) } - expectedTop := &pilosa.Topology{ + expectedTop := &Topology{ NodeIDs: []string{node.Node.ID}, } @@ -273,13 +271,13 @@ func TestCluster_ResizeStates(t *testing.T) { }) t.Run("Single node, in topology", func(t *testing.T) { - tc := test.NewTestCluster(0) + tc := NewClusterCluster(0) tc.AddNode(false) node := tc.Clusters[0] // write topology to data file - top := &pilosa.Topology{ + top := &Topology{ NodeIDs: []string{node.Node.ID}, } tc.WriteTopology(node.Path, top) @@ -290,8 +288,8 @@ func TestCluster_ResizeStates(t *testing.T) { } // Ensure that node comes up in state NORMAL. - if node.State() != pilosa.ClusterStateNormal { - t.Errorf("expected state: %v, but got: %v", pilosa.ClusterStateNormal, node.State()) + if node.State() != ClusterStateNormal { + t.Errorf("expected state: %v, but got: %v", ClusterStateNormal, node.State()) } // Close TestCluster. @@ -301,13 +299,13 @@ func TestCluster_ResizeStates(t *testing.T) { }) t.Run("Single node, not in topology", func(t *testing.T) { - tc := test.NewTestCluster(0) + tc := NewClusterCluster(0) tc.AddNode(false) node := tc.Clusters[0] // write topology to data file - top := &pilosa.Topology{ + top := &Topology{ NodeIDs: []string{"some-other-host"}, } tc.WriteTopology(node.Path, top) @@ -326,7 +324,7 @@ func TestCluster_ResizeStates(t *testing.T) { }) t.Run("Multiple nodes, no data", func(t *testing.T) { - tc := test.NewTestCluster(0) + tc := NewClusterCluster(0) tc.AddNode(false) // Open TestCluster. @@ -340,13 +338,13 @@ func TestCluster_ResizeStates(t *testing.T) { node1 := tc.Clusters[1] // Ensure that nodes comes up in state NORMAL. - if node0.State() != pilosa.ClusterStateNormal { - t.Errorf("expected node0 state: %v, but got: %v", pilosa.ClusterStateNormal, node0.State()) - } else if node1.State() != pilosa.ClusterStateNormal { - t.Errorf("expected node1 state: %v, but got: %v", pilosa.ClusterStateNormal, node1.State()) + if node0.State() != ClusterStateNormal { + t.Errorf("expected node0 state: %v, but got: %v", ClusterStateNormal, node0.State()) + } else if node1.State() != ClusterStateNormal { + t.Errorf("expected node1 state: %v, but got: %v", ClusterStateNormal, node1.State()) } - expectedTop := &pilosa.Topology{ + expectedTop := &Topology{ NodeIDs: []string{node0.Node.ID, node1.Node.ID}, } @@ -364,12 +362,12 @@ func TestCluster_ResizeStates(t *testing.T) { }) t.Run("Multiple nodes, in/not in topology", func(t *testing.T) { - tc := test.NewTestCluster(0) + tc := NewClusterCluster(0) tc.AddNode(false) node0 := tc.Clusters[0] // write topology to data file - top := &pilosa.Topology{ + top := &Topology{ NodeIDs: []string{"node0", "node2"}, } tc.WriteTopology(node0.Path, top) @@ -380,8 +378,8 @@ func TestCluster_ResizeStates(t *testing.T) { } // Ensure that node is in state STARTING before the other node joins. - if node0.State() != pilosa.ClusterStateStarting { - t.Errorf("expected node0 state: %v, but got: %v", pilosa.ClusterStateStarting, node0.State()) + if node0.State() != ClusterStateStarting { + t.Errorf("expected node0 state: %v, but got: %v", ClusterStateStarting, node0.State()) } // Expect an error by adding a node not in the topology. @@ -395,10 +393,10 @@ func TestCluster_ResizeStates(t *testing.T) { node2 := tc.Clusters[2] // Ensure that node comes up in state NORMAL. - if node0.State() != pilosa.ClusterStateNormal { - t.Errorf("expected node0 state: %v, but got: %v", pilosa.ClusterStateNormal, node0.State()) - } else if node2.State() != pilosa.ClusterStateNormal { - t.Errorf("expected node1 state: %v, but got: %v", pilosa.ClusterStateNormal, node2.State()) + if node0.State() != ClusterStateNormal { + t.Errorf("expected node0 state: %v, but got: %v", ClusterStateNormal, node0.State()) + } else if node2.State() != ClusterStateNormal { + t.Errorf("expected node1 state: %v, but got: %v", ClusterStateNormal, node2.State()) } // Close TestCluster. @@ -408,7 +406,7 @@ func TestCluster_ResizeStates(t *testing.T) { }) t.Run("Multiple nodes, with data", func(t *testing.T) { - tc := test.NewTestCluster(0) + tc := NewClusterCluster(0) tc.AddNode(false) node0 := tc.Clusters[0] @@ -418,20 +416,20 @@ func TestCluster_ResizeStates(t *testing.T) { } // Add Bit Data to node0. - if err := tc.CreateFrame("i", "f", pilosa.FrameOptions{}); err != nil { + if err := tc.CreateFrame("i", "f", FrameOptions{}); err != nil { t.Fatal(err) } tc.SetBit("i", "f", "standard", 1, 101, nil) tc.SetBit("i", "f", "standard", 1, 1300000, nil) // Add Field Data to node0. - if err := tc.CreateFrame("i", "fields", pilosa.FrameOptions{ + if err := tc.CreateFrame("i", "fields", FrameOptions{ InverseEnabled: false, - //CacheType: pilosa.CacheTypeNone, - Fields: []*pilosa.Field{ + //CacheType: CacheTypeNone, + Fields: []*Field{ { Name: "fld0", - Type: pilosa.FieldTypeInt, + Type: FieldTypeInt, Min: -100, Max: 100, }, @@ -461,13 +459,13 @@ func TestCluster_ResizeStates(t *testing.T) { node1 := tc.Clusters[1] // Ensure that nodes come up in state NORMAL. - if node0.State() != pilosa.ClusterStateNormal { - t.Errorf("expected node0 state: %v, but got: %v", pilosa.ClusterStateNormal, node0.State()) - } else if node1.State() != pilosa.ClusterStateNormal { - t.Errorf("expected node1 state: %v, but got: %v", pilosa.ClusterStateNormal, node1.State()) + if node0.State() != ClusterStateNormal { + t.Errorf("expected node0 state: %v, but got: %v", ClusterStateNormal, node0.State()) + } else if node1.State() != ClusterStateNormal { + t.Errorf("expected node1 state: %v, but got: %v", ClusterStateNormal, node1.State()) } - expectedTop := &pilosa.Topology{ + expectedTop := &Topology{ NodeIDs: []string{node0.Node.ID, node1.Node.ID}, } @@ -510,7 +508,7 @@ func TestCluster_ResizeStates(t *testing.T) { // Ensures that coordinator can be changed. func TestCluster_UpdateCoordinator(t *testing.T) { t.Run("UpdateCoordinator", func(t *testing.T) { - c := test.NewCluster(2) + c := NewTestCluster(2) oldNode := c.Nodes[0] newNode := c.Nodes[1] diff --git a/server/cluster_test.go b/server/cluster_test.go index 1a64ce305..326753a3d 100644 --- a/server/cluster_test.go +++ b/server/cluster_test.go @@ -396,7 +396,7 @@ func TestCluster_GossipMembership(t *testing.T) { var eg errgroup.Group eg.Go(func() error { // Pass invalid seed as first in list - _, err = m1.RunWithTransport("localhost", 0, []string{"http://localhost:8765", seed}) + _, err := m1.RunWithTransport("localhost", 0, []string{"http://localhost:8765", seed}) if err != nil { return err } @@ -409,7 +409,7 @@ func TestCluster_GossipMembership(t *testing.T) { eg.Go(func() error { // Pass invalid seed as last in list - _, err = m2.RunWithTransport("localhost", 0, []string{seed, "http://localhost:8765"}) + _, err := m2.RunWithTransport("localhost", 0, []string{seed, "http://localhost:8765"}) if err != nil { return err } diff --git a/test/pilosa.go b/test/pilosa.go index 24838d5de..f6e3a3457 100644 --- a/test/pilosa.go +++ b/test/pilosa.go @@ -66,7 +66,6 @@ func NewMain(opts ...MainOpt) *Main { m.Command.Stdin = &m.Stdin m.Command.Stdout = &m.Stdout m.Command.Stderr = &m.Stderr - for _, opt := range opts { err := opt(m) if err != nil { @@ -74,8 +73,10 @@ func NewMain(opts ...MainOpt) *Main { } } - - m.SetupServer() + err = m.SetupServer() + if err != nil { + panic(err) + } if testing.Verbose() { m.Command.Stdout = io.MultiWriter(os.Stdout, m.Command.Stdout) diff --git a/utils_test.go b/utils_test.go new file mode 100644 index 000000000..7bc17d424 --- /dev/null +++ b/utils_test.go @@ -0,0 +1,425 @@ +package pilosa + +import ( + "bufio" + "bytes" + "fmt" + "io/ioutil" + "path/filepath" + "sync" + "time" + + "github.com/gogo/protobuf/proto" + "github.com/pilosa/pilosa/internal" +) + +// NewCluster returns a cluster with n nodes and uses a mod-based hasher. +func NewTestCluster(n int) *Cluster { + path, err := ioutil.TempDir("", "pilosa-cluster-") + if err != nil { + panic(err) + } + + c := NewCluster() + c.ReplicaN = 1 + c.Hasher = NewTestModHasher() + c.Path = path + c.Topology = NewTopology() + + for i := 0; i < n; i++ { + c.Nodes = append(c.Nodes, &Node{ + ID: fmt.Sprintf("node%d", i), + URI: NewTestURI("http", fmt.Sprintf("host%d", i), uint16(0)), + }) + } + + c.Node = c.Nodes[0] + c.Coordinator = c.Nodes[0].ID + c.SetState(ClusterStateNormal) + + return c +} + +// NewURI is a test URI creator that intentionally swallows errors. +func NewTestURI(scheme, host string, port uint16) URI { + uri := DefaultURI() + uri.SetScheme(scheme) + uri.SetHost(host) + uri.SetPort(port) + return *uri +} + +func NewTestURIFromHostPort(host string, port uint16) URI { + uri := DefaultURI() + uri.SetHost(host) + uri.SetPort(port) + return *uri +} + +// ModHasher represents a simple, mod-based hashing. +type TestModHasher struct{} + +// NewModHasher returns a new instance of ModHasher with n buckets. +func NewTestModHasher() *TestModHasher { return &TestModHasher{} } + +func (*TestModHasher) Hash(key uint64, n int) int { return int(key) % n } + +// ClusterCluster represents a cluster of test nodes, each of which +// has a Cluster. +type ClusterCluster struct { + Clusters []*Cluster + + common *commonClusterSettings + + mu sync.RWMutex + resizing bool + resizeDone chan struct{} +} + +type commonClusterSettings struct { + Nodes []*Node +} + +func (t *ClusterCluster) CreateIndex(name string) error { + for _, c := range t.Clusters { + if _, err := c.Holder.CreateIndexIfNotExists(name, IndexOptions{}); err != nil { + return err + } + } + return nil +} + +func (t *ClusterCluster) CreateFrame(index, frame string, opt FrameOptions) error { + for _, c := range t.Clusters { + idx, err := c.Holder.CreateIndexIfNotExists(index, IndexOptions{}) + if err != nil { + return err + } + if _, err := idx.CreateFrame(frame, opt); err != nil { + return err + } + } + return nil +} +func (t *ClusterCluster) SetBit(index, frame, view string, rowID, colID uint64, x *time.Time) error { + // Determine which node should receive the SetBit. + c0 := t.Clusters[0] // use the first node's cluster to determine slice location. + slice := colID / SliceWidth + nodes := c0.SliceNodes(index, slice) + + for _, node := range nodes { + c := t.clusterByID(node.ID) + if c == nil { + continue + } + f := c.Holder.Frame(index, frame) + if f == nil { + return fmt.Errorf("index/frame does not exist: %s/%s", index, frame) + } + _, err := f.SetBit(view, rowID, colID, x) + if err != nil { + return err + } + } + + return nil +} + +func (t *ClusterCluster) SetFieldValue(index, frame string, columnID uint64, name string, value int64) error { + // Determine which node should receive the SetFieldValue. + c0 := t.Clusters[0] // use the first node's cluster to determine slice location. + slice := columnID / SliceWidth + nodes := c0.SliceNodes(index, slice) + + for _, node := range nodes { + c := t.clusterByID(node.ID) + if c == nil { + continue + } + f := c.Holder.Frame(index, frame) + if f == nil { + return fmt.Errorf("index/frame does not exist: %s/%s", index, frame) + } + _, err := f.SetFieldValue(columnID, name, value) + if err != nil { + return err + } + } + + return nil +} + +func (t *ClusterCluster) clusterByID(id string) *Cluster { + for _, c := range t.Clusters { + if c.Node.ID == id { + return c + } + } + return nil +} + +// AddNode adds a node to the cluster and (potentially) starts a resize job. +func (t *ClusterCluster) AddNode(saveTopology bool) error { + id := len(t.Clusters) + + c, err := t.addCluster(id, saveTopology) + if err != nil { + return err + } + + // Send NodeJoin event to coordinator. + if id > 0 { + coord := t.Clusters[0] + ev := &NodeEvent{ + Event: NodeJoin, + Node: c.Node, + } + + if err := coord.ReceiveEvent(ev); err != nil { + return err + } + + // Wait for the AddNode job to finish. + if c.State() != ClusterStateNormal { + t.resizeDone = make(chan struct{}) + t.mu.Lock() + t.resizing = true + t.mu.Unlock() + <-t.resizeDone + } + } + + return nil +} + +// WriteTopology writes the given topology to disk. +func (t *ClusterCluster) WriteTopology(path string, top *Topology) error { + if buf, err := proto.Marshal(top.Encode()); err != nil { + return err + } else if err := ioutil.WriteFile(filepath.Join(path, ".topology"), buf, 0666); err != nil { + return err + } + return nil +} + +func (t *ClusterCluster) addCluster(i int, saveTopology bool) (*Cluster, error) { + + id := fmt.Sprintf("node%d", i) + uri := NewTestURI("http", fmt.Sprintf("host%d", i), uint16(0)) + + node := &Node{ + ID: id, + URI: uri, + } + + // add URI to common + //t.common.NodeIDs = append(t.common.NodeIDs, id) + //sort.Sort(t.common.NodeIDs) + + // add node to common + t.common.Nodes = append(t.common.Nodes, node) + + // create node-specific temp directory + path, err := ioutil.TempDir("", fmt.Sprintf("pilosa-cluster-node-%d-", i)) + if err != nil { + return nil, err + } + + // holder + h := NewHolder() + h.Path = path + + // cluster + c := NewCluster() + c.ReplicaN = 1 + c.Hasher = NewTestModHasher() + c.Path = path + c.Topology = NewTopology() + c.Holder = h + c.MemberSet = NewStaticMemberSet(c.Nodes) + c.Node = node + c.Coordinator = t.common.Nodes[0].ID // the first node is the coordinator + c.Broadcaster = t + + // add nodes + if saveTopology { + for _, n := range t.common.Nodes { + c.AddNode(n) + } + } + + // Add this node to the ClusterCluster. + t.Clusters = append(t.Clusters, c) + + return c, nil +} + +// NewClusterCluster returns a new instance of test.Cluster. +func NewClusterCluster(n int) *ClusterCluster { + + tc := &ClusterCluster{ + common: &commonClusterSettings{}, + } + + // add clusters + for i := 0; i < n; i++ { + _, err := tc.addCluster(i, true) + if err != nil { + panic(err) + } + } + return tc +} + +// SetState sets the state of the cluster on each node. +func (t *ClusterCluster) SetState(state string) { + for _, c := range t.Clusters { + c.SetState(state) + } +} + +// Open opens all clusters in the test cluster. +func (t *ClusterCluster) Open() error { + for _, c := range t.Clusters { + if err := c.Open(); err != nil { + return err + } + if err := c.Holder.Open(); err != nil { + return err + } + if err := c.SetNodeState(NodeStateReady); err != nil { + return err + } + } + + // Start the listener on the coordinator. + if len(t.Clusters) == 0 { + return nil + } + t.Clusters[0].ListenForJoins() + + return nil +} + +// Close closes all clusters in the test cluster. +func (t *ClusterCluster) Close() error { + for _, c := range t.Clusters { + err := c.Close() + if err != nil { + return err + } + } + return nil +} + +// ClusterCluster implements Broadcaster interface. + +// SendSync is a test implemenetation of Broadcaster SendSync method. +func (t *ClusterCluster) SendSync(pb proto.Message) error { + switch obj := pb.(type) { + case *internal.ClusterStatus: + // Apply the send message to all nodes (except the coordinator). + for _, c := range t.Clusters { + c.MergeClusterStatus(obj) + } + t.mu.RLock() + if obj.State == ClusterStateNormal && t.resizing { + close(t.resizeDone) + } + t.mu.RUnlock() + } + + return nil +} + +// SendAsync is a test implemenetation of Broadcaster SendAsync method. +func (t *ClusterCluster) SendAsync(pb proto.Message) error { + return nil +} + +// SendTo is a test implemenetation of Broadcaster SendTo method. +func (t *ClusterCluster) SendTo(to *Node, pb proto.Message) error { + switch obj := pb.(type) { + case *internal.ResizeInstruction: + err := t.FollowResizeInstruction(obj) + if err != nil { + return err + } + case *internal.ResizeInstructionComplete: + coord := t.clusterByID(to.ID) + go coord.MarkResizeInstructionComplete(obj) + } + return nil +} + +// FollowResizeInstruction is a version of cluster.FollowResizeInstruction used for testing. +func (t *ClusterCluster) FollowResizeInstruction(instr *internal.ResizeInstruction) error { + + // Prepare the return message. + complete := &internal.ResizeInstructionComplete{ + JobID: instr.JobID, + Node: instr.Node, + Error: "", + } + + // Stop processing on any error. + if err := func() error { + + // figure out which node it was meant for, then call the operation on that cluster + // basically need to mimic this: client.RetrieveSliceFromURI(context.Background(), src.Index, src.Frame, src.View, src.Slice, srcURI) + instrNode := DecodeNode(instr.Node) + destCluster := t.clusterByID(instrNode.ID) + + // Sync the schema received in the resize instruction. + if err := destCluster.Holder.ApplySchema(instr.Schema); err != nil { + return err + } + + for _, src := range instr.Sources { + srcNode := DecodeNode(src.Node) + srcCluster := t.clusterByID(srcNode.ID) + + srcFragment := srcCluster.Holder.Fragment(src.Index, src.Frame, src.View, src.Slice) + destFragment := destCluster.Holder.Fragment(src.Index, src.Frame, src.View, src.Slice) + if destFragment == nil { + // Create fragment on destination if it doesn't exist. + f := destCluster.Holder.Frame(src.Index, src.Frame) + v := f.View(src.View) + var err error + destFragment, err = v.CreateFragmentIfNotExists(src.Slice) + if err != nil { + return err + } + } + + buf := bytes.NewBuffer(nil) + + bw := bufio.NewWriter(buf) + br := bufio.NewReader(buf) + + // Get the fragment from source. + if _, err := srcFragment.WriteTo(bw); err != nil { + return err + } + + // Flush the bufio.buf to the io.Writer (buf). + bw.Flush() + + // Write data to destination. + if _, err := destFragment.ReadFrom(br); err != nil { + return err + } + } + + return nil + }(); err != nil { + complete.Error = err.Error() + } + + node := DecodeNode(instr.Coordinator) + if err := t.SendTo(node, complete); err != nil { + return err + } + + return nil +} From 2ef78af0ca86af9336cf61d1b7d88ab7eec692ce Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Thu, 26 Apr 2018 10:42:50 -0500 Subject: [PATCH 15/58] adjusted protection around cluster coordinator mutations --- cluster.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cluster.go b/cluster.go index c9f5fd9a8..c7285d17a 100644 --- a/cluster.go +++ b/cluster.go @@ -315,7 +315,7 @@ func (c *Cluster) SetCoordinator(n *Node) error { } // Update IsCoordinator on all nodes (locally). - _ = c.UpdateCoordinator(n) + _ = c.updateCoordinator(n) // Send the update coordinator message to all nodes. err := c.Broadcaster.SendSync( @@ -335,6 +335,12 @@ func (c *Cluster) SetCoordinator(n *Node) error { // to true, and sets all other nodes to false. Returns true if the value // changed. func (c *Cluster) UpdateCoordinator(n *Node) bool { + c.mu.Lock() + defer c.mu.Unlock() + _ = c.updateCoordinator(n) +} + +func (c *Cluster) updateCoordinator(n *Node) bool { var changed bool if c.Coordinator != n.ID { c.Coordinator = n.ID From 4a1d15ec9dd235d39628488947ec1d337f23aca2 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Thu, 26 Apr 2018 11:43:39 -0500 Subject: [PATCH 16/58] missed a file on last commit --- cluster.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster.go b/cluster.go index c7285d17a..b02bd50d2 100644 --- a/cluster.go +++ b/cluster.go @@ -337,7 +337,7 @@ func (c *Cluster) SetCoordinator(n *Node) error { func (c *Cluster) UpdateCoordinator(n *Node) bool { c.mu.Lock() defer c.mu.Unlock() - _ = c.updateCoordinator(n) + return c.updateCoordinator(n) } func (c *Cluster) updateCoordinator(n *Node) bool { From 985b56f50b6f0f660b1f86d6810fb967d19c2d03 Mon Sep 17 00:00:00 2001 From: Matthew Jaffee Date: Fri, 27 Apr 2018 10:20:24 -0500 Subject: [PATCH 17/58] fix utils_test comments --- utils_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/utils_test.go b/utils_test.go index 7bc17d424..05afeac19 100644 --- a/utils_test.go +++ b/utils_test.go @@ -13,7 +13,7 @@ import ( "github.com/pilosa/pilosa/internal" ) -// NewCluster returns a cluster with n nodes and uses a mod-based hasher. +// NewTestCluster returns a cluster with n nodes and uses a mod-based hasher. func NewTestCluster(n int) *Cluster { path, err := ioutil.TempDir("", "pilosa-cluster-") if err != nil { @@ -40,7 +40,7 @@ func NewTestCluster(n int) *Cluster { return c } -// NewURI is a test URI creator that intentionally swallows errors. +// NewTestURI is a test URI creator that intentionally swallows errors. func NewTestURI(scheme, host string, port uint16) URI { uri := DefaultURI() uri.SetScheme(scheme) @@ -59,13 +59,14 @@ func NewTestURIFromHostPort(host string, port uint16) URI { // ModHasher represents a simple, mod-based hashing. type TestModHasher struct{} -// NewModHasher returns a new instance of ModHasher with n buckets. +// NewTestModHasher returns a new instance of ModHasher with n buckets. func NewTestModHasher() *TestModHasher { return &TestModHasher{} } func (*TestModHasher) Hash(key uint64, n int) int { return int(key) % n } // ClusterCluster represents a cluster of test nodes, each of which // has a Cluster. +// ClusterCluster implements Broadcaster interface. type ClusterCluster struct { Clusters []*Cluster @@ -101,6 +102,7 @@ func (t *ClusterCluster) CreateFrame(index, frame string, opt FrameOptions) erro } return nil } + func (t *ClusterCluster) SetBit(index, frame, view string, rowID, colID uint64, x *time.Time) error { // Determine which node should receive the SetBit. c0 := t.Clusters[0] // use the first node's cluster to determine slice location. @@ -312,8 +314,6 @@ func (t *ClusterCluster) Close() error { return nil } -// ClusterCluster implements Broadcaster interface. - // SendSync is a test implemenetation of Broadcaster SendSync method. func (t *ClusterCluster) SendSync(pb proto.Message) error { switch obj := pb.(type) { From 84ed1953bd60a8cdb5eb9324ac79d3ab68746eb7 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Wed, 2 May 2018 11:52:35 -0500 Subject: [PATCH 18/58] Update changelog --- CHANGELOG.md | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09562dc06..ed2baf7af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,9 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). -## Unreleased +## [v0.9.0] - 2018-05-02 + +This version contains 183 contribution from 12 contributors. There are 138 files changed, 17,084 insertions, and 7,482 deletions. ### Added @@ -23,7 +25,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Add local and cluster IDs ([#1013](https://github.com/pilosa/pilosa/pull/1013)) - Add HolderCleaner and view.DeleteFragment ([#985](https://github.com/pilosa/pilosa/pull/985)) - Add set-coordinator endpoint ([#963](https://github.com/pilosa/pilosa/pull/963)) -- Documentation improvements ([#1135](https://github.com/pilosa/pilosa/pull/1135), [#1154](https://github.com/pilosa/pilosa/pull/1154), [#1091](https://github.com/pilosa/pilosa/pull/1091), [#1108](https://github.com/pilosa/pilosa/pull/1108), [#1087](https://github.com/pilosa/pilosa/pull/1087), [#1086](https://github.com/pilosa/pilosa/pull/1086), [#1026](https://github.com/pilosa/pilosa/pull/1026), [#1022](https://github.com/pilosa/pilosa/pull/1022), [#1007](https://github.com/pilosa/pilosa/pull/1007), [#981](https://github.com/pilosa/pilosa/pull/981), [#901](https://github.com/pilosa/pilosa/pull/901), [#972](https://github.com/pilosa/pilosa/pull/972)) +- Implement Min/Max BSI queries ([#1191](https://github.com/pilosa/pilosa/pull/1191)) +- Documentation improvements ([#1135](https://github.com/pilosa/pilosa/pull/1135), [#1154](https://github.com/pilosa/pilosa/pull/1154), [#1091](https://github.com/pilosa/pilosa/pull/1091), [#1108](https://github.com/pilosa/pilosa/pull/1108), [#1087](https://github.com/pilosa/pilosa/pull/1087), [#1086](https://github.com/pilosa/pilosa/pull/1086), [#1026](https://github.com/pilosa/pilosa/pull/1026), [#1022](https://github.com/pilosa/pilosa/pull/1022), [#1007](https://github.com/pilosa/pilosa/pull/1007), [#981](https://github.com/pilosa/pilosa/pull/981), [#901](https://github.com/pilosa/pilosa/pull/901), [#972](https://github.com/pilosa/pilosa/pull/972)), ([#1215](https://github.com/pilosa/pilosa/pull/1215)), ([#1213](https://github.com/pilosa/pilosa/pull/1213)), ([#1224](https://github.com/pilosa/pilosa/pull/1224)) ### Changed @@ -42,12 +45,21 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Refactored HTTPClient handling ([#991](https://github.com/pilosa/pilosa/pull/991)) - Remove FrameSchema. Move Fields to the Frame struct ([#907](https://github.com/pilosa/pilosa/pull/907)) - Deprecated RangeEnabled option ([#1205](https://github.com/pilosa/pilosa/pull/1205)) +- Refactoring pilosa/server ([#1220](https://github.com/pilosa/pilosa/pull/1220)) +- Clean up flipBitmap and add tests ([#1223](https://github.com/pilosa/pilosa/pull/1223)) +- Move pilosa.Config to pilosa/server.Config ([#1216](https://github.com/pilosa/pilosa/pull/1216)) ### Removed - Remove the Gossip stutter from memberlist-related config options ([#1171](https://github.com/pilosa/pilosa/pull/1171)) - Remove old GossipPort and GossipSeed config options ([#1142](https://github.com/pilosa/pilosa/pull/1142)) - Remove cluster type `http` from docs ([#1130](https://github.com/pilosa/pilosa/pull/1130)) +- Remove holder.Peek, combine with HasData, move server logic ([#1226](https://github.com/pilosa/pilosa/pull/1226)) +- Remove PATCH frame endpoint ([#1222](https://github.com/pilosa/pilosa/pull/1222)) +- Remove Index.MergeSchemas() method ([#1219](https://github.com/pilosa/pilosa/pull/1219)) +- Remove references to Input Definition from the docs ([#1212](https://github.com/pilosa/pilosa/pull/1212)) +- Remove Index.TimeQuantum ([#1209](https://github.com/pilosa/pilosa/pull/1209)) +- Remove SecurityManager. Implement api restrictions in api package. ([#1207](https://github.com/pilosa/pilosa/pull/1207)) ### Fixed @@ -60,6 +72,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fix count/bitmap mismatch bug ([#1084](https://github.com/pilosa/pilosa/pull/1084)) - Fix edge case with Range() calls outside field Min/Max. Fixes #876. ([#979](https://github.com/pilosa/pilosa/pull/979)) - Bind the handler to all interfaces (0.0.0.0) in Dockerfile. Fixes #977. ([#980](https://github.com/pilosa/pilosa/pull/980)) +- Fix nil client bug in monitorAntiEntropy (and test) ([#1233](https://github.com/pilosa/pilosa/pull/1233)) +- Fix crash due to server.diagnostics.server not set ([#1229](https://github.com/pilosa/pilosa/pull/1229)) ### Performance From 46a929e1d64701e42d47088f2fe60e05911a1d94 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Fri, 4 May 2018 15:49:03 -0500 Subject: [PATCH 19/58] More updates for v0.9 branch --- CHANGELOG.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed2baf7af..f9671e748 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). -## [v0.9.0] - 2018-05-02 +## [v0.9.0] - 2018-05-04 This version contains 183 contribution from 12 contributors. There are 138 files changed, 17,084 insertions, and 7,482 deletions. @@ -22,11 +22,12 @@ This version contains 183 contribution from 12 contributors. There are 138 files - Modify `pilosa import` to support string rows/columns ([#1063](https://github.com/pilosa/pilosa/pull/1063)) - Add some statsd calls to HolderSyncer ([#1048](https://github.com/pilosa/pilosa/pull/1048)) - Adds support for memberlist gossip configuration via pilosa.Config ([#1014](https://github.com/pilosa/pilosa/pull/1014)) -- Add local and cluster IDs ([#1013](https://github.com/pilosa/pilosa/pull/1013)) +- Add local and cluster IDs ([#1013](https://github.com/pilosa/pilosa/pull/1013), [#1245](https://github.com/pilosa/pilosa/pull/1245)) - Add HolderCleaner and view.DeleteFragment ([#985](https://github.com/pilosa/pilosa/pull/985)) - Add set-coordinator endpoint ([#963](https://github.com/pilosa/pilosa/pull/963)) - Implement Min/Max BSI queries ([#1191](https://github.com/pilosa/pilosa/pull/1191)) -- Documentation improvements ([#1135](https://github.com/pilosa/pilosa/pull/1135), [#1154](https://github.com/pilosa/pilosa/pull/1154), [#1091](https://github.com/pilosa/pilosa/pull/1091), [#1108](https://github.com/pilosa/pilosa/pull/1108), [#1087](https://github.com/pilosa/pilosa/pull/1087), [#1086](https://github.com/pilosa/pilosa/pull/1086), [#1026](https://github.com/pilosa/pilosa/pull/1026), [#1022](https://github.com/pilosa/pilosa/pull/1022), [#1007](https://github.com/pilosa/pilosa/pull/1007), [#981](https://github.com/pilosa/pilosa/pull/981), [#901](https://github.com/pilosa/pilosa/pull/901), [#972](https://github.com/pilosa/pilosa/pull/972)), ([#1215](https://github.com/pilosa/pilosa/pull/1215)), ([#1213](https://github.com/pilosa/pilosa/pull/1213)), ([#1224](https://github.com/pilosa/pilosa/pull/1224)) +- Log time/version to startup log ([#1246](https://github.com/pilosa/pilosa/pull/1246)) +- Documentation improvements ([#1135](https://github.com/pilosa/pilosa/pull/1135), [#1154](https://github.com/pilosa/pilosa/pull/1154), [#1091](https://github.com/pilosa/pilosa/pull/1091), [#1108](https://github.com/pilosa/pilosa/pull/1108), [#1087](https://github.com/pilosa/pilosa/pull/1087), [#1086](https://github.com/pilosa/pilosa/pull/1086), [#1026](https://github.com/pilosa/pilosa/pull/1026), [#1022](https://github.com/pilosa/pilosa/pull/1022), [#1007](https://github.com/pilosa/pilosa/pull/1007), [#981](https://github.com/pilosa/pilosa/pull/981), [#901](https://github.com/pilosa/pilosa/pull/901), [#972](https://github.com/pilosa/pilosa/pull/972), [#1215](https://github.com/pilosa/pilosa/pull/1215), [#1213](https://github.com/pilosa/pilosa/pull/1213), [#1224](https://github.com/pilosa/pilosa/pull/1224), [#1250](https://github.com/pilosa/pilosa/pull/1250)) ### Changed @@ -48,6 +49,7 @@ This version contains 183 contribution from 12 contributors. There are 138 files - Refactoring pilosa/server ([#1220](https://github.com/pilosa/pilosa/pull/1220)) - Clean up flipBitmap and add tests ([#1223](https://github.com/pilosa/pilosa/pull/1223)) - Move pilosa.Config to pilosa/server.Config ([#1216](https://github.com/pilosa/pilosa/pull/1216)) +- Vendor github.com/golang/groupcache/lru ([#1221](https://github.com/pilosa/pilosa/pull/1221)) ### Removed @@ -74,6 +76,7 @@ This version contains 183 contribution from 12 contributors. There are 138 files - Bind the handler to all interfaces (0.0.0.0) in Dockerfile. Fixes #977. ([#980](https://github.com/pilosa/pilosa/pull/980)) - Fix nil client bug in monitorAntiEntropy (and test) ([#1233](https://github.com/pilosa/pilosa/pull/1233)) - Fix crash due to server.diagnostics.server not set ([#1229](https://github.com/pilosa/pilosa/pull/1229)) +- Fix some cluster race conditions ([#1228](https://github.com/pilosa/pilosa/pull/1228)) ### Performance From 995a90ac9d3b1f6578f19c77b99d917cd5357c6d Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Fri, 4 May 2018 16:22:00 -0500 Subject: [PATCH 20/58] Update pilosa version in installation docs --- docs/installation.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/installation.md b/docs/installation.md index db0828b2f..a0632c34c 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -74,19 +74,19 @@ There are four ways to install Pilosa on MacOS: Use [Homebrew](https://brew.sh/) 1. Download the latest release: ``` - curl -L -O https://github.com/pilosa/pilosa/releases/download/v0.8.3/pilosa-v0.8.3-darwin-amd64.tar.gz + curl -L -O https://github.com/pilosa/pilosa/releases/download/v0.9.0/pilosa-v0.9.0-darwin-amd64.tar.gz ``` Other releases can be downloaded from our Releases page on Github. 2. Extract the binary: ``` - tar xfz pilosa-v0.8.3-darwin-amd64.tar.gz + tar xfz pilosa-v0.9.0-darwin-amd64.tar.gz ``` 3. Move the binary into your PATH so you can run `pilosa` from any shell: ``` - cp -i pilosa-v0.8.3-darwin-amd64/pilosa /usr/local/bin + cp -i pilosa-v0.9.0-darwin-amd64/pilosa /usr/local/bin ``` 4. Make sure Pilosa is installed successfully: @@ -234,19 +234,19 @@ There are three ways to install Pilosa on Linux: download the binary (recommende 1. To install the latest version of Pilosa, download the latest release: ``` - curl -L -O https://github.com/pilosa/pilosa/releases/download/v0.8.3/pilosa-v0.8.3-linux-amd64.tar.gz + curl -L -O https://github.com/pilosa/pilosa/releases/download/v0.9.0/pilosa-v0.9.0-linux-amd64.tar.gz ``` Note: This assumes you are using an `amd64` compatible architecture. Other releases can be downloaded from our Releases page on Github. 2. Extract the binary: ``` - tar xfz pilosa-v0.8.3-linux-amd64.tar.gz + tar xfz pilosa-v0.9.0-linux-amd64.tar.gz ``` 3. Move the binary into your PATH so you can run `pilosa` from any shell: ``` - cp -i pilosa-v0.8.3-linux-amd64/pilosa /usr/local/bin + cp -i pilosa-v0.9.0-linux-amd64/pilosa /usr/local/bin ``` 4. Make sure Pilosa is installed successfully: From 3529043fbcc4df1c29b5026138f055e9fc05bf57 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Fri, 4 May 2018 16:22:08 -0500 Subject: [PATCH 21/58] Update Go version --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 404be937f..0392dbe87 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.10 as builder +FROM golang:1.10.2 as builder COPY . /go/src/github.com/pilosa/pilosa/ From 4454bedb3d14f1df9bc7a1c197a855f557f5e451 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Fri, 4 May 2018 16:28:19 -0500 Subject: [PATCH 22/58] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9671e748..2a0dc777c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [v0.9.0] - 2018-05-04 -This version contains 183 contribution from 12 contributors. There are 138 files changed, 17,084 insertions, and 7,482 deletions. +This version contains 188 contribution from 12 contributors. There are 141 files changed, 17,832 insertions, and 7,503 deletions. ### Added From 76ee46bd0d265eb4cd87e24797d9860a9b7bebf3 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Fri, 4 May 2018 17:03:43 -0500 Subject: [PATCH 23/58] Add note about upgrading to v0.9 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a0dc777c..32c84b835 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). This version contains 188 contribution from 12 contributors. There are 141 files changed, 17,832 insertions, and 7,503 deletions. +*Please see special [upgrading instructions](https://www.pilosa.com/docs/latest/administration/#version-0-9) for this release.* + ### Added - Add ability to dynamically resize clusters ([#982](https://github.com/pilosa/pilosa/pull/982), [#946](https://github.com/pilosa/pilosa/pull/946), [#929](https://github.com/pilosa/pilosa/pull/929), [#927](https://github.com/pilosa/pilosa/pull/927), [#917](https://github.com/pilosa/pilosa/pull/917), [#913](https://github.com/pilosa/pilosa/pull/913), [#912](https://github.com/pilosa/pilosa/pull/912), [#908](https://github.com/pilosa/pilosa/pull/908)) From 7cecfd30cf7c5354cc926f150c15ef2e9aa7f650 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Fri, 4 May 2018 17:07:39 -0500 Subject: [PATCH 24/58] Changelog fixes --- CHANGELOG.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32c84b835..5ab4ad1bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ This version contains 188 contribution from 12 contributors. There are 141 files - Add QueryResult.Type to protobuf message to distiguish results at the client ([#1064](https://github.com/pilosa/pilosa/pull/1064)) - Modify `pilosa import` to support string rows/columns ([#1063](https://github.com/pilosa/pilosa/pull/1063)) - Add some statsd calls to HolderSyncer ([#1048](https://github.com/pilosa/pilosa/pull/1048)) -- Adds support for memberlist gossip configuration via pilosa.Config ([#1014](https://github.com/pilosa/pilosa/pull/1014)) +- Add support for memberlist gossip configuration via pilosa.Config ([#1014](https://github.com/pilosa/pilosa/pull/1014)) - Add local and cluster IDs ([#1013](https://github.com/pilosa/pilosa/pull/1013), [#1245](https://github.com/pilosa/pilosa/pull/1245)) - Add HolderCleaner and view.DeleteFragment ([#985](https://github.com/pilosa/pilosa/pull/985)) - Add set-coordinator endpoint ([#963](https://github.com/pilosa/pilosa/pull/963)) @@ -45,10 +45,9 @@ This version contains 188 contribution from 12 contributors. There are 141 files - Use binary search in runAdd ([#1027](https://github.com/pilosa/pilosa/pull/1027)) - Use HTTP handler for gossip SendSync ([#1001](https://github.com/pilosa/pilosa/pull/1001)) - Group the write operations in syncBlock by MaxWritesPerRequest ([#950](https://github.com/pilosa/pilosa/pull/950)) -- Refactored HTTPClient handling ([#991](https://github.com/pilosa/pilosa/pull/991)) +- Refactor HTTPClient handling ([#991](https://github.com/pilosa/pilosa/pull/991)) - Remove FrameSchema. Move Fields to the Frame struct ([#907](https://github.com/pilosa/pilosa/pull/907)) -- Deprecated RangeEnabled option ([#1205](https://github.com/pilosa/pilosa/pull/1205)) -- Refactoring pilosa/server ([#1220](https://github.com/pilosa/pilosa/pull/1220)) +- Refactor pilosa/server ([#1220](https://github.com/pilosa/pilosa/pull/1220)) - Clean up flipBitmap and add tests ([#1223](https://github.com/pilosa/pilosa/pull/1223)) - Move pilosa.Config to pilosa/server.Config ([#1216](https://github.com/pilosa/pilosa/pull/1216)) - Vendor github.com/golang/groupcache/lru ([#1221](https://github.com/pilosa/pilosa/pull/1221)) @@ -80,6 +79,10 @@ This version contains 188 contribution from 12 contributors. There are 141 files - Fix crash due to server.diagnostics.server not set ([#1229](https://github.com/pilosa/pilosa/pull/1229)) - Fix some cluster race conditions ([#1228](https://github.com/pilosa/pilosa/pull/1228)) +### Deprecated + +- Deprecate RangeEnabled option ([#1205](https://github.com/pilosa/pilosa/pull/1205)) + ### Performance - Add benchmark for various container usage patterns ([#1017](https://github.com/pilosa/pilosa/pull/1017)) From b3f529cb1b08d8455b7a023c10e6b4db1a3de9ab Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Sun, 6 May 2018 19:44:36 -0700 Subject: [PATCH 25/58] remove unused NodeID method on InternalClient --- client.go | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/client.go b/client.go index c2e88c77b..eee1a3a9c 100644 --- a/client.go +++ b/client.go @@ -1142,30 +1142,6 @@ func (c *InternalHTTPClient) clientURI(ctx context.Context) *URI { return clientURI } -func (c *InternalHTTPClient) NodeID(uri *URI) (string, error) { - u := uriPathToURL(uri, "/id") - req, err := http.NewRequest("GET", u.String(), nil) - resp, err := c.HTTPClient.Do(req) - if err != nil { - return "", fmt.Errorf("executing http request: %v", err) - } - defer resp.Body.Close() - - // Read body. - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - return "", fmt.Errorf("reading response body: %v", err) - } - - // Return error if status is not OK. - switch resp.StatusCode { - case http.StatusOK: // ok - default: - return "", fmt.Errorf("unexpected response status code: %d: %s", resp.StatusCode, body) - } - return string(body), nil -} - // Bit represents the location of a single bit. type Bit struct { RowID uint64 @@ -1362,5 +1338,4 @@ type InternalClient interface { ColumnAttrDiff(ctx context.Context, index string, blks []AttrBlock) (map[uint64]map[string]interface{}, error) RowAttrDiff(ctx context.Context, index, frame string, blks []AttrBlock) (map[uint64]map[string]interface{}, error) SendMessage(ctx context.Context, pb proto.Message) error - NodeID(uri *URI) (string, error) } From 0d3df71e37b3a34cd31c1ff6c7723c5ac7e4d586 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Sun, 6 May 2018 20:12:23 -0700 Subject: [PATCH 26/58] add URI argument to InternalClient.SendMessage passing values through context is error prone and usually bad practice. --- client.go | 6 +++--- server.go | 7 +++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/client.go b/client.go index eee1a3a9c..ecfae5801 100644 --- a/client.go +++ b/client.go @@ -1098,13 +1098,13 @@ func (c *InternalHTTPClient) RowAttrDiff(ctx context.Context, index, frame strin } // SendMessage posts a message synchronously. -func (c *InternalHTTPClient) SendMessage(ctx context.Context, pb proto.Message) error { +func (c *InternalHTTPClient) SendMessage(ctx context.Context, uri *URI, pb proto.Message) error { msg, err := MarshalMessage(pb) if err != nil { return fmt.Errorf("marshaling message: %v", err) } - u := uriPathToURL(ctx.Value("uri").(*URI), "/cluster/message") + u := uriPathToURL(uri, "/cluster/message") req, err := http.NewRequest("POST", u.String(), bytes.NewReader(msg)) req.Header.Set("Content-Type", "application/x-protobuf") req.Header.Set("User-Agent", "pilosa/"+Version) @@ -1337,5 +1337,5 @@ type InternalClient interface { BlockData(ctx context.Context, index, frame, view string, slice uint64, block int) ([]uint64, []uint64, error) ColumnAttrDiff(ctx context.Context, index string, blks []AttrBlock) (map[uint64]map[string]interface{}, error) RowAttrDiff(ctx context.Context, index, frame string, blks []AttrBlock) (map[uint64]map[string]interface{}, error) - SendMessage(ctx context.Context, pb proto.Message) error + SendMessage(ctx context.Context, uri *URI, pb proto.Message) error } diff --git a/server.go b/server.go index 7617d7e69..52999b1c5 100644 --- a/server.go +++ b/server.go @@ -536,15 +536,15 @@ func (s *Server) ReceiveMessage(pb proto.Message) error { func (s *Server) SendSync(pb proto.Message) error { var eg errgroup.Group for _, node := range s.Cluster.Nodes { + node := node s.logger.Printf("SendSync to: %s", node.URI) // Don't forward the message to ourselves. if s.URI == node.URI { continue } - ctx := context.WithValue(context.Background(), "uri", &node.URI) eg.Go(func() error { - return s.defaultClient.SendMessage(ctx, pb) + return s.defaultClient.SendMessage(context.Background(), &node.URI, pb) }) } @@ -559,8 +559,7 @@ func (s *Server) SendAsync(pb proto.Message) error { // SendTo represents an implementation of Broadcaster. func (s *Server) SendTo(to *Node, pb proto.Message) error { s.logger.Printf("SendTo: %s", to.URI) - ctx := context.WithValue(context.Background(), "uri", &to.URI) - return s.defaultClient.SendMessage(ctx, pb) + return s.defaultClient.SendMessage(context.Background(), &to.URI, pb) } // Server implements StatusHandler. From 2738c922864edabfc3237511eb0364640f1cf10d Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Sun, 6 May 2018 20:30:58 -0700 Subject: [PATCH 27/58] stop passing uri via context to InternalClient.ExecuteQuery --- client.go | 14 ++++++++++---- client_test.go | 8 ++++---- ctl/bench.go | 2 +- executor.go | 3 +-- fragment.go | 2 +- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/client.go b/client.go index ecfae5801..8950e8406 100644 --- a/client.go +++ b/client.go @@ -223,8 +223,13 @@ func (c *InternalHTTPClient) FragmentNodes(ctx context.Context, index string, sl return a, nil } -// ExecuteQuery executes query against index on the server. -func (c *InternalHTTPClient) ExecuteQuery(ctx context.Context, index string, queryRequest *internal.QueryRequest) (*internal.QueryResponse, error) { +// QueryNode executes query against the index. +func (c *InternalHTTPClient) Query(ctx context.Context, index string, queryRequest *internal.QueryRequest) (*internal.QueryResponse, error) { + return c.QueryNode(ctx, c.defaultURI, index, queryRequest) +} + +// QueryNode executes query against the index, sending the request to the node specified. +func (c *InternalHTTPClient) QueryNode(ctx context.Context, uri *URI, index string, queryRequest *internal.QueryRequest) (*internal.QueryResponse, error) { if index == "" { return nil, ErrIndexRequired } else if queryRequest.Query == "" { @@ -238,7 +243,7 @@ func (c *InternalHTTPClient) ExecuteQuery(ctx context.Context, index string, que } // Create HTTP request. - u := c.clientURI(ctx).Path(fmt.Sprintf("/index/%s/query", index)) + u := uri.Path(fmt.Sprintf("/index/%s/query", index)) req, err := http.NewRequest("POST", u, bytes.NewReader(buf)) if err != nil { return nil, err @@ -1320,7 +1325,8 @@ type InternalClient interface { Schema(ctx context.Context) ([]*IndexInfo, error) CreateIndex(ctx context.Context, index string, opt IndexOptions) error FragmentNodes(ctx context.Context, index string, slice uint64) ([]*Node, error) - ExecuteQuery(ctx context.Context, index string, queryRequest *internal.QueryRequest) (*internal.QueryResponse, error) + Query(ctx context.Context, index string, queryRequest *internal.QueryRequest) (*internal.QueryResponse, error) + QueryNode(ctx context.Context, uri *URI, index string, queryRequest *internal.QueryRequest) (*internal.QueryResponse, error) Import(ctx context.Context, index, frame string, slice uint64, bits []Bit) error ImportK(ctx context.Context, index, frame string, bits []Bit) error EnsureIndex(ctx context.Context, name string, options IndexOptions) error diff --git a/client_test.go b/client_test.go index be3505acd..88dc3accb 100644 --- a/client_test.go +++ b/client_test.go @@ -150,7 +150,7 @@ func TestClient_MultiNode(t *testing.T) { Query: fmt.Sprintf(`TopN(frame="%s", n=%d)`, "f", topN), Remote: false, } - result, err := client[0].ExecuteQuery(context.Background(), "i", queryRequest) + result, err := client[0].Query(context.Background(), "i", queryRequest) if err != nil { t.Fatal(err) } @@ -168,7 +168,7 @@ func TestClient_MultiNode(t *testing.T) { hldr[1].Index("i").SetRemoteMaxSlice(maxSlice) hldr[2].Index("i").SetRemoteMaxSlice(maxSlice) - result, err = client[0].ExecuteQuery(context.Background(), "i", queryRequest) + result, err = client[0].Query(context.Background(), "i", queryRequest) if err != nil { t.Fatal(err) } @@ -188,11 +188,11 @@ func TestClient_MultiNode(t *testing.T) { t.Fatalf("Invalid TopN result set: %s", spew.Sdump(result)) } - result1, err := client[1].ExecuteQuery(context.Background(), "i", queryRequest) + result1, err := client[1].Query(context.Background(), "i", queryRequest) if err != nil { t.Fatal(err) } - result2, err := client[2].ExecuteQuery(context.Background(), "i", queryRequest) + result2, err := client[2].Query(context.Background(), "i", queryRequest) if err != nil { t.Fatal(err) } diff --git a/ctl/bench.go b/ctl/bench.go index 9e37fb704..b9743bb5e 100644 --- a/ctl/bench.go +++ b/ctl/bench.go @@ -95,7 +95,7 @@ func (cmd *BenchCommand) runSetBit(ctx context.Context, client pilosa.InternalCl Query: fmt.Sprintf(`SetBit(id=%d, frame="%s", columnID=%d)`, rowID, cmd.Frame, columnID), Remote: false, } - if _, err := client.ExecuteQuery(ctx, cmd.Index, queryRequest); err != nil { + if _, err := client.Query(ctx, cmd.Index, queryRequest); err != nil { return err } } diff --git a/executor.go b/executor.go index 62bc0e8c8..59e40511d 100644 --- a/executor.go +++ b/executor.go @@ -1490,8 +1490,7 @@ func (e *Executor) remoteExec(ctx context.Context, node *Node, index string, q * Remote: true, } - ctx = context.WithValue(ctx, "uri", node.URI) - pb, err := e.client.ExecuteQuery(ctx, index, pbreq) + pb, err := e.client.QueryNode(ctx, &node.URI, index, pbreq) if err != nil { return nil, err } diff --git a/fragment.go b/fragment.go index 448ba3202..7168f4443 100644 --- a/fragment.go +++ b/fragment.go @@ -1924,7 +1924,7 @@ func (s *FragmentSyncer) syncBlock(id int) error { Query: buffers[k].String(), Remote: true, } - _, err := clients[i].ExecuteQuery(context.Background(), f.Index(), queryRequest) + _, err := clients[i].Query(context.Background(), f.Index(), queryRequest) if err != nil { return err } From 420631e748925d8f3542fc734ab6b5f809629d83 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Sun, 6 May 2018 20:36:52 -0700 Subject: [PATCH 28/58] remove last vestiges of passing URI via context this chould be safe as context.WithValue doesn't seem to appear anywhere else in Pilosa --- client.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/client.go b/client.go index 8950e8406..314f7d4e2 100644 --- a/client.go +++ b/client.go @@ -88,7 +88,7 @@ func (c *InternalHTTPClient) MaxInverseSliceByIndex(ctx context.Context) (map[st // maxSliceByIndex returns the number of slices on a server by index. func (c *InternalHTTPClient) maxSliceByIndex(ctx context.Context, inverse bool) (map[string]uint64, error) { // Execute request against the host. - u := uriPathToURL(c.clientURI(ctx), "/slices/max") + u := uriPathToURL(c.defaultURI, "/slices/max") // Build request. req, err := http.NewRequest("GET", u.String(), nil) @@ -1137,16 +1137,6 @@ func (c *InternalHTTPClient) SendMessage(ctx context.Context, uri *URI, pb proto return nil } -func (c *InternalHTTPClient) clientURI(ctx context.Context) *URI { - clientURI := c.defaultURI - if contextURI, ok := ctx.Value("uri").(*URI); ok { - clientURI = contextURI - } else if contextURI, ok := ctx.Value("uri").(URI); ok { - clientURI = &contextURI - } - return clientURI -} - // Bit represents the location of a single bit. type Bit struct { RowID uint64 From b2eb8f02ee2395cfe6a5f3712ca2ea91bc0e7ee1 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Mon, 7 May 2018 12:42:39 -0700 Subject: [PATCH 29/58] fix QueryNode comment --- client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client.go b/client.go index 314f7d4e2..5997cf4ef 100644 --- a/client.go +++ b/client.go @@ -223,7 +223,7 @@ func (c *InternalHTTPClient) FragmentNodes(ctx context.Context, index string, sl return a, nil } -// QueryNode executes query against the index. +// Query executes query against the index. func (c *InternalHTTPClient) Query(ctx context.Context, index string, queryRequest *internal.QueryRequest) (*internal.QueryResponse, error) { return c.QueryNode(ctx, c.defaultURI, index, queryRequest) } From f5a4fd82b1d417ca05b3769b934002554be1b90a Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Tue, 8 May 2018 17:10:36 -0500 Subject: [PATCH 30/58] Wrap errors in ctl/*.go --- ctl/backup.go | 12 ++++++------ ctl/bench.go | 4 ++-- ctl/bench_test.go | 3 ++- ctl/check.go | 15 ++++++++------- ctl/check_test.go | 2 +- ctl/common.go | 5 +++-- ctl/export.go | 11 ++++++----- ctl/import.go | 24 ++++++++++++------------ ctl/inspect.go | 9 +++++---- ctl/restore.go | 8 ++++---- 10 files changed, 49 insertions(+), 44 deletions(-) diff --git a/ctl/backup.go b/ctl/backup.go index 1b16391e0..54a760695 100644 --- a/ctl/backup.go +++ b/ctl/backup.go @@ -16,12 +16,12 @@ package ctl import ( "context" - "errors" "io" "os" "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/server" + "github.com/pkg/errors" ) // BackupCommand represents a command for backing up a view. @@ -60,26 +60,26 @@ func (cmd *BackupCommand) Run(ctx context.Context) error { // Create a client to the server. client, err := CommandClient(cmd) if err != nil { - return err + return errors.Wrap(err, "creating client") } // Open output file. f, err := os.Create(cmd.Path) if err != nil { - return err + return errors.Wrap(err, "creating file") } defer f.Close() // Begin streaming backup. if err := client.BackupTo(ctx, f, cmd.Index, cmd.Frame, cmd.View); err != nil { - return err + return errors.Wrap(err, "backing up") } // Sync & close file to ensure durability. if err := f.Sync(); err != nil { - return err + return errors.Wrap(err, "syncing") } else if err = f.Close(); err != nil { - return err + return errors.Wrap(err, "closing file") } return nil diff --git a/ctl/bench.go b/ctl/bench.go index b9743bb5e..425894738 100644 --- a/ctl/bench.go +++ b/ctl/bench.go @@ -16,7 +16,6 @@ package ctl import ( "context" - "errors" "fmt" "io" "math/rand" @@ -25,6 +24,7 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/internal" "github.com/pilosa/pilosa/server" + "github.com/pkg/errors" ) // BenchCommand represents a command for benchmarking index operations. @@ -58,7 +58,7 @@ func (cmd *BenchCommand) Run(ctx context.Context) error { // Create a client to the server. client, err := CommandClient(cmd) if err != nil { - return err + return errors.Wrap(err, "creating client") } switch cmd.Op { diff --git a/ctl/bench_test.go b/ctl/bench_test.go index c3014e6ca..4790ccb44 100644 --- a/ctl/bench_test.go +++ b/ctl/bench_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/pilosa/pilosa" + "github.com/pkg/errors" ) func TestBenchCommand_InvalidOption(t *testing.T) { @@ -31,7 +32,7 @@ func TestBenchCommand_InvalidOption(t *testing.T) { cm := NewBenchCommand(stdin, stdout, stderr) err := cm.Run(context.Background()) - if err != pilosa.ErrHostRequired { + if errors.Cause(err) != pilosa.ErrHostRequired { t.Fatalf("Expect err: %s, actual err: %s", pilosa.ErrHostRequired, err) } diff --git a/ctl/check.go b/ctl/check.go index 34b7033fc..04f947eb4 100644 --- a/ctl/check.go +++ b/ctl/check.go @@ -24,6 +24,7 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/roaring" + "github.com/pkg/errors" ) // CheckCommand represents a command for performing consistency checks on data files. @@ -48,17 +49,17 @@ func (cmd *CheckCommand) Run(ctx context.Context) error { switch filepath.Ext(path) { case "": if err := cmd.checkBitmapFile(path); err != nil { - return err + return errors.Wrap(err, "checking bitmap") } case ".cache": if err := cmd.checkCacheFile(path); err != nil { - return err + return errors.Wrap(err, "checking cache") } case ".snapshotting": if err := cmd.checkSnapshotFile(path); err != nil { - return err + return errors.Wrap(err, "checking snapshot") } } } @@ -71,26 +72,26 @@ func (cmd *CheckCommand) checkBitmapFile(path string) error { // Open file handle. f, err := os.Open(path) if err != nil { - return err + return errors.Wrap(err, "opening file") } defer f.Close() fi, err := f.Stat() if err != nil { - return err + return errors.Wrap(err, "statting file") } // Memory map the file. data, err := syscall.Mmap(int(f.Fd()), 0, int(fi.Size()), syscall.PROT_READ, syscall.MAP_SHARED) if err != nil { - return err + return errors.Wrap(err, "mmapping") } defer syscall.Munmap(data) // Attach the mmap file to the bitmap. bm := roaring.NewBitmap() if err := bm.UnmarshalBinary(data); err != nil { - return err + return errors.Wrap(err, "unmarshalling") } // Perform consistency check. diff --git a/ctl/check_test.go b/ctl/check_test.go index 33a19feb3..56d70f97a 100644 --- a/ctl/check_test.go +++ b/ctl/check_test.go @@ -85,7 +85,7 @@ func TestCheckCommand_Run(t *testing.T) { var buf bytes.Buffer io.Copy(&buf, r) - if !strings.HasPrefix(err.Error(), "invalid roaring file") { + if !strings.HasPrefix(err.Error(), "checking bitmap: unmarshalling: invalid roaring file") { t.Fatalf("expect error: invalid roaring file, actual: '%s'", err) } // Todo: need correct roaring file for happy path diff --git a/ctl/common.go b/ctl/common.go index 3629e233e..11042a704 100644 --- a/ctl/common.go +++ b/ctl/common.go @@ -19,6 +19,7 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/server" + "github.com/pkg/errors" "github.com/spf13/pflag" ) @@ -42,7 +43,7 @@ func CommandClient(cmd CommandWithTLSSupport) (*pilosa.InternalHTTPClient, error if tlsConfig.CertificatePath != "" && tlsConfig.CertificateKeyPath != "" { cert, err := tls.LoadX509KeyPair(tlsConfig.CertificatePath, tlsConfig.CertificateKeyPath) if err != nil { - return nil, err + return nil, errors.Wrap(err, "loading keypair") } TLSConfig = &tls.Config{ Certificates: []tls.Certificate{cert}, @@ -51,7 +52,7 @@ func CommandClient(cmd CommandWithTLSSupport) (*pilosa.InternalHTTPClient, error } client, err := pilosa.NewInternalHTTPClient(cmd.TLSHost(), server.GetHTTPClient(TLSConfig)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting internal client") } return client, err } diff --git a/ctl/export.go b/ctl/export.go index 8c219a078..546d1ae1b 100644 --- a/ctl/export.go +++ b/ctl/export.go @@ -22,6 +22,7 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/server" + "github.com/pkg/errors" ) // ExportCommand represents a command for bulk exporting data from a server. @@ -68,7 +69,7 @@ func (cmd *ExportCommand) Run(ctx context.Context) error { if cmd.Path != "" { f, err := os.Create(cmd.Path) if err != nil { - return err + return errors.Wrap(err, "creating file") } defer f.Close() @@ -78,7 +79,7 @@ func (cmd *ExportCommand) Run(ctx context.Context) error { // Create a client to the server. client, err := CommandClient(cmd) if err != nil { - return err + return errors.Wrap(err, "creating client") } // Determine slice count. @@ -90,21 +91,21 @@ func (cmd *ExportCommand) Run(ctx context.Context) error { } if err != nil { - return err + return errors.Wrap(err, "getting slice count") } // Export each slice. for slice := uint64(0); slice <= maxSlices[cmd.Index]; slice++ { logger.Printf("exporting slice: %d", slice) if err := client.ExportCSV(ctx, cmd.Index, cmd.Frame, cmd.View, slice, w); err != nil { - return err + return errors.Wrap(err, "exporting") } } // Close writer, if applicable. if w, ok := w.(io.Closer); ok { if err := w.Close(); err != nil { - return err + return errors.Wrap(err, "closing") } } diff --git a/ctl/import.go b/ctl/import.go index c3c117eb3..004e47e96 100644 --- a/ctl/import.go +++ b/ctl/import.go @@ -17,7 +17,6 @@ package ctl import ( "context" "encoding/csv" - "errors" "fmt" "io" "log" @@ -28,6 +27,7 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/server" + "github.com/pkg/errors" ) // ImportCommand represents a command for bulk importing data. @@ -94,14 +94,14 @@ func (cmd *ImportCommand) Run(ctx context.Context) error { // Create a client to the server. client, err := CommandClient(cmd) if err != nil { - return err + return errors.Wrap(err, "creating client") } cmd.Client = client if cmd.CreateSchema { err := cmd.ensureSchema(ctx) if err != nil { - return err + return errors.Wrap(err, "ensuring schema") } } @@ -153,7 +153,7 @@ func (cmd *ImportCommand) bufferBits(ctx context.Context, path string) error { // Open file for reading. f, err := os.Open(path) if err != nil { - return err + return errors.Wrap(err, "opening file") } defer f.Close() @@ -173,7 +173,7 @@ func (cmd *ImportCommand) bufferBits(ctx context.Context, path string) error { if err == io.EOF { break } else if err != nil { - return err + return errors.Wrap(err, "reading") } // Ignore blank rows. @@ -243,7 +243,7 @@ func (cmd *ImportCommand) importBits(ctx context.Context, bits []pilosa.Bit) err logger.Printf("importing slice: %d, n=%d", slice, len(bits)) if err := cmd.Client.Import(ctx, cmd.Index, cmd.Frame, slice, bits); err != nil { - return err + return errors.Wrap(err, "importing") } } @@ -260,7 +260,7 @@ func (cmd *ImportCommand) bufferBitsK(ctx context.Context, path string) error { // Open file for reading. f, err := os.Open(path) if err != nil { - return err + return errors.Wrap(err, "opening file") } defer f.Close() @@ -280,7 +280,7 @@ func (cmd *ImportCommand) bufferBitsK(ctx context.Context, path string) error { if err == io.EOF { break } else if err != nil { - return err + return errors.Wrap(err, "reading") } // Ignore blank rows. @@ -340,7 +340,7 @@ func (cmd *ImportCommand) importBitsK(ctx context.Context, bits []pilosa.Bit) er logger.Printf("importing keys: n=%d", len(bits)) if err := cmd.Client.ImportK(ctx, cmd.Index, cmd.Frame, bits); err != nil { - return err + return errors.Wrap(err, "importing keys") } return nil @@ -356,7 +356,7 @@ func (cmd *ImportCommand) bufferFieldValues(ctx context.Context, path string) er // Open file for reading. f, err := os.Open(path) if err != nil { - return err + return errors.Wrap(err, "opening file") } defer f.Close() @@ -376,7 +376,7 @@ func (cmd *ImportCommand) bufferFieldValues(ctx context.Context, path string) er if err == io.EOF { break } else if err != nil { - return err + return errors.Wrap(err, "reading") } // Ignore blank rows. @@ -437,7 +437,7 @@ func (cmd *ImportCommand) importFieldValues(ctx context.Context, vals []pilosa.F logger.Printf("importing slice: %d, n=%d", slice, len(vals)) if err := cmd.Client.ImportValue(ctx, cmd.Index, cmd.Frame, cmd.Field, slice, vals); err != nil { - return err + return errors.Wrap(err, "importing values") } } diff --git a/ctl/inspect.go b/ctl/inspect.go index f5825c35a..0c38c3fb5 100644 --- a/ctl/inspect.go +++ b/ctl/inspect.go @@ -26,6 +26,7 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/roaring" + "github.com/pkg/errors" ) // InspectCommand represents a command for inspecting fragment data files. @@ -49,19 +50,19 @@ func (cmd *InspectCommand) Run(ctx context.Context) error { // Open file handle. f, err := os.Open(cmd.Path) if err != nil { - return err + return errors.Wrap(err, "opening file") } defer f.Close() fi, err := f.Stat() if err != nil { - return err + return errors.Wrap(err, "statting file") } // Memory map the file. data, err := syscall.Mmap(int(f.Fd()), 0, int(fi.Size()), syscall.PROT_READ, syscall.MAP_SHARED) if err != nil { - return err + return errors.Wrap(err, "mmapping") } defer syscall.Munmap(data) @@ -70,7 +71,7 @@ func (cmd *InspectCommand) Run(ctx context.Context) error { fmt.Fprintf(cmd.Stderr, "unmarshaling bitmap...") bm := roaring.NewBitmap() if err := bm.UnmarshalBinary(data); err != nil { - return err + return errors.Wrap(err, "unmarshalling") } fmt.Fprintf(cmd.Stderr, " (%s)\n", time.Since(t)) diff --git a/ctl/restore.go b/ctl/restore.go index 89ee8ac8d..ef863096e 100644 --- a/ctl/restore.go +++ b/ctl/restore.go @@ -16,12 +16,12 @@ package ctl import ( "context" - "errors" "io" "os" "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/server" + "github.com/pkg/errors" ) // RestoreCommand represents a command for restoring a frame from a backup. @@ -60,19 +60,19 @@ func (cmd *RestoreCommand) Run(ctx context.Context) error { // Create a client to the server. client, err := CommandClient(cmd) if err != nil { - return err + return errors.Wrap(err, "creating client") } // Open backup file. f, err := os.Open(cmd.Path) if err != nil { - return err + return errors.Wrap(err, "opening file") } defer f.Close() // Restore backup file to the cluster. if err := client.RestoreFrom(ctx, f, cmd.Index, cmd.Frame, cmd.View); err != nil { - return err + return errors.Wrap(err, "restoring") } return nil From f0b6fa4ee80d9e48040b54b1ef9fa63575107ba7 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Tue, 8 May 2018 17:20:06 -0500 Subject: [PATCH 31/58] Wrap errors in server/ --- server/server.go | 14 +++++++------- server/server_test.go | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/server/server.go b/server/server.go index 8fd699ce5..4038d12f2 100644 --- a/server/server.go +++ b/server/server.go @@ -105,18 +105,18 @@ func (m *Command) Start() (err error) { // SetupServer err = m.SetupServer() if err != nil { - return err + return errors.Wrap(err, "setting up server") } // SetupNetworking err = m.SetupNetworking() if err != nil { - return err + return errors.Wrap(err, "setting up networking") } // Initialize server. if err = m.Server.Open(); err != nil { - return fmt.Errorf("server.Open: %v", err) + return errors.Wrap(err, "opening server") } m.logger.Printf("Listening as %s\n", m.Server.URI) @@ -271,7 +271,7 @@ func (m *Command) SetupNetworking() error { for _, address := range m.Config.Cluster.Hosts { uri, err := pilosa.NewURIFromAddress(address) if err != nil { - return err + return errors.Wrap(err, "getting URI") } m.Server.Cluster.Nodes = append(m.Server.Cluster.Nodes, &pilosa.Node{ URI: *uri, @@ -287,7 +287,7 @@ func (m *Command) SetupNetworking() error { gossipPort, err := strconv.Atoi(m.Config.Gossip.Port) if err != nil { - return err + return errors.Wrap(err, "parsing port") } // get the host portion of addr to use for binding @@ -298,7 +298,7 @@ func (m *Command) SetupNetworking() error { } else { transport, err = gossip.NewTransport(gossipHost, gossipPort, m.logger.Logger()) if err != nil { - return err + return errors.Wrap(err, "getting transport") } } @@ -312,7 +312,7 @@ func (m *Command) SetupNetworking() error { m.Server.Cluster.EventReceiver = gossipEventReceiver gossipMemberSet, err := gossip.NewGossipMemberSet(m.Server.NodeID, m.Server.URI.Host(), m.Config.Gossip, gossipEventReceiver, m.Server, gossip.WithLogger(m.logger.Logger()), gossip.WithTransport(transport)) if err != nil { - return err + return errors.Wrap(err, "getting memberset") } m.Server.Cluster.MemberSet = gossipMemberSet m.Server.Broadcaster = m.Server diff --git a/server/server_test.go b/server/server_test.go index 75e0e2242..512ce5c11 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -32,6 +32,7 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/server" "github.com/pilosa/pilosa/test" + "github.com/pkg/errors" ) // Ensure program can process queries and maintain consistency. @@ -52,7 +53,7 @@ func TestMain_Set_Quick(t *testing.T) { // Execute SetBit() commands. for _, cmd := range cmds { - if err := client.CreateIndex(context.Background(), "i", pilosa.IndexOptions{}); err != nil && err != pilosa.ErrIndexExists { + if err := client.CreateIndex(context.Background(), "i", pilosa.IndexOptions{}); err != nil && errors.Cause(err) != pilosa.ErrIndexExists { t.Fatal(err) } if err := client.CreateFrame(context.Background(), "i", cmd.Frame, pilosa.FrameOptions{}); err != nil && err != pilosa.ErrFrameExists { From 0244f4b51d1a507f790ee90eabd66b9183201bb4 Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Wed, 9 May 2018 01:27:42 +0300 Subject: [PATCH 32/58] Added dataDir field to Server --- server.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/server.go b/server.go index 86f14c8b3..a74ae90f3 100644 --- a/server.go +++ b/server.go @@ -77,6 +77,7 @@ type Server struct { maxWritesPerRequest int defaultClient InternalClient + dataDir string } // ServerOption is a functional option type for pilosa.Server @@ -98,8 +99,7 @@ func OptServerReplicaN(n int) ServerOption { func OptServerDataDir(dir string) ServerOption { return func(s *Server) error { - s.Cluster.Path = dir - s.Holder.Path = dir + s.dataDir = dir return nil } } @@ -231,21 +231,16 @@ func NewServer(opts ...ServerOption) (*Server, error) { } } - path, err := expandDirName(s.Holder.Path) + path, err := expandDirName(s.dataDir) if err != nil { return nil, err } - s.Holder.Path = path + s.Holder.Path = path s.Holder.Logger = s.logger s.Holder.Stats.SetLogger(s.logger) - path, err = expandDirName(s.Cluster.Path) - if err != nil { - return nil, err - } s.Cluster.Path = path - s.Cluster.Logger = s.logger s.Cluster.Holder = s.Holder From 68924fe0adb043166f8aaf7ef393d6fa1403ce77 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Tue, 8 May 2018 17:40:15 -0500 Subject: [PATCH 33/58] Wrap errors in client.go --- client.go | 126 +++++++++++++++++++++++++++--------------------------- 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/client.go b/client.go index 5997cf4ef..7403a7680 100644 --- a/client.go +++ b/client.go @@ -19,7 +19,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "io" "io/ioutil" @@ -35,6 +34,7 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pilosa/pilosa/internal" + "github.com/pkg/errors" ) // ClientOptions represents the configuration for a InternalHTTPClient @@ -58,7 +58,7 @@ func NewInternalHTTPClient(host string, remoteClient *http.Client) (*InternalHTT uri, err := NewURIFromAddress(host) if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting URI") } client := NewInternalHTTPClientFromURI(uri, remoteClient) @@ -93,7 +93,7 @@ func (c *InternalHTTPClient) maxSliceByIndex(ctx context.Context, inverse bool) // Build request. req, err := http.NewRequest("GET", u.String(), nil) if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating request") } req.Header.Set("User-Agent", "pilosa/"+Version) @@ -101,7 +101,7 @@ func (c *InternalHTTPClient) maxSliceByIndex(ctx context.Context, inverse bool) // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "executing request") } defer resp.Body.Close() @@ -126,7 +126,7 @@ func (c *InternalHTTPClient) Schema(ctx context.Context) ([]*IndexInfo, error) { // Build request. req, err := http.NewRequest("GET", u, nil) if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating request") } req.Header.Set("User-Agent", "pilosa/"+Version) @@ -134,7 +134,7 @@ func (c *InternalHTTPClient) Schema(ctx context.Context) ([]*IndexInfo, error) { // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "executing request") } defer resp.Body.Close() @@ -154,14 +154,14 @@ func (c *InternalHTTPClient) CreateIndex(ctx context.Context, index string, opt Options: opt, }) if err != nil { - return err + return errors.Wrap(err, "encoding request") } // Create URL & HTTP request. u := uriPathToURL(c.defaultURI, fmt.Sprintf("/index/%s", index)) req, err := http.NewRequest("POST", u.String(), bytes.NewReader(buf)) if err != nil { - return err + return errors.Wrap(err, "creating request") } req.Header.Set("Content-Length", strconv.Itoa(len(buf))) req.Header.Set("Content-Type", "application/json") @@ -171,14 +171,14 @@ func (c *InternalHTTPClient) CreateIndex(ctx context.Context, index string, opt // Execute request against the host. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return err + return errors.Wrap(err, "executing request") } defer resp.Body.Close() // Read body. body, err := ioutil.ReadAll(resp.Body) if err != nil { - return err + return errors.Wrap(err, "reading") } // Handle response based on status code. @@ -201,7 +201,7 @@ func (c *InternalHTTPClient) FragmentNodes(ctx context.Context, index string, sl // Build request. req, err := http.NewRequest("GET", u.String(), nil) if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating request") } req.Header.Set("User-Agent", "pilosa/"+Version) @@ -209,7 +209,7 @@ func (c *InternalHTTPClient) FragmentNodes(ctx context.Context, index string, sl // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "executing request") } defer resp.Body.Close() @@ -239,14 +239,14 @@ func (c *InternalHTTPClient) QueryNode(ctx context.Context, uri *URI, index stri // Encode request object. buf, err := proto.Marshal(queryRequest) if err != nil { - return nil, err + return nil, errors.Wrap(err, "marshaling") } // Create HTTP request. u := uri.Path(fmt.Sprintf("/index/%s/query", index)) req, err := http.NewRequest("POST", u, bytes.NewReader(buf)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating request") } req.Header.Set("Content-Length", strconv.Itoa(len(buf))) @@ -257,14 +257,14 @@ func (c *InternalHTTPClient) QueryNode(ctx context.Context, uri *URI, index stri // Execute request against the host. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "executing request") } defer resp.Body.Close() // Read body and unmarshal response. body, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, err + return nil, errors.Wrap(err, "reading") } else if resp.StatusCode != http.StatusOK { return nil, errors.New(string(body)) } @@ -398,7 +398,7 @@ func (c *InternalHTTPClient) importNode(ctx context.Context, node *Node, buf []b u := nodePathToURL(node, "/import") req, err := http.NewRequest("POST", u.String(), bytes.NewReader(buf)) if err != nil { - return err + return errors.Wrap(err, "creating request") } req.Header.Set("Content-Length", strconv.Itoa(len(buf))) req.Header.Set("Content-Type", "application/x-protobuf") @@ -408,14 +408,14 @@ func (c *InternalHTTPClient) importNode(ctx context.Context, node *Node, buf []b // Execute request against the host. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return err + return errors.Wrap(err, "executing request") } defer resp.Body.Close() // Read body and unmarshal response. body, err := ioutil.ReadAll(resp.Body) if err != nil { - return err + return errors.Wrap(err, "reading") } else if resp.StatusCode != http.StatusOK { return errors.New(string(body)) } @@ -486,7 +486,7 @@ func (c *InternalHTTPClient) importValueNode(ctx context.Context, node *Node, bu u := nodePathToURL(node, "/import-value") req, err := http.NewRequest("POST", u.String(), bytes.NewReader(buf)) if err != nil { - return err + return errors.Wrap(err, "creating request") } req.Header.Set("Content-Length", strconv.Itoa(len(buf))) req.Header.Set("Content-Type", "application/x-protobuf") @@ -496,14 +496,14 @@ func (c *InternalHTTPClient) importValueNode(ctx context.Context, node *Node, bu // Execute request against the host. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return err + return errors.Wrap(err, "executing request") } defer resp.Body.Close() // Read body and unmarshal response. body, err := ioutil.ReadAll(resp.Body) if err != nil { - return err + return errors.Wrap(err, "reading") } else if resp.StatusCode != http.StatusOK { return errors.New(string(body)) } @@ -564,7 +564,7 @@ func (c *InternalHTTPClient) exportNodeCSV(ctx context.Context, node *Node, inde // Generate HTTP request. req, err := http.NewRequest("GET", u.String(), nil) if err != nil { - return err + return errors.Wrap(err, "creating request") } req.Header.Set("Accept", "text/csv") req.Header.Set("User-Agent", "pilosa/"+Version) @@ -572,7 +572,7 @@ func (c *InternalHTTPClient) exportNodeCSV(ctx context.Context, node *Node, inde // Execute request against the host. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return err + return errors.Wrap(err, "executing request") } defer resp.Body.Close() @@ -583,7 +583,7 @@ func (c *InternalHTTPClient) exportNodeCSV(ctx context.Context, node *Node, inde // Copy body to writer. if _, err := io.Copy(w, resp.Body); err != nil { - return err + return errors.Wrap(err, "copying") } return nil @@ -618,13 +618,13 @@ func (c *InternalHTTPClient) BackupTo(ctx context.Context, w io.Writer, index, f // Backup every slice to the tar file. for i := uint64(0); i <= maxSlices[index]; i++ { if err := c.backupSliceTo(ctx, tw, index, frame, view, i); err != nil { - return err + return errors.Wrap(err, "backing up slice") } } // Close tar file. if err := tw.Close(); err != nil { - return err + return errors.Wrap(err, "closing") } return nil @@ -644,9 +644,9 @@ func (c *InternalHTTPClient) backupSliceTo(ctx context.Context, tw *tar.Writer, // Read entire buffer to determine file size. data, err := ioutil.ReadAll(r) if err != nil { - return err + return errors.Wrap(err, "reading") } else if err := r.Close(); err != nil { - return err + return errors.Wrap(err, "closing") } // Write slice file header. @@ -656,12 +656,12 @@ func (c *InternalHTTPClient) backupSliceTo(ctx context.Context, tw *tar.Writer, Size: int64(len(data)), ModTime: time.Now(), }); err != nil { - return err + return errors.Wrap(err, "writing header") } // Write buffer to file. if _, err := tw.Write(data); err != nil { - return fmt.Errorf("write buffer: %s", err) + return errors.Wrap(err, "writing buffer") } return nil @@ -711,7 +711,7 @@ func (c *InternalHTTPClient) backupSliceNode(ctx context.Context, index, frame, // Build request. req, err := http.NewRequest("GET", u.String(), nil) if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating request") } req.Header.Set("User-Agent", "pilosa/"+Version) @@ -719,7 +719,7 @@ func (c *InternalHTTPClient) backupSliceNode(ctx context.Context, index, frame, // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "executing request") } // Return error if status is not OK. @@ -751,7 +751,7 @@ func (c *InternalHTTPClient) RestoreFrom(ctx context.Context, r io.Reader, index if err == io.EOF { return nil } else if err != nil { - return err + return errors.Wrap(err, "opening") } // Parse slice from entry name. @@ -763,12 +763,12 @@ func (c *InternalHTTPClient) RestoreFrom(ctx context.Context, r io.Reader, index // Read file into buffer. var buf bytes.Buffer if _, err := io.CopyN(&buf, tr, hdr.Size); err != nil { - return err + return errors.Wrap(err, "copying") } // Restore file to all nodes that own it. if err := c.restoreSliceFrom(ctx, buf.Bytes(), index, frame, view, slice); err != nil { - return err + return errors.Wrap(err, "restoring") } } } @@ -794,14 +794,14 @@ func (c *InternalHTTPClient) restoreSliceFrom(ctx context.Context, buf []byte, i // Build request. req, err := http.NewRequest("POST", u.String(), bytes.NewReader(buf)) if err != nil { - return err + return errors.Wrap(err, "creating request") } req.Header.Set("Content-Type", "application/octet-stream") req.Header.Set("User-Agent", "pilosa/"+Version) resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return err + return errors.Wrap(err, "executing request") } resp.Body.Close() @@ -825,14 +825,14 @@ func (c *InternalHTTPClient) CreateFrame(ctx context.Context, index, frame strin Options: opt, }) if err != nil { - return err + return errors.Wrap(err, "marshaling") } // Create URL & HTTP request. u := uriPathToURL(c.defaultURI, fmt.Sprintf("/index/%s/frame/%s", index, frame)) req, err := http.NewRequest("POST", u.String(), bytes.NewReader(buf)) if err != nil { - return err + return errors.Wrap(err, "creating request") } req.Header.Set("Content-Length", strconv.Itoa(len(buf))) req.Header.Set("Content-Type", "application/json") @@ -842,14 +842,14 @@ func (c *InternalHTTPClient) CreateFrame(ctx context.Context, index, frame strin // Execute request against the host. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return err + return errors.Wrap(err, "executing request") } defer resp.Body.Close() // Read body. body, err := ioutil.ReadAll(resp.Body) if err != nil { - return err + return errors.Wrap(err, "reading") } // Handle response based on status code. @@ -873,7 +873,7 @@ func (c *InternalHTTPClient) RestoreFrame(ctx context.Context, host, index, fram // Build request. req, err := http.NewRequest("POST", u.String(), nil) if err != nil { - return err + return errors.Wrap(err, "creating request") } req.Header.Set("Content-Type", "application/octet-stream") req.Header.Set("User-Agent", "pilosa/"+Version) @@ -881,7 +881,7 @@ func (c *InternalHTTPClient) RestoreFrame(ctx context.Context, host, index, fram // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return err + return errors.Wrap(err, "executing request") } resp.Body.Close() @@ -899,7 +899,7 @@ func (c *InternalHTTPClient) FrameViews(ctx context.Context, index, frame string u := uriPathToURL(c.defaultURI, fmt.Sprintf("/index/%s/frame/%s/views", index, frame)) req, err := http.NewRequest("GET", u.String(), nil) if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating request") } req.Header.Set("Accept", "application/json") req.Header.Set("User-Agent", "pilosa/"+Version) @@ -907,7 +907,7 @@ func (c *InternalHTTPClient) FrameViews(ctx context.Context, index, frame string // Execute request against the host. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "executing request") } defer resp.Body.Close() @@ -924,7 +924,7 @@ func (c *InternalHTTPClient) FrameViews(ctx context.Context, index, frame string // Decode response. var rsp getFrameViewsResponse if err := json.NewDecoder(resp.Body).Decode(&rsp); err != nil { - return nil, err + return nil, errors.Wrap(err, "decoding") } return rsp.Views, nil } @@ -943,7 +943,7 @@ func (c *InternalHTTPClient) FragmentBlocks(ctx context.Context, index, frame, v // Build request. req, err := http.NewRequest("GET", u.String(), nil) if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating request") } req.Header.Set("User-Agent", "pilosa/"+Version) @@ -951,7 +951,7 @@ func (c *InternalHTTPClient) FragmentBlocks(ctx context.Context, index, frame, v // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "executing request") } defer resp.Body.Close() @@ -967,7 +967,7 @@ func (c *InternalHTTPClient) FragmentBlocks(ctx context.Context, index, frame, v // Decode response object. var rsp getFragmentBlocksResponse if err := json.NewDecoder(resp.Body).Decode(&rsp); err != nil { - return nil, err + return nil, errors.Wrap(err, "decoding") } return rsp.Blocks, nil } @@ -982,13 +982,13 @@ func (c *InternalHTTPClient) BlockData(ctx context.Context, index, frame, view s Block: uint64(block), }) if err != nil { - return nil, nil, err + return nil, nil, errors.Wrap(err, "marshaling") } u := uriPathToURL(c.defaultURI, "/fragment/block/data") req, err := http.NewRequest("GET", u.String(), bytes.NewReader(buf)) if err != nil { - return nil, nil, err + return nil, nil, errors.Wrap(err, "creating request") } req.Header.Set("Content-Type", "application/protobuf") req.Header.Set("Content-Length", strconv.Itoa(len(buf))) @@ -997,7 +997,7 @@ func (c *InternalHTTPClient) BlockData(ctx context.Context, index, frame, view s resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return nil, nil, err + return nil, nil, errors.Wrap(err, "executing request") } defer resp.Body.Close() @@ -1013,9 +1013,9 @@ func (c *InternalHTTPClient) BlockData(ctx context.Context, index, frame, view s // Decode response object. var rsp internal.BlockDataResponse if body, err := ioutil.ReadAll(resp.Body); err != nil { - return nil, nil, err + return nil, nil, errors.Wrap(err, "reading") } else if err := proto.Unmarshal(body, &rsp); err != nil { - return nil, nil, err + return nil, nil, errors.Wrap(err, "unmarhsaling") } return rsp.RowIDs, rsp.ColumnIDs, nil } @@ -1027,13 +1027,13 @@ func (c *InternalHTTPClient) ColumnAttrDiff(ctx context.Context, index string, b // Encode request. buf, err := json.Marshal(postIndexAttrDiffRequest{Blocks: blks}) if err != nil { - return nil, err + return nil, errors.Wrap(err, "marshaling") } // Build request. req, err := http.NewRequest("POST", u.String(), bytes.NewReader(buf)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating request") } req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", "pilosa/"+Version) @@ -1041,7 +1041,7 @@ func (c *InternalHTTPClient) ColumnAttrDiff(ctx context.Context, index string, b // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "executing request") } defer resp.Body.Close() @@ -1055,7 +1055,7 @@ func (c *InternalHTTPClient) ColumnAttrDiff(ctx context.Context, index string, b // Decode response object. var rsp postIndexAttrDiffResponse if err := json.NewDecoder(resp.Body).Decode(&rsp); err != nil { - return nil, err + return nil, errors.Wrap(err, "decoding") } return rsp.Attrs, nil } @@ -1067,13 +1067,13 @@ func (c *InternalHTTPClient) RowAttrDiff(ctx context.Context, index, frame strin // Encode request. buf, err := json.Marshal(postFrameAttrDiffRequest{Blocks: blks}) if err != nil { - return nil, err + return nil, errors.Wrap(err, "marshaling") } // Build request. req, err := http.NewRequest("POST", u.String(), bytes.NewReader(buf)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating request") } req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", "pilosa/"+Version) @@ -1081,7 +1081,7 @@ func (c *InternalHTTPClient) RowAttrDiff(ctx context.Context, index, frame strin // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) if err != nil { - return nil, err + return nil, errors.Wrap(err, "executing request") } defer resp.Body.Close() @@ -1097,7 +1097,7 @@ func (c *InternalHTTPClient) RowAttrDiff(ctx context.Context, index, frame strin // Decode response object. var rsp postFrameAttrDiffResponse if err := json.NewDecoder(resp.Body).Decode(&rsp); err != nil { - return nil, err + return nil, errors.Wrap(err, "decoding") } return rsp.Attrs, nil } From f83911812173ea265f5de461d504bfc83eeb1975 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Tue, 8 May 2018 18:17:22 -0500 Subject: [PATCH 34/58] Wrap errors in executor --- executor.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/executor.go b/executor.go index 59e40511d..b6dc08d41 100644 --- a/executor.go +++ b/executor.go @@ -16,7 +16,6 @@ package pilosa import ( "context" - "errors" "fmt" "net/http" "sort" @@ -24,6 +23,7 @@ import ( "github.com/pilosa/pilosa/internal" "github.com/pilosa/pilosa/pql" + "github.com/pkg/errors" ) // DefaultFrame is the frame used if one is not specified. @@ -149,7 +149,7 @@ func (e *Executor) Execute(ctx context.Context, index string, q *pql.Query, slic // executeCall executes a call. func (e *Executor) executeCall(ctx context.Context, index string, c *pql.Call, slices []uint64, opt *ExecOptions) (interface{}, error) { if err := e.validateCallArgs(c); err != nil { - return nil, err + return nil, errors.Wrap(err, "validating args") } indexTag := fmt.Sprintf("index:%s", index) // Special handling for mutation and top-n calls. @@ -344,7 +344,7 @@ func (e *Executor) executeBitmapCall(ctx context.Context, index string, c *pql.C if columnID, ok, err := c.UintArg(columnLabel); ok && err == nil { attrs, err := idx.ColumnAttrStore().Attrs(columnID) if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting column attrs") } bm.Attrs = attrs } else if err != nil { @@ -354,11 +354,11 @@ func (e *Executor) executeBitmapCall(ctx context.Context, index string, c *pql.C if fr := idx.Frame(frame); fr != nil { rowID, _, err := c.UintArg(rowLabel) if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting row") } attrs, err := fr.RowAttrStore().Attrs(rowID) if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting row attrs") } bm.Attrs = attrs } @@ -400,7 +400,7 @@ func (e *Executor) executeSumCountSlice(ctx context.Context, index string, c *pq if len(c.Children) == 1 { bm, err := e.executeBitmapCallSlice(ctx, index, c.Children[0], slice) if err != nil { - return ValCount{}, err + return ValCount{}, errors.Wrap(err, "executing bitmap call") } filter = bm } @@ -425,7 +425,7 @@ func (e *Executor) executeSumCountSlice(ctx context.Context, index string, c *pq vsum, vcount, err := fragment.FieldSum(filter, field.BitDepth()) if err != nil { - return ValCount{}, err + return ValCount{}, errors.Wrap(err, "computing sum") } return ValCount{ Val: int64(vsum) + (int64(vcount) * field.Min), @@ -527,7 +527,7 @@ func (e *Executor) executeTopN(ctx context.Context, index string, c *pql.Call, s // Execute original query. pairs, err := e.executeTopNSlices(ctx, index, c, slices, opt) if err != nil { - return nil, err + return nil, errors.Wrap(err, "finding top results") } // If this call is against specific ids, or we didn't get results, @@ -544,7 +544,7 @@ func (e *Executor) executeTopN(ctx context.Context, index string, c *pql.Call, s trimmedList, err := e.executeTopNSlices(ctx, index, other, slices, opt) if err != nil { - return nil, err + return nil, errors.Wrap(err, "retrieving full counts") } if n != 0 && int(n) < len(trimmedList) { @@ -883,7 +883,7 @@ func (e *Executor) executeFieldRangeSlice(ctx context.Context, index string, c * predicates, err := cond.IntSliceValue() if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting condition value") } // Only support two integers for the between operation. @@ -1573,7 +1573,7 @@ func (e *Executor) mapReduce(ctx context.Context, index string, slices []uint64, // Start mapping across all primary owners. if err := e.mapper(ctx, ch, nodes, index, slices, c, opt, mapFn, reduceFn); err != nil { - return nil, err + return nil, errors.Wrap(err, "starting mapper") } // Iterate over all map responses and reduce. From 4452cd7260920f8892e361a065b209231cb72c1f Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Tue, 8 May 2018 18:29:04 -0500 Subject: [PATCH 35/58] Wrap errors in cluster.go --- cluster.go | 62 ++++++++++++++++++++-------------------- cluster_internal_test.go | 5 ++-- 2 files changed, 34 insertions(+), 33 deletions(-) diff --git a/cluster.go b/cluster.go index b02bd50d2..c2062383a 100644 --- a/cluster.go +++ b/cluster.go @@ -721,7 +721,7 @@ func (c *Cluster) fragSources(to *Cluster, idx *Index) (map[string][]*internal.R // Determine if a node is being added or removed. action, diffNodeID, err := c.diff(to) if err != nil { - return nil, err + return nil, errors.Wrap(err, "diffing") } // Initialize the map with all the nodes in `to`. @@ -905,7 +905,7 @@ func (c *Cluster) Open() error { // Load topology file if it exists. if err := c.loadTopology(); err != nil { - return fmt.Errorf("load topology: %v", err) + return errors.Wrap(err, "loading topology") } c.ID = c.Topology.ClusterID @@ -1006,7 +1006,7 @@ func (c *Cluster) handleNodeAction(nodeAction nodeAction) error { if err := c.setStateAndBroadcast(ClusterStateNormal); err != nil { c.Logger.Printf("setStateAndBroadcast error: err=%s", err) } - return err + return errors.Wrap(err, "setting state") } // j.Run() runs in a goroutine because in the case where the @@ -1023,14 +1023,14 @@ func (c *Cluster) handleNodeAction(nodeAction nodeAction) error { // Make sure j.Run() didn't return an error. if eg.Wait() != nil { - return err + return errors.Wrap(err, "running job") } c.Logger.Printf("received jobResult: %s", jobResult) switch jobResult { case ResizeJobStateDone: if err := c.CompleteCurrentJob(ResizeJobStateDone); err != nil { - return err + return errors.Wrap(err, "completing finished job") } // Add/remove uri to/from the cluster. if j.action == ResizeJobActionRemove { @@ -1040,7 +1040,7 @@ func (c *Cluster) handleNodeAction(nodeAction nodeAction) error { } case ResizeJobStateAborted: if err := c.CompleteCurrentJob(ResizeJobStateAborted); err != nil { - return err + return errors.Wrap(err, "completing aborted job") } } return nil @@ -1055,7 +1055,7 @@ func (c *Cluster) setStateAndBroadcast(state string) error { func (c *Cluster) sendTo(node *Node, msg proto.Message) error { if err := c.Broadcaster.SendTo(node, msg); err != nil { - return err + return errors.Wrap(err, "sending") } return nil } @@ -1124,7 +1124,7 @@ func (c *Cluster) generateResizeJob(nodeAction nodeAction) (*ResizeJob, error) { j, err := c.generateResizeJobByAction(nodeAction) if err != nil { - return nil, err + return nil, errors.Wrap(err, "generating job") } c.Logger.Printf("generated ResizeJob: %d", j.ID) @@ -1171,7 +1171,7 @@ func (c *Cluster) generateResizeJobByAction(nodeAction nodeAction) (*ResizeJob, for _, idx := range c.Holder.Indexes() { fragSources, err := c.fragSources(toCluster, idx) if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting sources") } for id, sources := range fragSources { @@ -1223,7 +1223,7 @@ func (c *Cluster) FollowResizeInstruction(instr *internal.ResizeInstruction) err // Make sure the cluster status on this node agrees with the Coordinator // before attempting a resize. if err := c.MergeClusterStatus(instr.ClusterStatus); err != nil { - return err + return errors.Wrap(err, "merging cluster status") } c.Logger.Printf("MergeClusterStatus done, start goroutine") @@ -1248,7 +1248,7 @@ func (c *Cluster) FollowResizeInstruction(instr *internal.ResizeInstruction) err // Sync the schema received in the resize instruction. c.Logger.Printf("Holder ApplySchema") if err := c.Holder.ApplySchema(instr.Schema); err != nil { - return err + return errors.Wrap(err, "applying schema") } // Create a client for calling remote nodes. @@ -1269,13 +1269,13 @@ func (c *Cluster) FollowResizeInstruction(instr *internal.ResizeInstruction) err // Create view. v, err := f.CreateViewIfNotExists(src.View) if err != nil { - return err + return errors.Wrap(err, "creating view") } // Create the local fragment. frag, err := v.CreateFragmentIfNotExists(src.Slice) if err != nil { - return err + return errors.Wrap(err, "creating fragment") } // Stream slice from remote node. @@ -1291,7 +1291,7 @@ func (c *Cluster) FollowResizeInstruction(instr *internal.ResizeInstruction) err if err == ErrFragmentNotFound { return nil } - return err + return errors.Wrap(err, "retrieving slice") } else if rd == nil { return fmt.Errorf("slice %v doesn't exist on host: %s", src.Slice, src.Node.URI) } @@ -1304,7 +1304,7 @@ func (c *Cluster) FollowResizeInstruction(instr *internal.ResizeInstruction) err } return nil }(); err != nil { - return err + return errors.Wrap(err, "copying remote slice") } } return nil @@ -1438,7 +1438,7 @@ func (j *ResizeJob) Run() error { err := j.distributeResizeInstructions() if err != nil { j.result <- ResizeJobStateAborted - return err + return errors.Wrap(err, "distributing instructions") } return nil } @@ -1475,7 +1475,7 @@ func (j *ResizeJob) distributeResizeInstructions() error { } j.Logger.Printf("send resize instructions: %v", instr) if err := j.Broadcaster.SendTo(node, instr); err != nil { - return err + return errors.Wrap(err, "sending instruction") } } return nil @@ -1581,16 +1581,16 @@ func (c *Cluster) loadTopology() error { c.Topology = NewTopology() return nil } else if err != nil { - return err + return errors.Wrap(err, "reading file") } var pb internal.Topology if err := proto.Unmarshal(buf, &pb); err != nil { - return err + return errors.Wrap(err, "unmarshalling") } top, err := decodeTopology(&pb) if err != nil { - return err + return errors.Wrap(err, "decoding") } c.Topology = top @@ -1601,13 +1601,13 @@ func (c *Cluster) loadTopology() error { func (c *Cluster) saveTopology() error { if err := os.MkdirAll(c.Path, 0777); err != nil { - return err + return errors.Wrap(err, "creating directory") } if buf, err := proto.Marshal(encodeTopology(c.Topology)); err != nil { - return err + return errors.Wrap(err, "marshalling") } else if err := ioutil.WriteFile(filepath.Join(c.Path, ".topology"), buf, 0666); err != nil { - return err + return errors.Wrap(err, "writing file") } return nil } @@ -1704,7 +1704,7 @@ func (c *Cluster) nodeJoin(node *Node) error { } if err := c.AddNode(node); err != nil { - return err + return errors.Wrap(err, "adding node for agreement") } // Only change to normal if there is no existing data. Otherwise, @@ -1742,7 +1742,7 @@ func (c *Cluster) nodeJoin(node *Node) error { // If the holder does not yet contain data, go ahead and add the node. if ok, err := c.Holder.HasData(); !ok && err == nil { if err := c.AddNode(node); err != nil { - return err + return errors.Wrap(err, "adding node") } return c.setStateAndBroadcast(ClusterStateNormal) } else if err != nil { @@ -1752,7 +1752,7 @@ func (c *Cluster) nodeJoin(node *Node) error { // If the cluster has data, we need to change to RESIZING and // kick off the resizing process. if err := c.setStateAndBroadcast(ClusterStateResizing); err != nil { - return err + return errors.Wrap(err, "broadcasting state") } c.joiningLeavingNodes <- nodeAction{node, ResizeJobActionAdd} @@ -1784,7 +1784,7 @@ func (c *Cluster) NodeLeave(node *Node) error { _, err := c.generateResizeJobByAction(nodeAction{c.nodeByID(node.ID), ResizeJobActionRemove}) if err != nil { - return err + return errors.Wrap(err, "generating job") } return c.nodeLeave(node) @@ -1802,7 +1802,7 @@ func (c *Cluster) nodeLeave(node *Node) error { // If the holder does not yet contain data, go ahead and remove the node. if ok, err := c.Holder.HasData(); !ok && err == nil { if err := c.RemoveNode(n); err != nil { - return err + return errors.Wrap(err, "removing node") } return c.setStateAndBroadcast(ClusterStateNormal) } else if err != nil { @@ -1812,7 +1812,7 @@ func (c *Cluster) nodeLeave(node *Node) error { // If the cluster has data then change state to RESIZING and // kick off the resizing process. if err := c.setStateAndBroadcast(ClusterStateResizing); err != nil { - return err + return errors.Wrap(err, "broadcasting state") } c.joiningLeavingNodes <- nodeAction{n, ResizeJobActionRemove} @@ -1836,7 +1836,7 @@ func (c *Cluster) MergeClusterStatus(cs *internal.ClusterStatus) error { // Add all nodes from the coordinator. for _, node := range officialNodes { if err := c.AddNode(node); err != nil { - return err + return errors.Wrap(err, "adding node") } } @@ -1857,7 +1857,7 @@ func (c *Cluster) MergeClusterStatus(cs *internal.ClusterStatus) error { for _, nodeID := range nodeIDsToRemove { if err := c.RemoveNode(c.nodeByID(nodeID)); err != nil { - return err + return errors.Wrap(err, "removing node") } } diff --git a/cluster_internal_test.go b/cluster_internal_test.go index da9989760..6b1f6e72b 100644 --- a/cluster_internal_test.go +++ b/cluster_internal_test.go @@ -17,6 +17,7 @@ package pilosa import ( "io/ioutil" "reflect" + "strings" "testing" "github.com/pilosa/pilosa/internal" @@ -243,8 +244,8 @@ func TestFragSources(t *testing.T) { actual, err := (test.from).fragSources(test.to, test.idx) if test.err != "" { - if err.Error() != test.err { - t.Fatalf("expected error: %s", test.err) + if !strings.Contains(err.Error(), test.err) { + t.Fatalf("expected error: %s, got: %s", test.err, err.Error()) } } else { if err != nil { From 3907e21f6f70c7751b8a36c61fb6ce163f04af67 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Tue, 8 May 2018 18:40:44 -0500 Subject: [PATCH 36/58] Wrap errors in frame --- frame.go | 68 ++++++++++++++++++++++++------------------------- handler_test.go | 4 +-- holder_test.go | 4 +-- 3 files changed, 38 insertions(+), 38 deletions(-) diff --git a/frame.go b/frame.go index 0c87210bf..966079619 100644 --- a/frame.go +++ b/frame.go @@ -15,7 +15,6 @@ package pilosa import ( - "errors" "fmt" "io/ioutil" "os" @@ -27,6 +26,7 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pilosa/pilosa/internal" "github.com/pilosa/pilosa/pql" + "github.com/pkg/errors" ) // Default frame settings. @@ -156,7 +156,7 @@ func (f *Frame) SetCacheSize(v uint32) error { // Persist meta data to disk on change. f.cacheSize = v if err := f.saveMeta(); err != nil { - return err + return errors.Wrap(err, "saving") } return nil @@ -192,19 +192,19 @@ func (f *Frame) Open() error { if err := func() error { // Ensure the frame's path exists. if err := os.MkdirAll(f.path, 0777); err != nil { - return err + return errors.Wrap(err, "creating dir") } if err := f.loadMeta(); err != nil { - return err + return errors.Wrap(err, "loading") } if err := f.openViews(); err != nil { - return err + return errors.Wrap(err, "opening views") } if err := f.rowAttrStore.Open(); err != nil { - return err + return errors.Wrap(err, "opening attrstore") } return nil @@ -222,13 +222,13 @@ func (f *Frame) openViews() error { if os.IsNotExist(err) { return nil } else if err != nil { - return err + return errors.Wrap(err, "opening view directory") } defer file.Close() fis, err := file.Readdir(0) if err != nil { - return err + return errors.Wrap(err, "reading") } for _, fi := range fis { @@ -262,10 +262,10 @@ func (f *Frame) loadMeta() error { //f.fields return nil } else if err != nil { - return err + return errors.Wrap(err, "reading") } else { if err := proto.Unmarshal(buf, &pb); err != nil { - return err + return errors.Wrap(err, "unmarshaling") } } @@ -288,12 +288,12 @@ func (f *Frame) saveMeta() error { fo := f.options() buf, err := proto.Marshal(fo.Encode()) if err != nil { - return err + return errors.Wrap(err, "marshaling") } // Write to meta file. if err := ioutil.WriteFile(filepath.Join(f.path, ".meta"), buf, 0666); err != nil { - return err + return errors.Wrap(err, "writing") } return nil @@ -365,7 +365,7 @@ func (f *Frame) CreateField(field *Field) error { // addField adds a single field to fields. func (f *Frame) addField(field *Field) error { if err := ValidateField(field); err != nil { - return err + return errors.Wrap(err, "validating") } else if f.HasField(field.Name) { return ErrFieldExists } @@ -388,7 +388,7 @@ func (f *Frame) GetFields() ([]*Field, error) { err := f.loadMeta() if err != nil { - return nil, err + return nil, errors.Wrap(err, "loading meta") } return f.fields, nil @@ -410,9 +410,9 @@ func (f *Frame) DeleteField(name string) error { delete(f.views, viewName) if err := view.Close(); err != nil { - return err + return errors.Wrap(err, "closing") } else if err := os.RemoveAll(view.Path()); err != nil { - return err + return errors.Wrap(err, "deleting directory") } } @@ -453,7 +453,7 @@ func (f *Frame) SetTimeQuantum(q TimeQuantum) error { // Persist meta data to disk. if err := f.saveMeta(); err != nil { - return err + return errors.Wrap(err, "saving meta") } return nil @@ -522,7 +522,7 @@ func (f *Frame) CreateViewIfNotExists(name string) (*View, error) { View: name, }) if err != nil { - return nil, err + return nil, errors.Wrap(err, "sending CreateView message") } } @@ -547,7 +547,7 @@ func (f *Frame) createViewIfNotExistsBase(name string) (*View, bool, error) { view := f.newView(f.ViewPath(name), name) if err := view.Open(); err != nil { - return nil, false, err + return nil, false, errors.Wrap(err, "opening") } view.RowAttrStore = f.rowAttrStore f.views[view.Name()] = view @@ -574,12 +574,12 @@ func (f *Frame) DeleteView(name string) error { // Close data files before deletion. if err := view.Close(); err != nil { - return err + return errors.Wrap(err, "closing") } // Delete view directory. if err := os.RemoveAll(view.Path()); err != nil { - return err + return errors.Wrap(err, "deleting directory") } delete(f.views, name) @@ -597,12 +597,12 @@ func (f *Frame) SetBit(name string, rowID, colID uint64, t *time.Time) (changed // Retrieve view. Exit if it doesn't exist. view, err := f.CreateViewIfNotExists(name) if err != nil { - return changed, err + return changed, errors.Wrap(err, "creating view") } // Set non-time bit. if v, err := view.SetBit(rowID, colID); err != nil { - return changed, err + return changed, errors.Wrap(err, "setting on view") } else if v { changed = v } @@ -616,11 +616,11 @@ func (f *Frame) SetBit(name string, rowID, colID uint64, t *time.Time) (changed for _, subname := range ViewsByTime(name, *t, f.TimeQuantum()) { view, err := f.CreateViewIfNotExists(subname) if err != nil { - return changed, err + return changed, errors.Wrapf(err, "creating view %s", subname) } if c, err := view.SetBit(rowID, colID); err != nil { - return changed, err + return changed, errors.Wrapf(err, "setting on view %s", subname) } else if c { changed = true } @@ -639,12 +639,12 @@ func (f *Frame) ClearBit(name string, rowID, colID uint64, t *time.Time) (change // Retrieve view. Exit if it doesn't exist. view, err := f.CreateViewIfNotExists(name) if err != nil { - return changed, err + return changed, errors.Wrap(err, "creating view") } // Clear non-time bit. if v, err := view.ClearBit(rowID, colID); err != nil { - return changed, err + return changed, errors.Wrap(err, "setting on view") } else if v { changed = v } @@ -658,11 +658,11 @@ func (f *Frame) ClearBit(name string, rowID, colID uint64, t *time.Time) (change for _, subname := range ViewsByTime(name, *t, f.TimeQuantum()) { view, err := f.CreateViewIfNotExists(subname) if err != nil { - return changed, err + return changed, errors.Wrapf(err, "creating view %s", subname) } if c, err := view.ClearBit(rowID, colID); err != nil { - return changed, err + return changed, errors.Wrapf(err, "setting on view %s", subname) } else if c { changed = true } @@ -708,7 +708,7 @@ func (f *Frame) SetFieldValue(columnID uint64, name string, value int64) (change // Fetch target view. view, err := f.CreateViewIfNotExists(ViewFieldPrefix + name) if err != nil { - return false, err + return false, errors.Wrap(err, "creating view") } // Determine base value to store. @@ -886,12 +886,12 @@ func (f *Frame) Import(rowIDs, columnIDs []uint64, timestamps []*time.Time) erro view, err := f.CreateViewIfNotExists(key.View) if err != nil { - return err + return errors.Wrap(err, "creating view") } frag, err := view.CreateFragmentIfNotExists(key.Slice) if err != nil { - return err + return errors.Wrap(err, "creating view") } if err := frag.Import(data.RowIDs, data.ColumnIDs); err != nil { @@ -938,12 +938,12 @@ func (f *Frame) ImportValue(fieldName string, columnIDs []uint64, values []int64 // because we need to know bitDepth (based on min/max value). view, err := f.CreateViewIfNotExists(key.View) if err != nil { - return err + return errors.Wrap(err, "creating view") } frag, err := view.CreateFragmentIfNotExists(key.Slice) if err != nil { - return err + return errors.Wrap(err, "creating fragment") } baseValues := make([]uint64, len(data.Values)) diff --git a/handler_test.go b/handler_test.go index 4187b3303..3e5aea2c9 100644 --- a/handler_test.go +++ b/handler_test.go @@ -894,7 +894,7 @@ func TestHandler_Frame_AddField(t *testing.T) { ) if err != nil { t.Fatal(err) - } else if body := MustReadAll(resp.Body); string(body) != `invalid field type`+"\n" { + } else if body := MustReadAll(resp.Body); string(body) != `validating: invalid field type`+"\n" { t.Fatalf("unexpected body: %q", body) } else if err := resp.Body.Close(); err != nil { t.Fatal(err) @@ -916,7 +916,7 @@ func TestHandler_Frame_AddField(t *testing.T) { ) if err != nil { t.Fatal(err) - } else if body := MustReadAll(resp.Body); string(body) != `invalid field range`+"\n" { + } else if body := MustReadAll(resp.Body); string(body) != `validating: invalid field range`+"\n" { t.Fatalf("unexpected body: %q", body) } else if err := resp.Body.Close(); err != nil { t.Fatal(err) diff --git a/holder_test.go b/holder_test.go index e819c6e94..6d300d84c 100644 --- a/holder_test.go +++ b/holder_test.go @@ -127,7 +127,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } - if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=foo, err=open frame: name=bar, err=unexpected EOF") { + if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=foo, err=open frame: name=bar, err=loading: unmarshaling: unexpected EOF") { t.Fatalf("unexpected error: %s", err) } }) @@ -145,7 +145,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } - if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=foo, err=open frame: name=bar, err=invalid database") { + if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=foo, err=open frame: name=bar, err=opening attrstore: invalid database") { t.Fatalf("unexpected error: %s", err) } }) From 4f51373734541cb246005b23fbbe3b1a11cfbfc3 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Tue, 8 May 2018 19:43:36 -0500 Subject: [PATCH 37/58] Wrap errors in holder --- holder.go | 54 +++++++++++++++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/holder.go b/holder.go index f048aac44..5c4afc211 100644 --- a/holder.go +++ b/holder.go @@ -96,19 +96,19 @@ func (h *Holder) Open() error { h.Logger.Printf("open holder path: %s", h.Path) if err := os.MkdirAll(h.Path, 0777); err != nil { - return err + return errors.Wrap(err, "creating directory") } // Open path to read all index directories. f, err := os.Open(h.Path) if err != nil { - return err + return errors.Wrap(err, "opening directory") } defer f.Close() fis, err := f.Readdir(0) if err != nil { - return err + return errors.Wrap(err, "reading directory") } for _, fi := range fis { @@ -123,7 +123,7 @@ func (h *Holder) Open() error { h.Logger.Printf("ERROR opening index: %s, err=%s", fi.Name(), err) continue } else if err != nil { - return err + return errors.Wrap(err, "opening index") } if err := index.Open(); err != nil { if err == ErrName { @@ -158,7 +158,7 @@ func (h *Holder) Close() error { for _, index := range h.indexes { if err := index.Close(); err != nil { - return err + return errors.Wrap(err, "closing index") } } return nil @@ -245,20 +245,20 @@ func (h *Holder) ApplySchema(schema *internal.Schema) error { opt := IndexOptions{} idx, err := h.CreateIndexIfNotExists(index.Name, opt) if err != nil { - return err + return errors.Wrap(err, "creating index") } // Create frames that don't exist. for _, f := range index.Frames { opt := decodeFrameOptions(f.Meta) frame, err := idx.CreateFrameIfNotExists(f.Name, *opt) if err != nil { - return err + return errors.Wrap(err, "creating frame") } // Create views that don't exist. for _, v := range f.Views { _, err := frame.CreateViewIfNotExists(v) if err != nil { - return err + return errors.Wrap(err, "creating view") } } } @@ -347,11 +347,11 @@ func (h *Holder) createIndex(name string, opt IndexOptions) (*Index, error) { // Otherwise create a new index. index, err := h.newIndex(h.IndexPath(name), name) if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating") } if err := index.Open(); err != nil { - return nil, err + return nil, errors.Wrap(err, "opening") } // Update options. @@ -387,12 +387,12 @@ func (h *Holder) DeleteIndex(name string) error { // Close index. if err := index.Close(); err != nil { - return err + return errors.Wrap(err, "closing") } // Delete index directory. if err := os.RemoveAll(h.IndexPath(name)); err != nil { - return err + return errors.Wrap(err, "removing directory") } // Remove reference. @@ -528,7 +528,7 @@ func (h *Holder) loadNodeID() (string, error) { nodeID := "" h.Logger.Printf("load NodeID: %s", idPath) if err := os.MkdirAll(h.Path, 0777); err != nil { - return "", err + return "", errors.Wrap(err, "creating directory") } nodeIDBytes, err := ioutil.ReadFile(idPath) @@ -538,10 +538,10 @@ func (h *Holder) loadNodeID() (string, error) { nodeID = uuid.NewV4().String() err = ioutil.WriteFile(idPath, []byte(nodeID), 0600) if err != nil { - return "", err + return "", errors.Wrap(err, "writing file") } } else if err != nil { - return "", err + return "", errors.Wrap(err, "reading file") } return nodeID, nil @@ -667,7 +667,7 @@ func (s *HolderSyncer) syncIndex(index string) error { // Read block checksums. blks, err := idx.ColumnAttrStore().Blocks() if err != nil { - return err + return errors.Wrap(err, "getting blocks") } s.Stats.CountWithCustomTags("ColumnAttrStoreBlocks", int64(len(blks)), 1.0, []string{indexTag}) @@ -679,7 +679,7 @@ func (s *HolderSyncer) syncIndex(index string) error { // Skip update and recomputation if no attributes have changed. m, err := client.ColumnAttrDiff(context.Background(), index, blks) if err != nil { - return err + return errors.Wrap(err, "getting differing blocks") } else if len(m) == 0 { continue } @@ -687,13 +687,13 @@ func (s *HolderSyncer) syncIndex(index string) error { // Update local copy. if err := idx.ColumnAttrStore().SetBulkAttrs(m); err != nil { - return err + return errors.Wrap(err, "setting attrs") } // Recompute blocks. blks, err = idx.ColumnAttrStore().Blocks() if err != nil { - return err + return errors.Wrap(err, "recomputing blocks") } } @@ -713,7 +713,7 @@ func (s *HolderSyncer) syncFrame(index, name string) error { // Read block checksums. blks, err := f.RowAttrStore().Blocks() if err != nil { - return err + return errors.Wrap(err, "getting blocks") } s.Stats.CountWithCustomTags("RowAttrStoreBlocks", int64(len(blks)), 1.0, []string{indexTag, frameTag}) @@ -727,7 +727,7 @@ func (s *HolderSyncer) syncFrame(index, name string) error { if err == ErrFrameNotFound { continue // frame not created remotely yet, skip } else if err != nil { - return err + return errors.Wrap(err, "getting differing blocks") } else if len(m) == 0 { continue } @@ -735,13 +735,13 @@ func (s *HolderSyncer) syncFrame(index, name string) error { // Update local copy. if err := f.RowAttrStore().SetBulkAttrs(m); err != nil { - return err + return errors.Wrap(err, "setting attrs") } // Recompute blocks. blks, err = f.RowAttrStore().Blocks() if err != nil { - return err + return errors.Wrap(err, "recomputing blocks") } } @@ -759,13 +759,13 @@ func (s *HolderSyncer) syncFragment(index, frame, view string, slice uint64) err // Ensure view exists locally. v, err := f.CreateViewIfNotExists(view) if err != nil { - return err + return errors.Wrap(err, "creating view") } // Ensure fragment exists locally. frag, err := v.CreateFragmentIfNotExists(slice) if err != nil { - return err + return errors.Wrap(err, "creating fragment") } // Sync fragments together. @@ -777,7 +777,7 @@ func (s *HolderSyncer) syncFragment(index, frame, view string, slice uint64) err RemoteClient: s.RemoteClient, } if err := fs.SyncFragment(); err != nil { - return err + return errors.Wrap(err, "syncing") } return nil @@ -827,7 +827,7 @@ func (c *HolderCleaner) CleanHolder() error { } // Delete fragment. if err := view.DeleteFragment(fragSlice); err != nil { - return err + return errors.Wrap(err, "deleting fragment") } } } From a3dc756be49fb4de05b232ca20fe4c3b5dc6c9cf Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Tue, 8 May 2018 19:46:26 -0500 Subject: [PATCH 38/58] Wrap errors in handler --- handler.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/handler.go b/handler.go index 7bf11af1b..6eccd70a8 100644 --- a/handler.go +++ b/handler.go @@ -349,7 +349,7 @@ func (p *postIndexRequest) UnmarshalJSON(b []byte) error { // m is an overflow map used to capture additional, unexpected keys. m := make(map[string]interface{}) if err := json.Unmarshal(b, &m); err != nil { - return err + return errors.Wrap(err, "unmarshalling unexpected values") } validIndexOptions := getValidOptions(IndexOptions{}) @@ -360,7 +360,7 @@ func (p *postIndexRequest) UnmarshalJSON(b []byte) error { // Unmarshal expected values. var _p _postIndexRequest if err := json.Unmarshal(b, &_p); err != nil { - return err + return errors.Wrap(err, "unmarshalling expected values") } p.Options = _p.Options @@ -526,7 +526,7 @@ func (p *postFrameRequest) UnmarshalJSON(b []byte) error { // m is an overflow map used to capture additional, unexpected keys. m := make(map[string]interface{}) if err := json.Unmarshal(b, &m); err != nil { - return err + return errors.Wrap(err, "unmarshaling unexpected keys") } validFrameOptions := getValidOptions(FrameOptions{}) @@ -538,7 +538,7 @@ func (p *postFrameRequest) UnmarshalJSON(b []byte) error { // Unmarshal expected values. var _p _postFrameRequest if err := json.Unmarshal(b, &_p); err != nil { - return err + return errors.Wrap(err, "unmarshalling expected keys") } p.Options = _p.Options @@ -813,13 +813,13 @@ func (h *Handler) readProtobufQueryRequest(r *http.Request) (*QueryRequest, erro // Slurp the body. body, err := ioutil.ReadAll(r.Body) if err != nil { - return nil, err + return nil, errors.Wrap(err, "reading") } // Unmarshal into object. var req internal.QueryRequest if err := proto.Unmarshal(body, &req); err != nil { - return nil, err + return nil, errors.Wrap(err, "unmarshalling") } return decodeQueryRequest(&req), nil @@ -832,7 +832,7 @@ func (h *Handler) readURLQueryRequest(r *http.Request) (*QueryRequest, error) { // Parse query string. buf, err := ioutil.ReadAll(r.Body) if err != nil { - return nil, err + return nil, errors.Wrap(err, "reading") } query := string(buf) @@ -877,9 +877,9 @@ func (h *Handler) writeQueryResponse(w http.ResponseWriter, r *http.Request, res // writeProtobufQueryResponse writes the response from the executor to w as protobuf. func (h *Handler) writeProtobufQueryResponse(w http.ResponseWriter, resp *QueryResponse) error { if buf, err := proto.Marshal(encodeQueryResponse(resp)); err != nil { - return err + return errors.Wrap(err, "marshalling") } else if _, err := w.Write(buf); err != nil { - return err + return errors.Wrap(err, "writing") } return nil } @@ -1327,7 +1327,7 @@ func parseUint64Slice(s string) ([]uint64, error) { // Parse number. num, err := strconv.ParseUint(str, 10, 64) if err != nil { - return nil, err + return nil, errors.Wrap(err, "parsing int") } a = append(a, num) } @@ -1591,7 +1591,7 @@ func GetTimeStamp(data map[string]interface{}, timeField string) (int64, error) v, err := time.Parse(TimeFormat, timestamp) if err != nil { - return 0, err + return 0, errors.Wrap(err, "parsing timestamp") } return v.Unix(), nil From 248d9b4ffb961af714a11f82d9a7ea32b2f28c07 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Tue, 8 May 2018 19:53:05 -0500 Subject: [PATCH 39/58] Wrap errors in view --- view.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/view.go b/view.go index dbcc73b8d..4f2ae1cd7 100644 --- a/view.go +++ b/view.go @@ -24,6 +24,7 @@ import ( "github.com/pilosa/pilosa/internal" "github.com/pilosa/pilosa/pql" + "github.com/pkg/errors" ) // View layout modes. @@ -105,13 +106,13 @@ func (v *View) Open() error { if err := func() error { // Ensure the view's path exists. if err := os.MkdirAll(v.path, 0777); err != nil { - return err + return errors.Wrap(err, "creating view directory") } else if err := os.MkdirAll(filepath.Join(v.path, "fragments"), 0777); err != nil { - return err + return errors.Wrap(err, "creating fragments directory") } if err := v.openFragments(); err != nil { - return err + return errors.Wrap(err, "opening fragments") } return nil @@ -129,13 +130,13 @@ func (v *View) openFragments() error { if os.IsNotExist(err) { return nil } else if err != nil { - return err + return errors.Wrap(err, "opening fragments directory") } defer file.Close() fis, err := file.Readdir(0) if err != nil { - return err + return errors.Wrap(err, "reading fragments directory") } for _, fi := range fis { @@ -168,7 +169,7 @@ func (v *View) Close() error { // Close all fragments. for _, frag := range v.fragments { if err := frag.Close(); err != nil { - return err + return errors.Wrap(err, "closing fragment") } } v.fragments = make(map[uint64]*Fragment) @@ -240,7 +241,7 @@ func (v *View) createFragmentIfNotExists(slice uint64) (*Fragment, error) { // Initialize and open fragment. frag := v.newFragment(v.FragmentPath(slice), slice) if err := frag.Open(); err != nil { - return nil, err + return nil, errors.Wrap(err, "opening fragment") } frag.RowAttrStore = v.RowAttrStore @@ -256,7 +257,7 @@ func (v *View) createFragmentIfNotExists(slice uint64) (*Fragment, error) { IsInverse: IsInverseView(v.name), }) if err != nil { - return nil, err + return nil, errors.Wrap(err, "sending message") } } @@ -286,12 +287,12 @@ func (v *View) DeleteFragment(slice uint64) error { // Close data files before deletion. if err := fragment.Close(); err != nil { - return err + return errors.Wrap(err, "closing fragment") } // Delete fragment file. if err := os.Remove(fragment.Path()); err != nil { - return err + return errors.Wrap(err, "deleting fragment file") } // Delete fragment cache file. From 43ec69d08b31f6bd511701067d53b8777cf0b940 Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Wed, 9 May 2018 09:32:18 -0500 Subject: [PATCH 40/58] API.URI was not being used. removed it. --- api.go | 5 ++--- client_test.go | 8 -------- ctl/backup_test.go | 6 +----- ctl/export_test.go | 9 ++------- ctl/import_test.go | 11 +---------- ctl/restore_test.go | 6 +----- server.go | 1 - test/handler.go | 7 ------- 8 files changed, 7 insertions(+), 46 deletions(-) diff --git a/api.go b/api.go index 4fe08f9a5..691e076ff 100644 --- a/api.go +++ b/api.go @@ -46,7 +46,6 @@ type API struct { BroadcastHandler BroadcastHandler StatusHandler StatusHandler Cluster *Cluster - URI URI RemoteClient *http.Client Logger Logger } @@ -294,7 +293,7 @@ func (api *API) ExportCSV(ctx context.Context, indexName string, frameName strin // Validate that this handler owns the slice. if !api.Cluster.OwnsSlice(api.LocalID(), indexName, slice) { - api.Logger.Printf("host does not own slice %s-%s slice:%d", api.URI, indexName, slice) + api.Logger.Printf("node %s does not own slice %d of index %s", api.LocalID(), slice, indexName) return ErrClusterDoesNotOwnSlice } @@ -957,7 +956,7 @@ func (api *API) LongQueryTime() time.Duration { func (api *API) indexFrame(indexName string, frameName string, slice uint64) (*Index, *Frame, error) { // Validate that this handler owns the slice. if !api.Cluster.OwnsSlice(api.LocalID(), indexName, slice) { - api.Logger.Printf("host does not own slice %s-%s slice:%d", api.URI, indexName, slice) + api.Logger.Printf("node %s does not own slice %d of index %s", api.LocalID(), slice, indexName) return nil, nil, ErrClusterDoesNotOwnSlice } diff --git a/client_test.go b/client_test.go index 88dc3accb..e2b806412 100644 --- a/client_test.go +++ b/client_test.go @@ -37,7 +37,6 @@ func createCluster(c *pilosa.Cluster) ([]*test.Server, []*test.Holder) { for i := 0; i < numNodes; i++ { hldr[i] = test.MustOpenHolder() server[i] = test.NewServer() - server[i].Handler.API.URI = server[i].HostURI() server[i].Handler.API.Cluster = c server[i].Handler.API.Cluster.Nodes[i].URI = server[i].HostURI() server[i].Handler.API.Holder = hldr[i].Holder @@ -218,7 +217,6 @@ func TestClient_Import(t *testing.T) { s := test.NewServer() defer s.Close() - s.Handler.API.URI = s.HostURI() s.Handler.API.Cluster = test.NewCluster(1) s.Handler.API.Cluster.Nodes[0].URI = s.HostURI() s.Handler.API.Holder = hldr.Holder @@ -269,7 +267,6 @@ func TestClient_ImportInverseEnabled(t *testing.T) { s := test.NewServer() defer s.Close() - s.Handler.API.URI = s.HostURI() s.Handler.API.Cluster = test.NewCluster(1) s.Handler.API.Cluster.Nodes[0].URI = s.HostURI() s.Handler.API.Holder = hldr.Holder @@ -318,7 +315,6 @@ func TestClient_ImportValue(t *testing.T) { s := test.NewServer() defer s.Close() - s.Handler.API.URI = s.HostURI() s.Handler.API.Cluster = test.NewCluster(1) s.Handler.API.Cluster.Nodes[0].URI = s.HostURI() s.Handler.API.Holder = hldr.Holder @@ -386,7 +382,6 @@ func TestClient_BackupRestore(t *testing.T) { s := test.NewServer() defer s.Close() - s.Handler.API.URI = s.HostURI() s.Handler.API.Cluster = test.NewCluster(1) s.Handler.API.Cluster.Nodes[0].URI = s.HostURI() s.Handler.API.Holder = hldr.Holder @@ -452,7 +447,6 @@ func TestClient_BackupInverseView(t *testing.T) { s := test.NewServer() defer s.Close() - s.Handler.API.URI = s.HostURI() s.Handler.API.Cluster = test.NewCluster(1) s.Handler.API.Cluster.Nodes[0].URI = s.HostURI() s.Handler.API.Holder = hldr.Holder @@ -489,7 +483,6 @@ func TestClient_BackupInvalidView(t *testing.T) { s := test.NewServer() defer s.Close() - s.Handler.API.URI = s.HostURI() s.Handler.API.Cluster = test.NewCluster(1) s.Handler.API.Cluster.Nodes[0].URI = s.HostURI() s.Handler.API.Holder = hldr.Holder @@ -518,7 +511,6 @@ func TestClient_FragmentBlocks(t *testing.T) { s := test.NewServer() defer s.Close() - s.Handler.API.URI = s.HostURI() s.Handler.API.Cluster = test.NewCluster(1) s.Handler.API.Cluster.Nodes[0].URI = s.HostURI() s.Handler.API.Holder = hldr.Holder diff --git a/ctl/backup_test.go b/ctl/backup_test.go index d80b72475..a073bc8a8 100644 --- a/ctl/backup_test.go +++ b/ctl/backup_test.go @@ -46,11 +46,7 @@ func TestBackupCommand_Run(t *testing.T) { s := test.NewServer() defer s.Close() - uri, err := pilosa.NewURIFromAddress(s.Host()) - if err != nil { - t.Fatal(err) - } - s.Handler.API.URI = *uri + s.Handler.API.Cluster = test.NewCluster(1) s.Handler.API.Cluster.Nodes[0].URI = s.HostURI() s.Handler.API.Holder = hldr.Holder diff --git a/ctl/export_test.go b/ctl/export_test.go index b414f711f..48a90a065 100644 --- a/ctl/export_test.go +++ b/ctl/export_test.go @@ -59,11 +59,7 @@ func TestExportCommand_Run(t *testing.T) { defer hldr.Close() s := test.NewServer() defer s.Close() - uri, err := pilosa.NewURIFromAddress(s.Host()) - if err != nil { - t.Fatal(err) - } - s.Handler.API.URI = *uri + s.Handler.API.Cluster = test.NewCluster(1) s.Handler.API.Cluster.Nodes[0].URI = s.HostURI() s.Handler.API.Holder = hldr.Holder @@ -75,8 +71,7 @@ func TestExportCommand_Run(t *testing.T) { cm.Index = "i" cm.Frame = "f" cm.View = pilosa.ViewStandard - err = cm.Run(context.Background()) - if err != nil { + if err := cm.Run(context.Background()); err != nil { t.Fatalf("Export Run doesn't work: %s", err) } } diff --git a/ctl/import_test.go b/ctl/import_test.go index 6cbc8271a..08eea8323 100644 --- a/ctl/import_test.go +++ b/ctl/import_test.go @@ -65,11 +65,7 @@ func TestImportCommand_Run(t *testing.T) { defer hldr.Close() s := test.NewServer() defer s.Close() - uri, err := pilosa.NewURIFromAddress(s.Host()) - if err != nil { - t.Fatal(err) - } - s.Handler.API.URI = *uri + s.Handler.API.Cluster = test.NewCluster(1) s.Handler.API.Cluster.Nodes[0].URI = s.HostURI() s.Handler.API.Holder = hldr.Holder @@ -103,12 +99,7 @@ func TestImportCommand_RunValue(t *testing.T) { defer hldr.Close() s := test.NewServer() defer s.Close() - uri, err := pilosa.NewURIFromAddress(s.Host()) - if err != nil { - t.Fatal(err) - } - s.Handler.API.URI = *uri s.Handler.API.Cluster = test.NewCluster(1) s.Handler.API.Cluster.Nodes[0].URI = s.HostURI() s.Handler.API.Holder = hldr.Holder diff --git a/ctl/restore_test.go b/ctl/restore_test.go index 04cb30415..896db57c5 100644 --- a/ctl/restore_test.go +++ b/ctl/restore_test.go @@ -48,11 +48,7 @@ func TestRestoreCommand_Run(t *testing.T) { s := test.NewServer() defer s.Close() - uri, err := pilosa.NewURIFromAddress(s.Host()) - if err != nil { - t.Fatal(err) - } - s.Handler.API.URI = *uri + s.Handler.API.Cluster = test.NewCluster(1) s.Handler.API.Cluster.Nodes[0].URI = s.HostURI() s.Handler.API.Holder = hldr.Holder diff --git a/server.go b/server.go index 43caa9558..c8b631580 100644 --- a/server.go +++ b/server.go @@ -300,7 +300,6 @@ func (s *Server) Open() error { s.handler.API.Broadcaster = s.Broadcaster s.handler.API.BroadcastHandler = s s.handler.API.StatusHandler = s - s.handler.API.URI = s.URI s.handler.API.Cluster = s.Cluster // Initialize Holder. diff --git a/test/handler.go b/test/handler.go index c325d3243..50b081812 100644 --- a/test/handler.go +++ b/test/handler.go @@ -75,13 +75,6 @@ func NewServer() *Server { } s.Server = httptest.NewServer(s.Handler.Handler) - // Update handler to use hostname. - uri, err := pilosa.NewURIFromAddress(s.Host()) - if err != nil { - panic(err) - } - s.Handler.API.URI = *uri - // Handler test messages can no-op. s.Handler.API.Broadcaster = pilosa.NopBroadcaster // Create a default cluster on the handler From e4c11a28b118516458cdbadd9eaeef8d860e4fe7 Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Wed, 9 May 2018 10:42:11 -0500 Subject: [PATCH 41/58] make sure gossipMemberSet.Logger is set during server setup --- server/server.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/server/server.go b/server/server.go index 3098cddeb..a3824cbc3 100644 --- a/server/server.go +++ b/server/server.go @@ -300,10 +300,19 @@ func (m *Command) SetupNetworking() error { gossipEventReceiver := gossip.NewGossipEventReceiver(m.logger) m.Server.Cluster.EventReceiver = gossipEventReceiver - gossipMemberSet, err := gossip.NewGossipMemberSet(m.Server.NodeID, m.Server.URI.Host(), m.Config.Gossip, gossipEventReceiver, m.Server, gossip.WithLogger(m.logger.Logger()), gossip.WithTransport(transport)) + gossipMemberSet, err := gossip.NewGossipMemberSet( + m.Server.NodeID, + m.Server.URI.Host(), + m.Config.Gossip, + gossipEventReceiver, + m.Server, + gossip.WithLogger(m.logger.Logger()), + gossip.WithTransport(transport), + ) if err != nil { return err } + gossipMemberSet.Logger = m.logger m.Server.Cluster.MemberSet = gossipMemberSet m.Server.Broadcaster = m.Server m.Server.BroadcastReceiver = gossipMemberSet From b74e5e7ec703ad3c4de1b0e9be136d9aac854318 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Wed, 9 May 2018 10:46:16 -0500 Subject: [PATCH 42/58] Update error message --- holder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/holder.go b/holder.go index 5c4afc211..2475d61d0 100644 --- a/holder.go +++ b/holder.go @@ -777,7 +777,7 @@ func (s *HolderSyncer) syncFragment(index, frame, view string, slice uint64) err RemoteClient: s.RemoteClient, } if err := fs.SyncFragment(); err != nil { - return errors.Wrap(err, "syncing") + return errors.Wrap(err, "syncing fragment") } return nil From 5e043919ae9ce898b4937ce0bba39aee82b923e5 Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Wed, 9 May 2018 10:48:49 -0500 Subject: [PATCH 43/58] default GossipMemberSet.Logger to NopLogger --- gossip/gossip.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gossip/gossip.go b/gossip/gossip.go index 25dd40cc4..49b66d8a2 100644 --- a/gossip/gossip.go +++ b/gossip/gossip.go @@ -169,7 +169,9 @@ func WithLogger(logger *log.Logger) GossipMemberSetOption { // NewGossipMemberSet returns a new instance of GossipMemberSet based on options. func NewGossipMemberSet(name string, host string, cfg Config, ger *GossipEventReceiver, sh pilosa.StatusHandler, options ...GossipMemberSetOption) (*GossipMemberSet, error) { - g := &GossipMemberSet{} + g := &GossipMemberSet{ + Logger: pilosa.NopLogger, + } // options for _, opt := range options { From 4f6947af3b5730664d08e69deb74299a171ef1a0 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Wed, 9 May 2018 10:50:17 -0500 Subject: [PATCH 44/58] Update error messages --- frame.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/frame.go b/frame.go index 966079619..5057280b2 100644 --- a/frame.go +++ b/frame.go @@ -192,11 +192,11 @@ func (f *Frame) Open() error { if err := func() error { // Ensure the frame's path exists. if err := os.MkdirAll(f.path, 0777); err != nil { - return errors.Wrap(err, "creating dir") + return errors.Wrap(err, "creating frame dir") } if err := f.loadMeta(); err != nil { - return errors.Wrap(err, "loading") + return errors.Wrap(err, "loading meta") } if err := f.openViews(); err != nil { @@ -228,7 +228,7 @@ func (f *Frame) openViews() error { fis, err := file.Readdir(0) if err != nil { - return errors.Wrap(err, "reading") + return errors.Wrap(err, "reading directory") } for _, fi := range fis { @@ -262,7 +262,7 @@ func (f *Frame) loadMeta() error { //f.fields return nil } else if err != nil { - return errors.Wrap(err, "reading") + return errors.Wrap(err, "reading meta") } else { if err := proto.Unmarshal(buf, &pb); err != nil { return errors.Wrap(err, "unmarshaling") @@ -293,7 +293,7 @@ func (f *Frame) saveMeta() error { // Write to meta file. if err := ioutil.WriteFile(filepath.Join(f.path, ".meta"), buf, 0666); err != nil { - return errors.Wrap(err, "writing") + return errors.Wrap(err, "writing meta") } return nil @@ -365,7 +365,7 @@ func (f *Frame) CreateField(field *Field) error { // addField adds a single field to fields. func (f *Frame) addField(field *Field) error { if err := ValidateField(field); err != nil { - return errors.Wrap(err, "validating") + return errors.Wrap(err, "validating field") } else if f.HasField(field.Name) { return ErrFieldExists } @@ -410,7 +410,7 @@ func (f *Frame) DeleteField(name string) error { delete(f.views, viewName) if err := view.Close(); err != nil { - return errors.Wrap(err, "closing") + return errors.Wrap(err, "closing view") } else if err := os.RemoveAll(view.Path()); err != nil { return errors.Wrap(err, "deleting directory") } @@ -547,7 +547,7 @@ func (f *Frame) createViewIfNotExistsBase(name string) (*View, bool, error) { view := f.newView(f.ViewPath(name), name) if err := view.Open(); err != nil { - return nil, false, errors.Wrap(err, "opening") + return nil, false, errors.Wrap(err, "opening view") } view.RowAttrStore = f.rowAttrStore f.views[view.Name()] = view @@ -574,7 +574,7 @@ func (f *Frame) DeleteView(name string) error { // Close data files before deletion. if err := view.Close(); err != nil { - return errors.Wrap(err, "closing") + return errors.Wrap(err, "closing view") } // Delete view directory. From a436d4d32a68b2c25532c9d2aa8d109745fefcb7 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Wed, 9 May 2018 15:00:39 -0500 Subject: [PATCH 45/58] fixed deadlock in setcooridnator --- cluster.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cluster.go b/cluster.go index b02bd50d2..8e342024e 100644 --- a/cluster.go +++ b/cluster.go @@ -307,16 +307,16 @@ func (c *Cluster) isCoordinator() bool { // nodes with its version of Cluster.Status. func (c *Cluster) SetCoordinator(n *Node) error { c.mu.Lock() - defer c.mu.Unlock() // Verify that the new Coordinator value matches // this node. if c.Node.ID != n.ID { + c.mu.Unlock() return fmt.Errorf("coordinator node does not match this node") } // Update IsCoordinator on all nodes (locally). _ = c.updateCoordinator(n) - + c.mu.Unlock() // Send the update coordinator message to all nodes. err := c.Broadcaster.SendSync( &internal.UpdateCoordinatorMessage{ From 57261bda9f84251bd7ebe8c5a5cfdd01d3495e62 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Wed, 9 May 2018 15:52:02 -0500 Subject: [PATCH 46/58] Fix spelling error --- client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client.go b/client.go index 7403a7680..72225aef2 100644 --- a/client.go +++ b/client.go @@ -1015,7 +1015,7 @@ func (c *InternalHTTPClient) BlockData(ctx context.Context, index, frame, view s if body, err := ioutil.ReadAll(resp.Body); err != nil { return nil, nil, errors.Wrap(err, "reading") } else if err := proto.Unmarshal(body, &rsp); err != nil { - return nil, nil, errors.Wrap(err, "unmarhsaling") + return nil, nil, errors.Wrap(err, "unmarshalling") } return rsp.RowIDs, rsp.ColumnIDs, nil } From 9710ac978d79c92464b2c2308560e3c9f091d2ab Mon Sep 17 00:00:00 2001 From: Will James Date: Wed, 9 May 2018 21:30:49 -0400 Subject: [PATCH 47/58] Correct usage of ClearBit Usage of `ClearBit` was incorrectly shown as `SetBit`. --- docs/query-language.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/query-language.md b/docs/query-language.md index 8ed0d5439..7d9f22886 100644 --- a/docs/query-language.md +++ b/docs/query-language.md @@ -160,8 +160,8 @@ Delete url value for repo 10. **Spec:** ``` -SetBit(, , , - [timestamp=TIMESTAMP]) +ClearBit(, , , + [timestamp=TIMESTAMP]) ``` **Description:** From 17520033c6b8d49b79b54bda5aec51748d0fc0d5 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 10 May 2018 10:57:27 -0500 Subject: [PATCH 48/58] Update test error strings --- handler_test.go | 4 ++-- holder_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/handler_test.go b/handler_test.go index 3e5aea2c9..30b204d06 100644 --- a/handler_test.go +++ b/handler_test.go @@ -894,7 +894,7 @@ func TestHandler_Frame_AddField(t *testing.T) { ) if err != nil { t.Fatal(err) - } else if body := MustReadAll(resp.Body); string(body) != `validating: invalid field type`+"\n" { + } else if body := MustReadAll(resp.Body); string(body) != `validating field: invalid field type`+"\n" { t.Fatalf("unexpected body: %q", body) } else if err := resp.Body.Close(); err != nil { t.Fatal(err) @@ -916,7 +916,7 @@ func TestHandler_Frame_AddField(t *testing.T) { ) if err != nil { t.Fatal(err) - } else if body := MustReadAll(resp.Body); string(body) != `validating: invalid field range`+"\n" { + } else if body := MustReadAll(resp.Body); string(body) != `validating field: invalid field range`+"\n" { t.Fatalf("unexpected body: %q", body) } else if err := resp.Body.Close(); err != nil { t.Fatal(err) diff --git a/holder_test.go b/holder_test.go index 6d300d84c..16980ec5b 100644 --- a/holder_test.go +++ b/holder_test.go @@ -127,7 +127,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } - if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=foo, err=open frame: name=bar, err=loading: unmarshaling: unexpected EOF") { + if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=foo, err=open frame: name=bar, err=loading meta: unmarshaling: unexpected EOF") { t.Fatalf("unexpected error: %s", err) } }) From 7a38c98901912fbb2aee5ea181c7565f1915ff1f Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 10 May 2018 11:24:51 -0500 Subject: [PATCH 49/58] Wrap errors in api --- api.go | 134 +++++++++++++++++++++--------------------- handler_test.go | 14 ++--- server/server_test.go | 4 +- 3 files changed, 76 insertions(+), 76 deletions(-) diff --git a/api.go b/api.go index 9f7356cb8..f86a1db28 100644 --- a/api.go +++ b/api.go @@ -91,14 +91,14 @@ func (api *API) validate(f apiMethod) error { // Query parses a PQL query out of the request and executes it. func (api *API) Query(ctx context.Context, req *QueryRequest) (QueryResponse, error) { if err := api.validate(apiQuery); err != nil { - return QueryResponse{}, errors.Wrap(err, "validate api method") + return QueryResponse{}, errors.Wrap(err, "validating api method") } resp := QueryResponse{} q, err := pql.NewParser(strings.NewReader(req.Query)).Parse() if err != nil { - return resp, err + return resp, errors.Wrap(err, "parsing") } execOpts := &ExecOptions{ Remote: req.Remote, @@ -107,7 +107,7 @@ func (api *API) Query(ctx context.Context, req *QueryRequest) (QueryResponse, er } results, err := api.Executor.Execute(ctx, req.Index, q, req.Slices, execOpts) if err != nil { - return resp, err + return resp, errors.Wrap(err, "executing") } resp.Results = results @@ -126,7 +126,7 @@ func (api *API) Query(ctx context.Context, req *QueryRequest) (QueryResponse, er // Retrieve column attributes across all calls. columnAttrSets, err := api.readColumnAttrSets(api.Holder.Index(req.Index), columnIDs) if err != nil { - return resp, err + return resp, errors.Wrap(err, "reading column attrs") } resp.ColumnAttrSets = columnAttrSets } @@ -144,7 +144,7 @@ func (api *API) readColumnAttrSets(index *Index, ids []uint64) ([]*ColumnAttrSet // Read attributes for column. Skip column if empty. attrs, err := index.ColumnAttrStore().Attrs(id) if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting attrs") } else if len(attrs) == 0 { continue } @@ -159,13 +159,13 @@ func (api *API) readColumnAttrSets(index *Index, ids []uint64) ([]*ColumnAttrSet // CreateIndex makes a new Pilosa index. func (api *API) CreateIndex(ctx context.Context, indexName string, options IndexOptions) (*Index, error) { if err := api.validate(apiCreateIndex); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } // Create index. index, err := api.Holder.CreateIndex(indexName, options) if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating index") } // Send the create index message to all nodes. err = api.Broadcaster.SendSync( @@ -175,7 +175,7 @@ func (api *API) CreateIndex(ctx context.Context, indexName string, options Index }) if err != nil { api.Logger.Printf("problem sending CreateIndex message: %s", err) - return nil, err + return nil, errors.Wrap(err, "sending CreateIndex message") } api.Holder.Stats.Count("createIndex", 1, 1.0) return index, nil @@ -184,7 +184,7 @@ func (api *API) CreateIndex(ctx context.Context, indexName string, options Index // Index retrieves the named index. func (api *API) Index(ctx context.Context, indexName string) (*Index, error) { if err := api.validate(apiIndex); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } index := api.Holder.Index(indexName) @@ -198,13 +198,13 @@ func (api *API) Index(ctx context.Context, indexName string) (*Index, error) { // nothing and returns no error. func (api *API) DeleteIndex(ctx context.Context, indexName string) error { if err := api.validate(apiDeleteIndex); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } // Delete index from the holder. err := api.Holder.DeleteIndex(indexName) if err != nil { - return err + return errors.Wrap(err, "deleting index") } // Send the delete index message to all nodes. err = api.Broadcaster.SendSync( @@ -213,7 +213,7 @@ func (api *API) DeleteIndex(ctx context.Context, indexName string) error { }) if err != nil { api.Logger.Printf("problem sending DeleteIndex message: %s", err) - return err + return errors.Wrap(err, "sending DeleteIndex message") } api.Holder.Stats.Count("deleteIndex", 1, 1.0) return nil @@ -222,7 +222,7 @@ func (api *API) DeleteIndex(ctx context.Context, indexName string) error { // CreateFrame makes the named frame in the named index with the given options. func (api *API) CreateFrame(ctx context.Context, indexName string, frameName string, options FrameOptions) (*Frame, error) { if err := api.validate(apiCreateFrame); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } // Find index. @@ -234,7 +234,7 @@ func (api *API) CreateFrame(ctx context.Context, indexName string, frameName str // Create frame. frame, err := index.CreateFrame(frameName, options) if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating frame") } // Send the create frame message to all nodes. @@ -246,7 +246,7 @@ func (api *API) CreateFrame(ctx context.Context, indexName string, frameName str }) if err != nil { api.Logger.Printf("problem sending CreateFrame message: %s", err) - return nil, err + return nil, errors.Wrap(err, "sending CreateFrame message") } api.Holder.Stats.CountWithCustomTags("createFrame", 1, 1.0, []string{fmt.Sprintf("index:%s", indexName)}) return frame, nil @@ -257,7 +257,7 @@ func (api *API) CreateFrame(ctx context.Context, indexName string, frameName str // action is taken. func (api *API) DeleteFrame(ctx context.Context, indexName string, frameName string) error { if err := api.validate(apiDeleteFrame); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } // Find index. @@ -268,7 +268,7 @@ func (api *API) DeleteFrame(ctx context.Context, indexName string, frameName str // Delete frame from the index. if err := index.DeleteFrame(frameName); err != nil { - return err + return errors.Wrap(err, "deleting frame") } // Send the delete frame message to all nodes. @@ -279,7 +279,7 @@ func (api *API) DeleteFrame(ctx context.Context, indexName string, frameName str }) if err != nil { api.Logger.Printf("problem sending DeleteFrame message: %s", err) - return err + return errors.Wrap(err, "sending DeleteFrame message") } api.Holder.Stats.CountWithCustomTags("deleteFrame", 1, 1.0, []string{fmt.Sprintf("index:%s", indexName)}) return nil @@ -289,7 +289,7 @@ func (api *API) DeleteFrame(ctx context.Context, indexName string, frameName str // CSV of the form , func (api *API) ExportCSV(ctx context.Context, indexName string, frameName string, viewName string, slice uint64, w io.Writer) error { if err := api.validate(apiExportCSV); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } // Validate that this handler owns the slice. @@ -314,7 +314,7 @@ func (api *API) ExportCSV(ctx context.Context, indexName string, frameName strin strconv.FormatUint(columnID, 10), }) }); err != nil { - return err + return errors.Wrap(err, "writing CSV") } // Ensure data is flushed. @@ -326,7 +326,7 @@ func (api *API) ExportCSV(ctx context.Context, indexName string, frameName strin // SliceNodes returns the node and all replicas which should contain a slice's data. func (api *API) SliceNodes(ctx context.Context, indexName string, slice uint64) ([]*Node, error) { if err := api.validate(apiSliceNodes); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } return api.Cluster.SliceNodes(indexName, slice), nil @@ -337,7 +337,7 @@ func (api *API) SliceNodes(ctx context.Context, indexName string, slice uint64) // the UnmarshalFragment API call. func (api *API) MarshalFragment(ctx context.Context, indexName string, frameName string, viewName string, slice uint64) (io.WriterTo, error) { if err := api.validate(apiMarshalFragment); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } // Retrieve fragment from holder. @@ -353,7 +353,7 @@ func (api *API) MarshalFragment(ctx context.Context, indexName string, frameName // fragment's data. func (api *API) UnmarshalFragment(ctx context.Context, indexName string, frameName string, viewName string, slice uint64, reader io.ReadCloser) error { if err := api.validate(apiUnmarshalFragment); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } // Retrieve frame. @@ -365,18 +365,18 @@ func (api *API) UnmarshalFragment(ctx context.Context, indexName string, frameNa // Retrieve view. view, err := f.CreateViewIfNotExists(viewName) if err != nil { - return err + return errors.Wrap(err, "creating view") } // Retrieve fragment from frame. frag, err := view.CreateFragmentIfNotExists(slice) if err != nil { - return err + return errors.Wrap(err, "creating fragment") } // Read fragment in from request body. if _, err := frag.ReadFrom(reader); err != nil { - return err + return errors.Wrap(err, "reading fragment") } return nil } @@ -386,7 +386,7 @@ func (api *API) UnmarshalFragment(ctx context.Context, indexName string, frameNa // ids from a "block" which is a subdivision of a fragment. func (api *API) FragmentBlockData(ctx context.Context, body io.Reader) ([]byte, error) { if err := api.validate(apiFragmentBlockData); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } reqBytes, err := ioutil.ReadAll(body) @@ -419,7 +419,7 @@ func (api *API) FragmentBlockData(ctx context.Context, body io.Reader) ([]byte, // FragmentBlocks returns the checksums and block ids for all blocks in the specified fragment. func (api *API) FragmentBlocks(ctx context.Context, indexName string, frameName string, viewName string, slice uint64) ([]FragmentBlock, error) { if err := api.validate(apiFragmentBlocks); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } // Retrieve fragment from holder. @@ -437,7 +437,7 @@ func (api *API) FragmentBlocks(ctx context.Context, indexName string, frameName // from replicas in the cluster and restores that data to it. func (api *API) RestoreFrame(ctx context.Context, indexName string, frameName string, host *URI) error { if err := api.validate(apiRestoreFrame); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } // Create a client for the remote cluster. @@ -446,7 +446,7 @@ func (api *API) RestoreFrame(ctx context.Context, indexName string, frameName st // Determine the maximum number of slices. maxSlices, err := client.MaxSliceByIndex(ctx) if err != nil { - return err + return errors.Wrap(err, "getting max slice") } // Retrieve frame. @@ -458,7 +458,7 @@ func (api *API) RestoreFrame(ctx context.Context, indexName string, frameName st // Retrieve list of all views. views, err := client.FrameViews(ctx, indexName, frameName) if err != nil { - return err + return errors.Wrap(err, "getting views") } // Loop over each slice and import it if this node owns it. @@ -473,19 +473,19 @@ func (api *API) RestoreFrame(ctx context.Context, indexName string, frameName st // Create view. v, err := f.CreateViewIfNotExists(view) if err != nil { - return err + return errors.Wrap(err, "creating view") } // Otherwise retrieve the local fragment. frag, err := v.CreateFragmentIfNotExists(slice) if err != nil { - return err + return errors.Wrap(err, "creating fragment") } // Stream backup from remote node. rd, err := client.BackupSlice(ctx, indexName, frameName, view, slice) if err != nil { - return err + return errors.Wrap(err, "getting backup") } else if rd == nil { continue // slice doesn't exist } @@ -494,7 +494,7 @@ func (api *API) RestoreFrame(ctx context.Context, indexName string, frameName st if err := func() error { defer rd.Close() if _, err := frag.ReadFrom(rd); err != nil { - return err + return errors.Wrap(err, "reading fragment") } return nil }(); err != nil { @@ -515,7 +515,7 @@ func (api *API) Hosts(ctx context.Context) []*Node { // CreateInputDefinition is deprecated and will be removed. Do not use it. func (api *API) CreateInputDefinition(ctx context.Context, indexName string, inputDefName string, inputDef InputDefinitionInfo) error { if err := api.validate(apiCreateInputDefinition); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } api.Logger.Printf(`CreateInputDefinition is deprecated and will be removed. @@ -553,7 +553,7 @@ Please open an issue if you need to continue using it.`) // InputDefinition is deprecated and will be removed. func (api *API) InputDefinition(ctx context.Context, indexName string, inputDefName string) (*InputDefinition, error) { if err := api.validate(apiInputDefinition); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } api.Logger.Printf(`InputDefinition is deprecated and will be removed.`) @@ -573,7 +573,7 @@ func (api *API) InputDefinition(ctx context.Context, indexName string, inputDefN // DeleteInputDefinition is deprecated and will be removed. func (api *API) DeleteInputDefinition(ctx context.Context, indexName string, inputDefName string) error { if err := api.validate(apiDeleteInputDefinition); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } api.Logger.Printf("DeleteInputDefinition is deprecated and will be removed.") @@ -602,7 +602,7 @@ func (api *API) DeleteInputDefinition(ctx context.Context, indexName string, inp // WriteInput is deprecated and will be removed. func (api *API) WriteInput(ctx context.Context, indexName string, inputDefName string, reqs []interface{}) error { if err := api.validate(apiWriteInput); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } api.Logger.Printf("WriteInput is deprecated and will be removed.") @@ -630,7 +630,7 @@ func (api *API) WriteInput(ctx context.Context, indexName string, inputDefName s // RecalculateCaches forces all TopN caches to be updated. Used mainly for integration tests. func (api *API) RecalculateCaches(ctx context.Context) error { if err := api.validate(apiRecalculateCaches); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } err := api.Broadcaster.SendSync(&internal.RecalculateCaches{}) @@ -645,7 +645,7 @@ func (api *API) RecalculateCaches(ctx context.Context) error { // the body and forwards it to the BroadcastHandler. func (api *API) ClusterMessage(ctx context.Context, reqBody io.Reader) error { if err := api.validate(apiClusterMessage); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } // Read entire body. @@ -681,7 +681,7 @@ func (api *API) Schema(ctx context.Context) []*IndexInfo { // CreateField creates a new BSI field in the given index and frame. func (api *API) CreateField(ctx context.Context, indexName string, frameName string, field *Field) error { if err := api.validate(apiCreateField); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } // Retrieve frame by name. @@ -692,7 +692,7 @@ func (api *API) CreateField(ctx context.Context, indexName string, frameName str // Create new field. if err := f.CreateField(field); err != nil { - return err + return errors.Wrap(err, "creating field") } // Send the create field message to all nodes. @@ -705,13 +705,13 @@ func (api *API) CreateField(ctx context.Context, indexName string, frameName str if err != nil { api.Logger.Printf("problem sending CreateField message: %s", err) } - return err + return errors.Wrap(err, "sending CreateField message") } // DeleteField deletes the given field. func (api *API) DeleteField(ctx context.Context, indexName string, frameName string, fieldName string) error { if err := api.validate(apiDeleteField); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } // Retrieve frame by name. @@ -722,7 +722,7 @@ func (api *API) DeleteField(ctx context.Context, indexName string, frameName str // Delete field. if err := f.DeleteField(fieldName); err != nil { - return err + return errors.Wrap(err, "deleting field") } // Send the delete field message to all nodes. @@ -735,13 +735,13 @@ func (api *API) DeleteField(ctx context.Context, indexName string, frameName str if err != nil { api.Logger.Printf("problem sending DeleteField message: %s", err) } - return err + return errors.Wrap(err, "sending DeleteField message") } // Fields returns the fields in the given frame. func (api *API) Fields(ctx context.Context, indexName string, frameName string) ([]*Field, error) { if err := api.validate(apiFields); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } index := api.Holder.index(indexName) @@ -760,7 +760,7 @@ func (api *API) Fields(ctx context.Context, indexName string, frameName string) // Views returns the views in the given frame. func (api *API) Views(ctx context.Context, indexName string, frameName string) ([]*View, error) { if err := api.validate(apiViews); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } // Retrieve views. @@ -777,7 +777,7 @@ func (api *API) Views(ctx context.Context, indexName string, frameName string) ( // DeleteView removes the given view. func (api *API) DeleteView(ctx context.Context, indexName string, frameName string, viewName string) error { if err := api.validate(apiDeleteView); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } // Retrieve frame. @@ -790,7 +790,7 @@ func (api *API) DeleteView(ctx context.Context, indexName string, frameName stri if err := f.DeleteView(viewName); err != nil { // Ignore this error because views do not exist on all nodes due to slice distribution. if err != ErrInvalidView { - return err + return errors.Wrap(err, "deleting view") } } @@ -805,13 +805,13 @@ func (api *API) DeleteView(ctx context.Context, indexName string, frameName stri api.Logger.Printf("problem sending DeleteView message: %s", err) } - return err + return errors.Wrap(err, "sending DeleteView message") } // IndexAttrDiff func (api *API) IndexAttrDiff(ctx context.Context, indexName string, blocks []AttrBlock) (map[uint64]map[string]interface{}, error) { if err := api.validate(apiIndexAttrDiff); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } // Retrieve index from holder. @@ -823,7 +823,7 @@ func (api *API) IndexAttrDiff(ctx context.Context, indexName string, blocks []At // Retrieve local blocks. localBlocks, err := index.ColumnAttrStore().Blocks() if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting blocks") } // Read all attributes from all mismatched blocks. @@ -832,7 +832,7 @@ func (api *API) IndexAttrDiff(ctx context.Context, indexName string, blocks []At // Retrieve block data. m, err := index.ColumnAttrStore().BlockData(blockID) if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting block") } // Copy to index-wide struct. @@ -845,7 +845,7 @@ func (api *API) IndexAttrDiff(ctx context.Context, indexName string, blocks []At func (api *API) FrameAttrDiff(ctx context.Context, indexName string, frameName string, blocks []AttrBlock) (map[uint64]map[string]interface{}, error) { if err := api.validate(apiFrameAttrDiff); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } // Retrieve index from holder. @@ -857,7 +857,7 @@ func (api *API) FrameAttrDiff(ctx context.Context, indexName string, frameName s // Retrieve local blocks. localBlocks, err := f.RowAttrStore().Blocks() if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting blocks") } // Read all attributes from all mismatched blocks. @@ -866,7 +866,7 @@ func (api *API) FrameAttrDiff(ctx context.Context, indexName string, frameName s // Retrieve block data. m, err := f.RowAttrStore().BlockData(blockID) if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting block") } // Copy to index-wide struct. @@ -880,12 +880,12 @@ func (api *API) FrameAttrDiff(ctx context.Context, indexName string, frameName s // Import bulk imports data into a particular index,frame,slice. func (api *API) Import(ctx context.Context, req internal.ImportRequest) error { if err := api.validate(apiImport); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } _, frame, err := api.indexFrame(req.Index, req.Frame, req.Slice) if err != nil { - return err + return errors.Wrap(err, "getting frame") } // Convert timestamps to time.Time. @@ -903,18 +903,18 @@ func (api *API) Import(ctx context.Context, req internal.ImportRequest) error { if err != nil { api.Logger.Printf("import error: index=%s, frame=%s, slice=%d, bits=%d, err=%s", req.Index, req.Frame, req.Slice, len(req.ColumnIDs), err) } - return err + return errors.Wrap(err, "importing") } // ImportValue bulk imports values into a particular field. func (api *API) ImportValue(ctx context.Context, req internal.ImportValueRequest) error { if err := api.validate(apiImportValue); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } _, frame, err := api.indexFrame(req.Index, req.Frame, req.Slice) if err != nil { - return err + return errors.Wrap(err, "getting frame") } // Import into fragment. @@ -922,7 +922,7 @@ func (api *API) ImportValue(ctx context.Context, req internal.ImportValueRequest if err != nil { api.Logger.Printf("import error: index=%s, frame=%s, slice=%d, field=%s, bits=%d, err=%s", req.Index, req.Frame, req.Slice, req.Field, len(req.ColumnIDs), err) } - return err + return errors.Wrap(err, "importing") } // MaxSlices returns the maximum slice number for each index in a map. @@ -1050,7 +1050,7 @@ func (api *API) inputJSONDataParser(req map[string]interface{}, index *Index, na // SetCoordinator makes a new Node the cluster coordinator. func (api *API) SetCoordinator(ctx context.Context, id string) (oldNode, newNode *Node, err error) { if err := api.validate(apiSetCoordinator); err != nil { - return nil, nil, errors.Wrap(err, "validate api method") + return nil, nil, errors.Wrap(err, "validating api method") } oldNode = api.Cluster.NodeByID(api.Cluster.Coordinator) @@ -1080,7 +1080,7 @@ func (api *API) SetCoordinator(ctx context.Context, id string) (oldNode, newNode // removing the given node. func (api *API) RemoveNode(id string) (*Node, error) { if err := api.validate(apiRemoveNode); err != nil { - return nil, errors.Wrap(err, "validate api method") + return nil, errors.Wrap(err, "validating api method") } removeNode := api.Cluster.nodeByID(id) @@ -1099,7 +1099,7 @@ func (api *API) RemoveNode(id string) (*Node, error) { // ResizeAbort stops the current resize job. func (api *API) ResizeAbort() error { if err := api.validate(apiResizeAbort); err != nil { - return errors.Wrap(err, "validate api method") + return errors.Wrap(err, "validating api method") } err := api.Cluster.CompleteCurrentJob(ResizeJobStateAborted) diff --git a/handler_test.go b/handler_test.go index 4187b3303..9dfb801af 100644 --- a/handler_test.go +++ b/handler_test.go @@ -624,7 +624,7 @@ func TestHandler_Query_Err_JSON(t *testing.T) { h.ServeHTTP(w, test.MustNewHTTPRequest("POST", "/index/i/query", strings.NewReader(`Bitmap(id=100)`))) if w.Code != http.StatusBadRequest { t.Fatalf("unexpected status code: %d", w.Code) - } else if body := w.Body.String(); body != `{"error":"marker"}`+"\n" { + } else if body := w.Body.String(); body != `{"error":"executing: marker"}`+"\n" { t.Fatalf("unexpected body: %q", body) } } @@ -652,7 +652,7 @@ func TestHandler_Query_Err_Protobuf(t *testing.T) { var resp internal.QueryResponse if err := proto.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatal(err) - } else if s := resp.Err; s != `marker` { + } else if s := resp.Err; s != `executing: marker` { t.Fatalf("unexpected error: %s", s) } } @@ -684,7 +684,7 @@ func TestHandler_Query_ErrParse(t *testing.T) { h.ServeHTTP(w, test.MustNewHTTPRequest("POST", "/index/idx0/query?slices=0,1", strings.NewReader("bad_fn("))) if w.Code != http.StatusBadRequest { t.Fatalf("unexpected status code: %d", w.Code) - } else if body := w.Body.String(); body != `{"error":"expected comma, right paren, or identifier, found \"\" occurred at line 1, char 8"}`+"\n" { + } else if body := w.Body.String(); body != `{"error":"parsing: expected comma, right paren, or identifier, found \"\" occurred at line 1, char 8"}`+"\n" { t.Fatalf("unexpected body: %s", body) } } @@ -894,7 +894,7 @@ func TestHandler_Frame_AddField(t *testing.T) { ) if err != nil { t.Fatal(err) - } else if body := MustReadAll(resp.Body); string(body) != `invalid field type`+"\n" { + } else if body := MustReadAll(resp.Body); string(body) != `creating field: invalid field type`+"\n" { t.Fatalf("unexpected body: %q", body) } else if err := resp.Body.Close(); err != nil { t.Fatal(err) @@ -916,7 +916,7 @@ func TestHandler_Frame_AddField(t *testing.T) { ) if err != nil { t.Fatal(err) - } else if body := MustReadAll(resp.Body); string(body) != `invalid field range`+"\n" { + } else if body := MustReadAll(resp.Body); string(body) != `creating field: invalid field range`+"\n" { t.Fatalf("unexpected body: %q", body) } else if err := resp.Body.Close(); err != nil { t.Fatal(err) @@ -940,7 +940,7 @@ func TestHandler_Frame_AddField(t *testing.T) { ) if err != nil { t.Fatal(err) - } else if body := MustReadAll(resp.Body); string(body) != `field already exists`+"\n" { + } else if body := MustReadAll(resp.Body); string(body) != `creating field: field already exists`+"\n" { t.Fatalf("unexpected body: %q", body) } else if err := resp.Body.Close(); err != nil { t.Fatal(err) @@ -1006,7 +1006,7 @@ func TestHandler_Frame_DeleteField(t *testing.T) { t.Fatal(err) } else if body, err := ioutil.ReadAll(resp.Body); err != nil { t.Fatal(err) - } else if strings.TrimSpace(string(body)) != `field not found` { + } else if strings.TrimSpace(string(body)) != `deleting field: field not found` { t.Fatalf("unexpected body: %q", body) } else if err := resp.Body.Close(); err != nil { t.Fatal(err) diff --git a/server/server_test.go b/server/server_test.go index 75e0e2242..b78e8b79f 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -52,10 +52,10 @@ func TestMain_Set_Quick(t *testing.T) { // Execute SetBit() commands. for _, cmd := range cmds { - if err := client.CreateIndex(context.Background(), "i", pilosa.IndexOptions{}); err != nil && err != pilosa.ErrIndexExists { + if err := client.CreateIndex(context.Background(), "i", pilosa.IndexOptions{}); err != nil && !strings.Contains(err.Error(), "index already exists") { t.Fatal(err) } - if err := client.CreateFrame(context.Background(), "i", cmd.Frame, pilosa.FrameOptions{}); err != nil && err != pilosa.ErrFrameExists { + if err := client.CreateFrame(context.Background(), "i", cmd.Frame, pilosa.FrameOptions{}); err != nil && !strings.Contains(err.Error(), "frame already exists") { t.Fatal(err) } if _, err := m.Query("i", "", fmt.Sprintf(`SetBit(row=%d, frame=%q, col=%d)`, cmd.ID, cmd.Frame, cmd.ColumnID)); err != nil { From 4f300f5c2ef0854ed88a840bec6a605315083e31 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 10 May 2018 11:33:14 -0500 Subject: [PATCH 50/58] Wrap errors in index --- holder_test.go | 6 +++--- index.go | 38 +++++++++++++++++++------------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/holder_test.go b/holder_test.go index e819c6e94..45873959c 100644 --- a/holder_test.go +++ b/holder_test.go @@ -86,7 +86,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } - if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=test, err=invalid database") { + if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=test, err=opening attrstore: invalid database") { t.Fatalf("unexpected error: %s", err) } }) @@ -127,7 +127,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } - if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=foo, err=open frame: name=bar, err=unexpected EOF") { + if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=foo, err=opening frames: open frame: name=bar, err=unexpected EOF") { t.Fatalf("unexpected error: %s", err) } }) @@ -145,7 +145,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } - if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=foo, err=open frame: name=bar, err=invalid database") { + if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=foo, err=opening frames: open frame: name=bar, err=invalid database") { t.Fatalf("unexpected error: %s", err) } }) diff --git a/index.go b/index.go index 00ee98f11..419732fce 100644 --- a/index.go +++ b/index.go @@ -15,7 +15,6 @@ package pilosa import ( - "errors" "fmt" "io/ioutil" "os" @@ -26,6 +25,7 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pilosa/pilosa/internal" + "github.com/pkg/errors" ) // Default index settings. @@ -64,7 +64,7 @@ type Index struct { func NewIndex(path, name string) (*Index, error) { err := ValidateName(name) if err != nil { - return nil, err + return nil, errors.Wrap(err, "validating name") } return &Index{ @@ -109,20 +109,20 @@ func (i *Index) options() IndexOptions { func (i *Index) Open() error { // Ensure the path exists. if err := os.MkdirAll(i.path, 0777); err != nil { - return err + return errors.Wrap(err, "creating directory") } // Read meta file. if err := i.loadMeta(); err != nil { - return err + return errors.Wrap(err, "loading meta file") } if err := i.openFrames(); err != nil { - return err + return errors.Wrap(err, "opening frames") } if err := i.columnAttrStore.Open(); err != nil { - return err + return errors.Wrap(err, "opening attrstore") } if err := i.openInputDefinitions(); err != nil { @@ -136,13 +136,13 @@ func (i *Index) Open() error { func (i *Index) openFrames() error { f, err := os.Open(i.path) if err != nil { - return err + return errors.Wrap(err, "opening directory") } defer f.Close() fis, err := f.Readdir(0) if err != nil { - return err + return errors.Wrap(err, "reading directory") } for _, fi := range fis { @@ -171,10 +171,10 @@ func (i *Index) loadMeta() error { if os.IsNotExist(err) { return nil } else if err != nil { - return err + return errors.Wrap(err, "reading") } else { if err := proto.Unmarshal(buf, &pb); err != nil { - return err + return errors.Wrap(err, "unmarshalling") } } @@ -192,12 +192,12 @@ func (i *Index) saveMeta() error { // Marshal metadata. buf, err := proto.Marshal(&internal.IndexMeta{}) if err != nil { - return err + return errors.Wrap(err, "marshalling") } // Write to meta file. if err := ioutil.WriteFile(filepath.Join(i.path, ".meta"), buf, 0666); err != nil { - return err + return errors.Wrap(err, "writing") } return nil @@ -215,7 +215,7 @@ func (i *Index) Close() error { // Close all frames. for _, f := range i.frames { if err := f.Close(); err != nil { - return err + return errors.Wrap(err, "closing frame") } } i.frames = make(map[string]*Frame) @@ -384,18 +384,18 @@ func (i *Index) createFrame(name string, opt FrameOptions) (*Frame, error) { // Initialize frame. f, err := i.newFrame(i.FramePath(name), name) if err != nil { - return nil, err + return nil, errors.Wrap(err, "initializing") } // Open frame. if err := f.Open(); err != nil { - return nil, err + return nil, errors.Wrap(err, "opening") } // Set the time quantum. if err := f.SetTimeQuantum(opt.TimeQuantum); err != nil { f.Close() - return nil, err + return nil, errors.Wrap(err, "setting time quantum") } // Set cache type. @@ -415,7 +415,7 @@ func (i *Index) createFrame(name string, opt FrameOptions) (*Frame, error) { if err := f.saveMeta(); err != nil { f.Close() - return nil, err + return nil, errors.Wrap(err, "saving") } // Add to index's frame lookup. @@ -449,12 +449,12 @@ func (i *Index) DeleteFrame(name string) error { // Close frame. if err := f.Close(); err != nil { - return err + return errors.Wrap(err, "closing") } // Delete frame directory. if err := os.RemoveAll(i.FramePath(name)); err != nil { - return err + return errors.Wrap(err, "removing directory") } // Remove reference. From a1727205315362eee1c8884cb6d839be402a075c Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 10 May 2018 11:59:40 -0500 Subject: [PATCH 51/58] Update error check string --- holder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/holder.go b/holder.go index f048aac44..c7f9dabf4 100644 --- a/holder.go +++ b/holder.go @@ -119,7 +119,7 @@ func (h *Holder) Open() error { h.Logger.Printf("opening index: %s", filepath.Base(fi.Name())) index, err := h.newIndex(h.IndexPath(filepath.Base(fi.Name())), filepath.Base(fi.Name())) - if err == ErrName { + if errors.Cause(err) == ErrName { h.Logger.Printf("ERROR opening index: %s, err=%s", fi.Name(), err) continue } else if err != nil { From 0ba14600daf10a22cce9a257596979ee0070d86c Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 10 May 2018 12:08:46 -0500 Subject: [PATCH 52/58] Wrap a few more errors --- broadcast.go | 5 +++-- diagnostics.go | 8 +++++--- gossip/gossip.go | 2 +- server.go | 2 +- uri.go | 4 ++-- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/broadcast.go b/broadcast.go index de43f3b85..2be5e08db 100644 --- a/broadcast.go +++ b/broadcast.go @@ -20,6 +20,7 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pilosa/pilosa/internal" + "github.com/pkg/errors" ) // MemberSet represents an interface for Node membership and inter-node communication. @@ -187,7 +188,7 @@ func MarshalMessage(m proto.Message) ([]byte, error) { } buf, err := proto.Marshal(m) if err != nil { - return nil, err + return nil, errors.Wrap(err, "marshalling") } return append([]byte{typ}, buf...), nil } @@ -241,7 +242,7 @@ func UnmarshalMessage(buf []byte) (proto.Message, error) { } if err := proto.Unmarshal(buf, m); err != nil { - return nil, err + return nil, errors.Wrap(err, "unmarshalling") } return m, nil } diff --git a/diagnostics.go b/diagnostics.go index bfebbb495..46ffc2110 100644 --- a/diagnostics.go +++ b/diagnostics.go @@ -23,6 +23,8 @@ import ( "strings" "sync" "time" + + "github.com/pkg/errors" ) // Default version check URL. @@ -80,13 +82,13 @@ func (d *DiagnosticsCollector) Flush() error { d.metrics["Uptime"] = (time.Now().Unix() - d.startTime) buf, err := d.encode() if err != nil { - return err + return errors.Wrap(err, "encoding") } req, err := http.NewRequest("POST", d.host, bytes.NewReader(buf)) req.Header.Set("Content-Type", "application/json") resp, err := d.client.Do(req) if err != nil { - return err + return errors.Wrap(err, "posting") } // Intentionally ignoring response body, as user does not need to be notified of error. defer resp.Body.Close() @@ -99,7 +101,7 @@ func (d *DiagnosticsCollector) CheckVersion() error { req, err := http.NewRequest("GET", d.VersionURL, nil) resp, err := d.client.Do(req) if err != nil { - return err + return errors.Wrap(err, "getting version") } defer resp.Body.Close() diff --git a/gossip/gossip.go b/gossip/gossip.go index 49b66d8a2..2da6e3e4c 100644 --- a/gossip/gossip.go +++ b/gossip/gossip.go @@ -176,7 +176,7 @@ func NewGossipMemberSet(name string, host string, cfg Config, ger *GossipEventRe // options for _, opt := range options { if err := opt(g); err != nil { - return nil, err + return nil, errors.Wrap(err, "executing option") } } diff --git a/server.go b/server.go index c8b631580..e739fe5f0 100644 --- a/server.go +++ b/server.go @@ -629,7 +629,7 @@ func (s *Server) mergeRemoteStatus(ns *internal.NodeStatus) error { // Sync schema. if err := s.Holder.ApplySchema(ns.Schema); err != nil { - return err + return errors.Wrap(err, "applying schema") } // Sync maxSlices (standard). diff --git a/uri.go b/uri.go index beb5060b5..dfb82baef 100644 --- a/uri.go +++ b/uri.go @@ -16,13 +16,13 @@ package pilosa import ( "encoding/json" - "errors" "fmt" "regexp" "strconv" "strings" "github.com/pilosa/pilosa/internal" + "github.com/pkg/errors" ) var schemeRegexp = regexp.MustCompile("^[+a-z]+$") @@ -72,7 +72,7 @@ func NewURIFromHostPort(host string, port uint16) (*URI, error) { uri := DefaultURI() err := uri.SetHost(host) if err != nil { - return nil, err + return nil, errors.Wrap(err, "setting uri host") } uri.SetPort(port) return uri, nil From eb888b74f70e94d3377f0c0da573477f4bef7cf5 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 10 May 2018 12:12:55 -0500 Subject: [PATCH 53/58] Update test error strings --- boltdb/attrstore.go | 21 +++++++++++---------- holder_test.go | 4 ++-- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/boltdb/attrstore.go b/boltdb/attrstore.go index ebb539903..3604de5b6 100644 --- a/boltdb/attrstore.go +++ b/boltdb/attrstore.go @@ -27,6 +27,7 @@ import ( "github.com/boltdb/bolt" "github.com/pilosa/pilosa" + "github.com/pkg/errors" ) // AttrBlockSize is the size of attribute blocks for anti-entropy. @@ -93,7 +94,7 @@ func (s *AttrStore) Open() error { // Open storage. db, err := bolt.Open(s.path, 0666, &bolt.Options{Timeout: 1 * time.Second}) if err != nil { - return err + return errors.Wrap(err, "opening storage") } s.db = db @@ -104,7 +105,7 @@ func (s *AttrStore) Open() error { } return nil }); err != nil { - return err + return errors.Wrap(err, "initializing") } return nil @@ -136,7 +137,7 @@ func (s *AttrStore) Attrs(id uint64) (m map[string]interface{}, err error) { } return nil }); err != nil { - return nil, err + return nil, errors.Wrap(err, "finding attributes") } // Add to cache. @@ -154,7 +155,7 @@ func (s *AttrStore) SetAttrs(id uint64, m map[string]interface{}) error { // Check if the attributes already exist under a read-only lock. if attr, err := s.Attrs(id); err != nil { - return err + return errors.Wrap(err, "checking attrs") } else if attr != nil && mapContains(attr, m) { return nil } @@ -173,7 +174,7 @@ func (s *AttrStore) SetAttrs(id uint64, m map[string]interface{}) error { return nil }); err != nil { - return err + return errors.Wrap(err, "updating store") } // Swap attributes map in cache. @@ -222,7 +223,7 @@ func (s *AttrStore) SetBulkAttrs(m map[uint64]map[string]interface{}) error { func (s *AttrStore) Blocks() ([]pilosa.AttrBlock, error) { tx, err := s.db.Begin(false) if err != nil { - return nil, err + return nil, errors.Wrap(err, "starting transaction") } defer tx.Rollback() @@ -256,7 +257,7 @@ func (s *AttrStore) BlockData(i uint64) (map[uint64]map[string]interface{}, erro // Start read-only transaction. tx, err := s.db.Begin(false) if err != nil { - return nil, err + return nil, errors.Wrap(err, "starting transaction") } defer tx.Rollback() @@ -273,7 +274,7 @@ func (s *AttrStore) BlockData(i uint64) (map[uint64]map[string]interface{}, erro // Decode attribute map and associate with id. attrs, err := pilosa.DecodeAttrs(v) if err != nil { - return nil, err + return nil, errors.Wrap(err, "decoding attrs") } m[btou64(k)] = attrs @@ -329,10 +330,10 @@ func txUpdateAttrs(tx *bolt.Tx, id uint64, m map[string]interface{}) (map[string // Marshal and save new values. buf, err := pilosa.EncodeAttrs(attr) if err != nil { - return nil, err + return nil, errors.Wrap(err, "encoding attrs") } if err := tx.Bucket([]byte("attrs")).Put(u64tob(id), buf); err != nil { - return nil, err + return nil, errors.Wrap(err, "saving attrs") } return attr, nil } diff --git a/holder_test.go b/holder_test.go index e819c6e94..b197f0d01 100644 --- a/holder_test.go +++ b/holder_test.go @@ -86,7 +86,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } - if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=test, err=invalid database") { + if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=test, err=opening storage: invalid database") { t.Fatalf("unexpected error: %s", err) } }) @@ -145,7 +145,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } - if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=foo, err=open frame: name=bar, err=invalid database") { + if err := h.Reopen(); err == nil || !strings.Contains(err.Error(), "open index: name=foo, err=open frame: name=bar, err=opening storage: invalid database") { t.Fatalf("unexpected error: %s", err) } }) From 6e28624528d4cf7629229abdd4b434ea91cdebbd Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 10 May 2018 13:09:07 -0500 Subject: [PATCH 54/58] Wrap errors in fragment --- fragment.go | 114 ++++++++++++++++++++++++------------------------- holder_test.go | 2 +- 2 files changed, 58 insertions(+), 58 deletions(-) diff --git a/fragment.go b/fragment.go index 7168f4443..b2ce7f14d 100644 --- a/fragment.go +++ b/fragment.go @@ -21,7 +21,6 @@ import ( "container/heap" "context" "encoding/binary" - "errors" "fmt" "hash" "io" @@ -42,6 +41,7 @@ import ( "github.com/pilosa/pilosa/internal" "github.com/pilosa/pilosa/pql" "github.com/pilosa/pilosa/roaring" + "github.com/pkg/errors" ) const ( @@ -160,12 +160,12 @@ func (f *Fragment) Open() error { if err := func() error { // Initialize storage in a function so we can close if anything goes wrong. if err := f.openStorage(); err != nil { - return err + return errors.Wrap(err, "opening storage") } // Fill cache with rows persisted to disk. if err := f.openCache(); err != nil { - return err + return errors.Wrap(err, "opening cache") } // Clear checksums. @@ -206,7 +206,7 @@ func (f *Fragment) openStorage() error { // If the file is empty then initialize it with an empty bitmap. fi, err := f.file.Stat() if err != nil { - return err + return errors.Wrap(err, "statting file before") } else if fi.Size() == 0 { bi := bufio.NewWriter(f.file) if _, err := f.storage.WriteTo(bi); err != nil { @@ -215,7 +215,7 @@ func (f *Fragment) openStorage() error { bi.Flush() fi, err = f.file.Stat() if err != nil { - return err + return errors.Wrap(err, "statting file after") } } @@ -298,13 +298,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 + return errors.Wrap(err, "flushing cache") } // 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 + return errors.Wrap(err, "closing storage") } // Remove checksums. @@ -393,12 +393,12 @@ func (f *Fragment) setBit(rowID, columnID uint64) (changed bool, err error) { // Determine the position of the bit in the storage. pos, err := f.pos(rowID, columnID) if err != nil { - return false, err + return false, errors.Wrap(err, "getting bit ops") } // Write to storage. if changed, err = f.storage.Add(pos); err != nil { - return false, err + return false, errors.Wrap(err, "writing") } // Don't update the cache if nothing changed. @@ -411,7 +411,7 @@ func (f *Fragment) setBit(rowID, columnID uint64) (changed bool, err error) { // Increment number of operations until snapshot is required. if err := f.incrementOpN(); err != nil { - return false, err + return false, errors.Wrap(err, "incrementing") } // Get the row from row cache or fragment.storage. @@ -445,12 +445,12 @@ func (f *Fragment) clearBit(rowID, columnID uint64) (changed bool, err error) { // Determine the position of the bit in the storage. pos, err := f.pos(rowID, columnID) if err != nil { - return false, err + return false, errors.Wrap(err, "getting bit pos") } // Write to storage. if changed, err = f.storage.Remove(pos); err != nil { - return false, err + return false, errors.Wrap(err, "writing") } // Don't update the cache if nothing changed. @@ -463,7 +463,7 @@ func (f *Fragment) clearBit(rowID, columnID uint64) (changed bool, err error) { // Increment number of operations until snapshot is required. if err := f.incrementOpN(); err != nil { - return false, err + return false, errors.Wrap(err, "incrementing") } // Get the row from cache or fragment.storage. @@ -493,7 +493,7 @@ func (f *Fragment) FieldValue(columnID uint64, bitDepth uint) (value uint64, exi // If existence bit is unset then ignore remaining bits. if v, err := f.bit(uint64(bitDepth), columnID); err != nil { - return 0, false, err + return 0, false, errors.Wrap(err, "getting existence bit") } else if !v { return 0, false, nil } @@ -501,7 +501,7 @@ func (f *Fragment) FieldValue(columnID uint64, bitDepth uint) (value uint64, exi // Compute other bits into a value. for i := uint(0); i < bitDepth; i++ { if v, err := f.bit(uint64(i), columnID); err != nil { - return 0, false, err + return 0, false, errors.Wrapf(err, "getting value bit %d", i) } else if v { value |= (1 << i) } @@ -518,13 +518,13 @@ func (f *Fragment) SetFieldValue(columnID uint64, bitDepth uint, value uint64) ( for i := uint(0); i < bitDepth; i++ { if value&(1< Date: Thu, 10 May 2018 13:56:52 -0500 Subject: [PATCH 55/58] Address review comments --- fragment.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fragment.go b/fragment.go index b2ce7f14d..80b947a5d 100644 --- a/fragment.go +++ b/fragment.go @@ -518,13 +518,13 @@ func (f *Fragment) SetFieldValue(columnID uint64, bitDepth uint, value uint64) ( for i := uint(0); i < bitDepth; i++ { if value&(1< Date: Thu, 10 May 2018 16:02:40 -0500 Subject: [PATCH 56/58] Make error string more specific --- index.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.go b/index.go index 419732fce..8e227af83 100644 --- a/index.go +++ b/index.go @@ -415,7 +415,7 @@ func (i *Index) createFrame(name string, opt FrameOptions) (*Frame, error) { if err := f.saveMeta(); err != nil { f.Close() - return nil, errors.Wrap(err, "saving") + return nil, errors.Wrap(err, "saving meta") } // Add to index's frame lookup. From 01b56b9c09c77bb6912d073f435bcaea6dd9ce66 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 10 May 2018 17:17:45 -0500 Subject: [PATCH 57/58] Fix return type typo --- fragment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fragment.go b/fragment.go index 80b947a5d..6b5021c99 100644 --- a/fragment.go +++ b/fragment.go @@ -518,7 +518,7 @@ func (f *Fragment) SetFieldValue(columnID uint64, bitDepth uint, value uint64) ( for i := uint(0); i < bitDepth; i++ { if value&(1< Date: Thu, 10 May 2018 18:22:48 -0500 Subject: [PATCH 58/58] Fix typo --- handler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/handler_test.go b/handler_test.go index ec1bf8aa3..5877c449c 100644 --- a/handler_test.go +++ b/handler_test.go @@ -909,7 +909,7 @@ func TestHandler_Frame_AddField(t *testing.T) { ) if err != nil { t.Fatal(err) - } else if body := MustReadAll(resp.Body); string(body) != `creating field: validating field: invalid field range`+"\n" { + } else if body := MustReadAll(resp.Body); string(body) != `creating field: validating field: invalid field type`+"\n" { t.Fatalf("unexpected body: %q", body) } else if err := resp.Body.Close(); err != nil { t.Fatal(err)