From 85ebaf298d5c5f5446a62b8a447e136826611fa8 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Mon, 1 Oct 2018 11:04:47 -0500 Subject: [PATCH] removed logging from translate store replaced with error --- holder.go | 8 ++++++-- http/translator.go | 10 ++++------ http/translator_test.go | 10 +++++++--- server.go | 3 ++- server/handler_test.go | 2 +- translate.go | 13 ++++++------- 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/holder.go b/holder.go index 794104e53..80b13e3b6 100644 --- a/holder.go +++ b/holder.go @@ -52,7 +52,7 @@ type Holder struct { // Key/ID translation translateFile *TranslateFile - NewPrimaryTranslateStore func(interface{}) TranslateStore + NewPrimaryTranslateStore func(interface{}) (TranslateStore, error) // opened channel is closed once Open() completes. opened chan struct{} @@ -584,7 +584,11 @@ func (h *Holder) setPrimaryTranslateStore(node *Node) { if node != nil { nodeID = node.ID } - h.translateFile.SetPrimaryStore(nodeID, h.NewPrimaryTranslateStore(node)) + ts, err := h.NewPrimaryTranslateStore(node) + if err != nil { + h.Logger.Printf("setPrimaryTranslateStore: %s", err) + } + h.translateFile.SetPrimaryStore(nodeID, ts) } // holderSyncer is an active anti-entropy tool that compares the local holder diff --git a/http/translator.go b/http/translator.go index 7ca85a33f..f5e9f3ed3 100644 --- a/http/translator.go +++ b/http/translator.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "io/ioutil" - "log" "net/http" "net/url" "strconv" @@ -27,13 +26,12 @@ type translateStore struct { // NewTranslateStore returns a new instance of TranslateStore based on node. // DEPRECATED: Providing a string url to this function is being deprecated. Instead, // provide a *pilosa.Node. -func NewTranslateStore(node interface{}) pilosa.TranslateStore { +func NewTranslateStore(node interface{}) (pilosa.TranslateStore, error) { var n *pilosa.Node switch v := node.(type) { case string: - log.Printf("WARNING: providing a string url to NewTranslateStore() has been deprecated.") if uri, err := pilosa.NewURIFromAddress(v); err != nil { - log.Println(errors.Wrap(err, "creating uri")) + return nil, errors.Wrap(err, "creating uri") } else { n = &pilosa.Node{ ID: v, @@ -43,9 +41,9 @@ func NewTranslateStore(node interface{}) pilosa.TranslateStore { case *pilosa.Node: n = v default: - log.Printf("WARNING: a *pilosa.Node is the only type supported by NewTranslateStore().") + return nil, errors.New("*pilosa.Node is the only type supported by NewTranslateStore().") } - return &translateStore{node: n} + return &translateStore{node: n}, nil } // TranslateColumnsToUint64 is not currently implemented. diff --git a/http/translator_test.go b/http/translator_test.go index 18ba7c998..496d85cc9 100644 --- a/http/translator_test.go +++ b/http/translator_test.go @@ -42,7 +42,7 @@ func TestTranslateStore_Reader(t *testing.T) { } // Connect to server and stream all available data. - store := http.NewTranslateStore(primary.URL()) + store, _ := http.NewTranslateStore(primary.URL()) // Wait to ensure writes make it to translate store time.Sleep(500 * time.Millisecond) @@ -96,7 +96,7 @@ func TestTranslateStore_Reader(t *testing.T) { // Connect to server and begin streaming. ctx, cancel := context.WithCancel(context.Background()) - store := http.NewTranslateStore(primary.URL()) + store, _ := http.NewTranslateStore(primary.URL()) if _, err := store.Reader(ctx, 0); err != nil { t.Fatal(err) } @@ -124,7 +124,11 @@ func TestTranslateStore_Reader(t *testing.T) { primary := test.MustRunCluster(t, 1, []server.CommandOption{opts})[0] defer primary.Close() - _, err := http.NewTranslateStore(primary.URL()).Reader(context.Background(), 0) + ts, err := http.NewTranslateStore(primary.URL()) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + _, err = ts.Reader(context.Background(), 0) if err != pilosa.ErrNotImplemented { t.Fatalf("unexpected error: %s", err) } diff --git a/server.go b/server.go index 61e4838a1..ce817ba35 100644 --- a/server.go +++ b/server.go @@ -168,7 +168,8 @@ func OptServerPrimaryTranslateStore(store TranslateStore) ServerOption { } } -func OptServerPrimaryTranslateStoreFunc(tf func(interface{}) TranslateStore) ServerOption { +func OptServerPrimaryTranslateStoreFunc(tf func(interface{}) (TranslateStore, error)) ServerOption { + return func(s *Server) error { s.holder.NewPrimaryTranslateStore = tf return nil diff --git a/server/handler_test.go b/server/handler_test.go index 2c1aa9868..c0144cb6c 100644 --- a/server/handler_test.go +++ b/server/handler_test.go @@ -680,7 +680,7 @@ func TestClusterTranslator(t *testing.T) { cluster[0] = test.NewCommandNode(true) cluster[0].Config.Gossip.Port = "0" cluster[0].Start() - httpTranslateStore := http.NewTranslateStore(cluster[0].URL()) + httpTranslateStore, _ := http.NewTranslateStore(cluster[0].URL()) cluster[1] = test.NewCommandNode(false, server.OptCommandServerOptions( pilosa.OptServerPrimaryTranslateStore(httpTranslateStore), diff --git a/translate.go b/translate.go index be640a777..b875c415d 100644 --- a/translate.go +++ b/translate.go @@ -8,7 +8,6 @@ import ( "fmt" "io" "io/ioutil" - "log" "os" "path/filepath" "sync" @@ -371,14 +370,14 @@ func (s *TranslateFile) monitorReplication() { // Keep attempting to replicate until the store closes. for { if err := s.replicate(ctx); err != nil { - log.Printf("pilosa: replication error: %s", err) + s.logger.Printf("pilosa: replication error: %s", err) } select { case <-ctx.Done(): return case <-time.After(s.replicationRetryInterval): - log.Printf("pilosa: reconnecting to primary replica") + s.logger.Printf("pilosa: reconnecting to primary replica") } } } @@ -386,7 +385,7 @@ func (s *TranslateFile) monitorReplication() { // monitorPrimaryStoreEvents is executed in a separate goroutine and listens for changes // to the primary store assignment. func (s *TranslateFile) monitorPrimaryStoreEvents() { - log.Printf("monitor primary store events") + s.logger.Printf("monitor primary store events") // Keep handling events until the store closes. for { select { @@ -394,7 +393,7 @@ func (s *TranslateFile) monitorPrimaryStoreEvents() { return case ev := <-s.primaryStoreEvents: if err := s.handlePrimaryStoreEvent(ev); err != nil { - log.Printf("handle primary store event") + s.logger.Printf("handle primary store event") } } } @@ -404,7 +403,7 @@ func (s *TranslateFile) replicate(ctx context.Context) error { off := s.size() // Connect to remote primary. - log.Printf("pilosa: replicating from offset %d", off) + s.logger.Printf("pilosa: replicating from offset %d", off) rc, err := s.PrimaryTranslateStore.Reader(ctx, off) if err != nil { return err @@ -1119,7 +1118,7 @@ var nopTStore TranslateStore = nopTranslateStore{} // newNopTranslateStore returns a translate store which does nothing. It returns a global // object to avoid unnecessary allocations. -func newNopTranslateStore(interface{}) TranslateStore { return nopTStore } +func newNopTranslateStore(interface{}) (TranslateStore, error) { return nopTStore, nil } // nopTranslateStore represents a no-op implementation of the TranslateStore interface. type nopTranslateStore struct{}