From 8db7a78a937b66ce4d48f20cd1e6cfe578a390bb Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 23 Oct 2018 11:10:24 -0500 Subject: [PATCH 01/33] unpushed WIP on cluster-tests --- gossip/gossip.go | 28 ++++++++++++++++++++---- server.go | 16 ++++++++++---- server/config.go | 6 +++++ server/server.go | 6 +++++ server/server_test.go | 3 ++- test/pilosa.go | 51 ++++++++++++++++++++++++++++++++++++++++++- uri.go | 17 +++++++++++---- 7 files changed, 113 insertions(+), 14 deletions(-) diff --git a/gossip/gossip.go b/gossip/gossip.go index 789ed50b0..d03016792 100644 --- a/gossip/gossip.go +++ b/gossip/gossip.go @@ -205,8 +205,18 @@ func NewMemberSet(cfg Config, api *pilosa.API, options ...memberSetOption) (*mem conf.Name = api.Node().ID conf.BindAddr = api.Node().URI.Host conf.BindPort = port + if cfg.AdvertisePort != "" { + port, err = strconv.Atoi(cfg.Port) + if err != nil { + return nil, fmt.Errorf("convert advertise port: %s", err) + } + } conf.AdvertisePort = port - conf.AdvertiseAddr = hostToIP(api.Node().URI.Host) + if cfg.AdvertiseHost != "" { + conf.AdvertiseAddr = cfg.AdvertiseHost + } else { + conf.AdvertiseAddr = hostToIP(api.Node().URI.Host) + } // conf.TCPTimeout = time.Duration(cfg.StreamTimeout) conf.SuspicionMult = cfg.SuspicionMult @@ -447,10 +457,20 @@ func newTransport(conf *memberlist.Config) (*memberlist.NetTransport, error) { // Config holds toml-friendly memberlist configuration. type Config struct { + // Host is the host gossip will bind to. If left blank it will be set to the + // host from Pilosa. + Host string `toml:"host"` // Port indicates the port to which pilosa should bind for internal state sharing. - Port string `toml:"port"` - Seeds []string `toml:"seeds"` - Key string `toml:"key"` + Port string `toml:"port"` + // AdvertiseHost is the hostname or IP other nodes should use to connect to + // this host. If left blank, the value for Host will be used. This is useful + // in some proxy and NAT scenarios. + AdvertiseHost string `toml:"advertise-host` + // AdvertisePort is the port other nodes will use to connect to this one. + // Behaves like AdvertiseHost. + AdvertisePort string `toml:"advertise-port"` + Seeds []string `toml:"seeds"` + Key string `toml:"key"` // StreamTimeout is the timeout for establishing a stream connection with // a remote node for a full state sync, and for stream read and write // operations. Maps to memberlist TCPTimeout. diff --git a/server.go b/server.go index a4cb8fc8b..36190e315 100644 --- a/server.go +++ b/server.go @@ -62,6 +62,7 @@ type Server struct { // nolint: maligned nodeID string uri URI + advertiseURI URI antiEntropyInterval time.Duration metricInterval time.Duration diagnosticInterval time.Duration @@ -73,7 +74,7 @@ type Server struct { // nolint: maligned dataDir string } -// TODO: have this return an interface for Holder instead of concrete object? +// TODO (2.0): have this return an interface for Holder instead of concrete object? func (s *Server) Holder() *Holder { return s.holder } @@ -160,6 +161,13 @@ func OptServerInternalClient(c InternalClient) ServerOption { } } +func OptServerAdvertiseURI(u *URI) ServerOption { + return func(s *Server) error { + s.advertiseURI = *u + return nil + } +} + // DEPRECATED func OptServerPrimaryTranslateStore(store TranslateStore) ServerOption { return func(s *Server) error { @@ -296,7 +304,7 @@ func NewServer(opts ...ServerOption) (*Server, error) { // Set Cluster Node. node := &Node{ ID: s.nodeID, - URI: s.uri, + URI: s.advertiseURI, IsCoordinator: s.cluster.Coordinator == s.nodeID, } s.cluster.Node = node @@ -581,7 +589,7 @@ func (s *Server) SendSync(m Message) error { node := node s.logger.Printf("SendSync to: %s", node.URI) // Don't forward the message to ourselves. - if s.uri == node.URI { + if s.advertiseURI == node.URI { continue } @@ -676,7 +684,7 @@ func (s *Server) monitorDiagnostics() { s.diagnostics.Logger = s.logger s.diagnostics.SetVersion(Version) - s.diagnostics.Set("Host", s.uri.Host) + s.diagnostics.Set("Host", s.advertiseURI.Host) s.diagnostics.Set("Cluster", strings.Join(s.cluster.nodeIDs(), ",")) s.diagnostics.Set("NumNodes", len(s.cluster.nodes)) s.diagnostics.Set("NumCPU", runtime.NumCPU()) diff --git a/server/config.go b/server/config.go index d255e4bb6..210a2e298 100644 --- a/server/config.go +++ b/server/config.go @@ -36,9 +36,15 @@ type Config struct { // DataDir is the directory where Pilosa stores both indexed data and // running state such as cluster topology information. DataDir string `toml:"data-dir"` + // Bind is the host:port on which Pilosa will listen. Bind string `toml:"bind"` + // Advertise is the host:port that this node will report as its address to + // others. If left blank (the default), this will be set to the bind address + // once it is listening. + Advertise string `toml:"advertise"` + // MaxWritesPerRequest limits the number of mutating commands that can be in // a single request to the server. This includes Set, Clear, // SetRowAttrs & SetColumnAttrs. diff --git a/server/server.go b/server/server.go index 010eb2f8a..8521abdec 100644 --- a/server/server.go +++ b/server/server.go @@ -248,6 +248,11 @@ func (m *Command) SetupServer() error { uri.SetPort(uint16(m.ln.Addr().(*net.TCPAddr).Port)) } + advertURI, err := pilosa.NewURIFromAddressWithDefault(m.Config.Advertise, uri) + if err != nil { + return errors.Wrapf(err, "processing avertise address '%s'", m.Config.Advertise) + } + c := http.GetHTTPClient(TLSConfig) // Primary store configuration is handled automatically now. @@ -276,6 +281,7 @@ func (m *Command) SetupServer() error { pilosa.OptServerGCNotifier(gcnotify.NewActiveGCNotifier()), pilosa.OptServerStatsClient(statsClient), pilosa.OptServerURI(uri), + pilosa.OptServerAdvertiseURI(advertURI), pilosa.OptServerInternalClient(http.NewInternalClientFromURI(uri, c)), pilosa.OptServerPrimaryTranslateStoreFunc(http.NewTranslateStore), pilosa.OptServerClusterDisabled(m.Config.Cluster.Disabled, m.Config.Cluster.Hosts), diff --git a/server/server_test.go b/server/server_test.go index aa202a54c..7a5e85572 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -405,7 +405,6 @@ func TestClusteringNodesReplica1(t *testing.T) { // Create new main with the same config. config := cluster[2].Command.Config config.Translation.MapSize = 100000 - // config.Bind = cluster[2].API.Node().URI.HostPort() // this isn't necessary, but makes the test run way faster config.Gossip.Port = strconv.Itoa(int(cluster[2].Command.GossipTransport().URI.Port)) @@ -413,6 +412,8 @@ func TestClusteringNodesReplica1(t *testing.T) { cluster[2].Command = server.NewCommand(cluster[2].Stdin, cluster[2].Stdout, cluster[2].Stderr) cluster[2].Command.Config = config + time.Sleep(time.Second * 40) + // Run new program. if err := cluster[2].Start(); err != nil { t.Fatalf("restarting node 2: %v", err) diff --git a/test/pilosa.go b/test/pilosa.go index 27331d53f..46424e09d 100644 --- a/test/pilosa.go +++ b/test/pilosa.go @@ -19,11 +19,13 @@ import ( "context" "fmt" "io/ioutil" + "math/rand" gohttp "net/http" "os" "path" "strconv" "strings" + "sync/atomic" "testing" "time" @@ -31,8 +33,37 @@ import ( "github.com/pilosa/pilosa/http" "github.com/pilosa/pilosa/server" "github.com/pkg/errors" + + "github.com/Shopify/toxiproxy" + tox "github.com/Shopify/toxiproxy/client" ) +type portAllocator struct { + port *uint32 +} + +var ports *portAllocator + +// Next generates a new port and returns it +func (n *portAllocator) Next() (nextPort uint32) { + return atomic.AddUint32(n.port, 1) +} + +var proxy string + +func init() { + r := rand.New(rand.NewSource(int64(time.Now().Nanosecond()))) + p := uint32(r.Uint32()%40000 + 20000) + ports = &portAllocator{ + port: &p, + } + + tport := strconv.Itoa(int(ports.Next())) + proxy = "localhost:" + tport + go toxiproxy.NewServer().Listen("localhost", tport) + +} + //////////////////////////////////////////////////////////////////////////////////// // Command represents a test wrapper for server.Command. type Command struct { @@ -232,6 +263,8 @@ func MustNewCluster(tb testing.TB, size int, opts ...[]server.CommandOption) Clu // newCluster creates a new cluster func newCluster(size int, opts ...[]server.CommandOption) (Cluster, error) { + tclient := tox.NewClient(proxy) + if size == 0 { return nil, errors.New("cluster must contain at least one node") } @@ -245,8 +278,24 @@ func newCluster(size int, opts ...[]server.CommandOption) (Cluster, error) { if len(opts) > 0 { commandOpts = opts[i%len(opts)] } + aport := strconv.Itoa(int(ports.Next())) + name := "node" + strconv.Itoa(i) m := NewCommandNode(i == 0, commandOpts...) - err := ioutil.WriteFile(path.Join(m.Config.DataDir, ".id"), []byte("node"+strconv.Itoa(i)), 0600) + m.Config.Bind = "localhost:" + strconv.Itoa(int(ports.Next())) + m.Config.Advertise = "localhost:" + aport + p, err := tclient.CreateProxy(name+aport, m.Config.Advertise, m.Config.Bind) + if err != nil { + return nil, errors.Wrap(err, "setting up toxiproxy") + } + m.Config.Gossip.Port = strconv.Itoa(int(ports.Next())) + aport = strconv.Itoa(int(ports.Next())) + m.Config.Gossip.AdvertisePort = aport + p, err = tclient.CreateProxy(name+"-gossip"+aport, "localhost:"+m.Config.Gossip.AdvertisePort, "localhost:"+m.Config.Gossip.Port) + if err != nil { + return nil, errors.Wrap(err, "setting up toxiproxy for gossip") + } + fmt.Println(p) + err = ioutil.WriteFile(path.Join(m.Config.DataDir, ".id"), []byte(name), 0600) if err != nil { return nil, errors.Wrap(err, "writing node id") } diff --git a/uri.go b/uri.go index 231457691..7cf985f0b 100644 --- a/uri.go +++ b/uri.go @@ -82,6 +82,10 @@ func NewURIFromAddress(address string) (*URI, error) { return parseAddress(address) } +func NewURIFromAddressWithDefault(address string, base *URI) (*URI, error) { + return parseAddressWithDefault(address, base) +} + // setScheme sets the scheme of this URI. func (u *URI) setScheme(scheme string) error { m := schemeRegexp.FindStringSubmatch(scheme) @@ -154,20 +158,20 @@ func (u URI) Type() string { return "URI" } -func parseAddress(address string) (uri *URI, err error) { +func parseAddressWithDefault(address string, def *URI) (uri *URI, err error) { m := addressRegexp.FindStringSubmatch(address) if m == nil { return nil, errors.New("invalid address") } - scheme := "http" + scheme := def.Scheme if m[2] != "" { scheme = m[2] } - host := "localhost" + host := def.Host if m[3] != "" { host = m[3] } - var port = 10101 + var port = int(def.Port) if m[5] != "" { port, err = strconv.Atoi(m[5]) if err != nil { @@ -185,6 +189,11 @@ func parseAddress(address string) (uri *URI, err error) { return uri, nil } +func parseAddress(address string) (uri *URI, err error) { + u, err := parseAddressWithDefault(address, defaultURI()) + return u, err +} + // MarshalJSON marshals URI into a JSON-encoded byte slice. func (u *URI) MarshalJSON() ([]byte, error) { var output struct { From f643487ce086beb616a9ac00a500eb26df6f94f2 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Wed, 24 Oct 2018 16:55:39 -0500 Subject: [PATCH 02/33] add initial UDP proxy code to support proxying/partitioning memberlist --- internal/udproxy/udproxy.go | 145 +++++++++++++++++++++++++++++++ internal/udproxy/udproxy_test.go | 63 ++++++++++++++ 2 files changed, 208 insertions(+) create mode 100644 internal/udproxy/udproxy.go create mode 100644 internal/udproxy/udproxy_test.go diff --git a/internal/udproxy/udproxy.go b/internal/udproxy/udproxy.go new file mode 100644 index 000000000..fa30b22a5 --- /dev/null +++ b/internal/udproxy/udproxy.go @@ -0,0 +1,145 @@ +package udproxy + +import ( + "bytes" + "io" + "net" + "sync" + "time" + + "github.com/pkg/errors" + "golang.org/x/sync/errgroup" +) + +type Proxy struct { + upstreamAddr *net.UDPAddr + conn *net.UDPConn + + drop bool + dropLock sync.Mutex + + quit chan struct{} + eg errgroup.Group + // map from client address to upstream connection. We must maintain a + // separate connection to upstream for each client connection so that we can + // differentiate data sent back from upstream. + upstreams map[*net.UDPAddr]*net.UDPConn + // TODO - need to track a per-connection timeout so that "upstreams" doesn't + // grow indefinitely. +} + +func New(listenIP string, listenPort int, upstreamIP string, upstreamPort int) (*Proxy, error) { + uc, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP(listenIP), Port: listenPort}) + if err != nil { + return nil, errors.Wrap(err, "listening") + } + p := &Proxy{ + conn: uc, + upstreamAddr: &net.UDPAddr{IP: net.ParseIP(upstreamIP), Port: upstreamPort}, + quit: make(chan struct{}), + upstreams: make(map[*net.UDPAddr]*net.UDPConn), + } + if p.upstreamAddr.IP == nil { + return nil, errors.Errorf("unable to parse upstream ip '%s'", upstreamIP) + } + p.eg.Go(p.run) + return p, nil +} + +func (p *Proxy) Drop() { + p.dropLock.Lock() + p.drop = true + p.dropLock.Unlock() +} + +func (p *Proxy) Undrop() { + p.dropLock.Lock() + p.drop = false + p.dropLock.Unlock() +} + +func (p *Proxy) dropping() bool { + p.dropLock.Lock() + d := p.drop + p.dropLock.Unlock() + return d +} + +func (p *Proxy) run() error { + buf := make([]byte, 65507) + for { + select { + case <-p.quit: + return nil + default: + } + err := p.conn.SetReadDeadline(time.Now().Add(time.Millisecond * 10)) + if err != nil { + return errors.Wrap(err, "setting read deadline (run)") + } + n, addr, err := p.conn.ReadFromUDP(buf) + if err, ok := err.(net.Error); ok && err.Timeout() { + continue + } else if err != nil { + return errors.Wrap(err, "reading from udp conn") + } + upConn := p.upstreams[addr] + if upConn == nil { + p.upstreams[addr], err = net.DialUDP("udp", &net.UDPAddr{}, p.upstreamAddr) + if err != nil { + return errors.Wrap(err, "creating new connection to upstream") + } + p.eg.Go(func() error { + return p.proxyBack(addr, p.upstreams[addr]) + }) + upConn = p.upstreams[addr] + } + if !p.dropping() { + _, err = io.Copy(upConn, bytes.NewBuffer(buf[:n])) + if err != nil { + return errors.Wrap(err, "writing to upstream conn") + } + } + } +} + +func (p *Proxy) proxyBack(to *net.UDPAddr, from *net.UDPConn) error { + buf := make([]byte, 65507) + for { + select { + case <-p.quit: + return nil + default: + } + err := from.SetReadDeadline(time.Now().Add(time.Millisecond * 10)) + if err != nil { + return errors.Wrap(err, "setting read deadline (proxyBack)") + } + n, _, err := from.ReadFromUDP(buf) + if err, ok := err.(net.Error); ok && err.Timeout() { + continue + } else if err != nil { + return errors.Wrap(err, "reading from upstream") + } + if !p.dropping() { + _, err = io.Copy(addrWriter{c: p.conn, a: to}, bytes.NewBuffer(buf[:n])) + if err != nil { + return errors.Wrap(err, "writing back to client") + } + } + } +} + +type addrWriter struct { + c *net.UDPConn + a *net.UDPAddr +} + +func (a addrWriter) Write(b []byte) (n int, err error) { + return a.c.WriteTo(b, a.a) +} + +func (p *Proxy) Close() error { + close(p.quit) + return p.eg.Wait() +} diff --git a/internal/udproxy/udproxy_test.go b/internal/udproxy/udproxy_test.go new file mode 100644 index 000000000..edb915d1f --- /dev/null +++ b/internal/udproxy/udproxy_test.go @@ -0,0 +1,63 @@ +package udproxy_test + +import ( + "net" + "testing" + + "github.com/pilosa/pilosa/internal/udproxy" + "github.com/pkg/errors" + "golang.org/x/sync/errgroup" +) + +func TestUDProxy(t *testing.T) { + p, err := udproxy.New("127.0.0.1", 12345, "127.0.0.1", 12346) + if err != nil { + t.Fatalf("creating proxy: %v", err) + } + + uc, err := net.ListenUDP("udp", &net.UDPAddr{Port: 12346}) + if err != nil { + t.Fatalf("listening udp upstream: %v", err) + } + + resp := make([]byte, 8) + eg := errgroup.Group{} + eg.Go(func() error { + conn, err := net.DialUDP("udp", nil, &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 12345}) + if err != nil { + t.Fatalf("connecting to proxy: %v", err) + } + _, err = conn.Write([]byte("hello!")) + if err != nil { + return errors.Wrap(err, "writing to proxy") + } + _, err = conn.Read(resp) + if err != nil { + return errors.Wrap(err, "reading from proxy") + } + return nil + }) + + req := make([]byte, 10) + _, addr, err := uc.ReadFrom(req) + if err != nil { + t.Fatalf("upstream reading from proxy: %v", err) + } + if string(req[:6]) != "hello!" { + t.Fatalf("got unexpected request %s", req) + } + _, err = uc.WriteTo([]byte("goodbye"), addr) + if err != nil { + t.Fatalf("writing response: %v", err) + } + eg.Wait() + if string(resp[:7]) != "goodbye" { + t.Fatalf("got unexpected response '%v", resp) + } + err = p.Close() + if err != nil { + t.Fatalf("err closing proxy: '%v'", err) + } +} + +// TODO test dropping From 28591b4a916659166a6345ae2ad373f7d9d23572 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Wed, 24 Oct 2018 16:56:32 -0500 Subject: [PATCH 03/33] WIP: figure out why config.Gossip.Port is 0 after cluster start --- server/server_test.go | 2 ++ test/pilosa.go | 18 +++++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/server/server_test.go b/server/server_test.go index 7a5e85572..3900a7faa 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -407,7 +407,9 @@ func TestClusteringNodesReplica1(t *testing.T) { config.Translation.MapSize = 100000 // this isn't necessary, but makes the test run way faster + fmt.Println("!!!!!!!!!!", config.Gossip.Port) config.Gossip.Port = strconv.Itoa(int(cluster[2].Command.GossipTransport().URI.Port)) + fmt.Println("!!!!!!!!!!!!!!!!", config.Gossip.Port) cluster[2].Command = server.NewCommand(cluster[2].Stdin, cluster[2].Stdout, cluster[2].Stderr) cluster[2].Command.Config = config diff --git a/test/pilosa.go b/test/pilosa.go index 46424e09d..901e8ffd2 100644 --- a/test/pilosa.go +++ b/test/pilosa.go @@ -34,8 +34,8 @@ import ( "github.com/pilosa/pilosa/server" "github.com/pkg/errors" - "github.com/Shopify/toxiproxy" - tox "github.com/Shopify/toxiproxy/client" + "github.com/jaffee/toxiproxy" + tox "github.com/jaffee/toxiproxy/client" ) type portAllocator struct { @@ -283,18 +283,22 @@ func newCluster(size int, opts ...[]server.CommandOption) (Cluster, error) { m := NewCommandNode(i == 0, commandOpts...) m.Config.Bind = "localhost:" + strconv.Itoa(int(ports.Next())) m.Config.Advertise = "localhost:" + aport - p, err := tclient.CreateProxy(name+aport, m.Config.Advertise, m.Config.Bind) + _, err := tclient.CreateProxy(name+aport, m.Config.Advertise, m.Config.Bind) if err != nil { return nil, errors.Wrap(err, "setting up toxiproxy") } - m.Config.Gossip.Port = strconv.Itoa(int(ports.Next())) - aport = strconv.Itoa(int(ports.Next())) + gossipBindPort := strconv.Itoa(int(ports.Next())) + m.Config.Gossip.Port = gossipBindPort + gossipAdvertPort := strconv.Itoa(int(ports.Next())) m.Config.Gossip.AdvertisePort = aport - p, err = tclient.CreateProxy(name+"-gossip"+aport, "localhost:"+m.Config.Gossip.AdvertisePort, "localhost:"+m.Config.Gossip.Port) + _, err = tclient.CreateProxy(name+"-gossip"+gossipAdvertPort, "localhost:"+m.Config.Gossip.AdvertisePort, "localhost:"+m.Config.Gossip.Port) if err != nil { return nil, errors.Wrap(err, "setting up toxiproxy for gossip") } - fmt.Println(p) + _, err = tclient.CreateProxy(name+"-gossipudp"+gossipAdvertPort, "localhost:"+m.Config.Gossip.AdvertisePort, "localhost:"+m.Config.Gossip.Port, tox.CreateWithProtocol("udp")) + if err != nil { + return nil, errors.Wrap(err, "setting up toxiproxy for udp gossip") + } err = ioutil.WriteFile(path.Join(m.Config.DataDir, ".id"), []byte(name), 0600) if err != nil { return nil, errors.Wrap(err, "writing node id") From af7ece74fd031be8589c4e9c158fa06df8948488 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 26 Oct 2018 09:17:26 -0500 Subject: [PATCH 04/33] test drop and undrop in udproxy. --- internal/udproxy/udproxy.go | 10 ++++++++-- internal/udproxy/udproxy_test.go | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/internal/udproxy/udproxy.go b/internal/udproxy/udproxy.go index fa30b22a5..a79c0121f 100644 --- a/internal/udproxy/udproxy.go +++ b/internal/udproxy/udproxy.go @@ -47,12 +47,18 @@ func New(listenIP string, listenPort int, upstreamIP string, upstreamPort int) ( } func (p *Proxy) Drop() { + // sleep to attempt to ensure all traffic that was supposed to pass, did + // pass. + time.Sleep(time.Millisecond * 3) p.dropLock.Lock() p.drop = true p.dropLock.Unlock() } func (p *Proxy) Undrop() { + // this sleep is a cheap attempt to ensure everything that was supposed to + // be dropped was. + time.Sleep(time.Millisecond * 3) p.dropLock.Lock() p.drop = false p.dropLock.Unlock() @@ -73,7 +79,7 @@ func (p *Proxy) run() error { return nil default: } - err := p.conn.SetReadDeadline(time.Now().Add(time.Millisecond * 10)) + err := p.conn.SetReadDeadline(time.Now().Add(time.Millisecond)) if err != nil { return errors.Wrap(err, "setting read deadline (run)") } @@ -111,7 +117,7 @@ func (p *Proxy) proxyBack(to *net.UDPAddr, from *net.UDPConn) error { return nil default: } - err := from.SetReadDeadline(time.Now().Add(time.Millisecond * 10)) + err := from.SetReadDeadline(time.Now().Add(time.Millisecond)) if err != nil { return errors.Wrap(err, "setting read deadline (proxyBack)") } diff --git a/internal/udproxy/udproxy_test.go b/internal/udproxy/udproxy_test.go index edb915d1f..be746ea72 100644 --- a/internal/udproxy/udproxy_test.go +++ b/internal/udproxy/udproxy_test.go @@ -35,6 +35,16 @@ func TestUDProxy(t *testing.T) { if err != nil { return errors.Wrap(err, "reading from proxy") } + p.Drop() + _, err = conn.Write([]byte("hello2")) + if err != nil { + return errors.Wrap(err, "writing to dropping proxy") + } + p.Undrop() + _, err = conn.Write([]byte("hello3")) + if err != nil { + return errors.Wrap(err, "writing to undropping proxy") + } return nil }) @@ -50,10 +60,20 @@ func TestUDProxy(t *testing.T) { if err != nil { t.Fatalf("writing response: %v", err) } + + _, _, err = uc.ReadFrom(req) + if err != nil { + t.Fatalf("upstream reading from proxy: %v", err) + } + if string(req[:6]) != "hello3" { + t.Fatalf("got unexpected request %s", req) + } + eg.Wait() if string(resp[:7]) != "goodbye" { t.Fatalf("got unexpected response '%v", resp) } + err = p.Close() if err != nil { t.Fatalf("err closing proxy: '%v'", err) From 62c1bfe90ecfd2b074f02f36a31f63125f54432b Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 26 Oct 2018 16:13:20 -0500 Subject: [PATCH 05/33] implement MustNewClusterWithProxy and drop/undrop for partitioning --- server/server_test.go | 19 ++++++-- test/pilosa.go | 111 +++++++++++++++++++++++++++++++----------- 2 files changed, 97 insertions(+), 33 deletions(-) diff --git a/server/server_test.go b/server/server_test.go index 3900a7faa..f30c02774 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -379,7 +379,11 @@ func (p uint64Slice) Len() int { return len(p) } func (p uint64Slice) Less(i, j int) bool { return p[i] < p[j] } func TestClusteringNodesReplica1(t *testing.T) { - cluster := test.MustRunCluster(t, 3) + cluster := test.MustNewCluster(t, 3) + err := cluster.Start() + if err != nil { + t.Fatalf("starting cluster: %v", err) + } defer cluster.Close() var wait = true @@ -407,15 +411,11 @@ func TestClusteringNodesReplica1(t *testing.T) { config.Translation.MapSize = 100000 // this isn't necessary, but makes the test run way faster - fmt.Println("!!!!!!!!!!", config.Gossip.Port) config.Gossip.Port = strconv.Itoa(int(cluster[2].Command.GossipTransport().URI.Port)) - fmt.Println("!!!!!!!!!!!!!!!!", config.Gossip.Port) cluster[2].Command = server.NewCommand(cluster[2].Stdin, cluster[2].Stdout, cluster[2].Stderr) cluster[2].Command.Config = config - time.Sleep(time.Second * 40) - // Run new program. if err := cluster[2].Start(); err != nil { t.Fatalf("restarting node 2: %v", err) @@ -696,3 +696,12 @@ func TestClusterQueriesAfterRestart(t *testing.T) { } // TODO: confirm that things keep working if a node is hard-closed (no nodeLeave event) and immediately restarted with a different address. + +func TestClusterPartitioning(t *testing.T) { + cluster := test.MustNewClusterWithProxy(t, 3) + err := cluster.Start() + if err != nil { + t.Fatalf("starting cluster with proxy: %v", err) + } + +} diff --git a/test/pilosa.go b/test/pilosa.go index 901e8ffd2..cf197e163 100644 --- a/test/pilosa.go +++ b/test/pilosa.go @@ -31,11 +31,12 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/http" + "github.com/pilosa/pilosa/internal/udproxy" "github.com/pilosa/pilosa/server" "github.com/pkg/errors" - "github.com/jaffee/toxiproxy" - tox "github.com/jaffee/toxiproxy/client" + "github.com/Shopify/toxiproxy" + tox "github.com/Shopify/toxiproxy/client" ) type portAllocator struct { @@ -69,9 +70,49 @@ func init() { type Command struct { *server.Command + proxies commandProxies + commandOptions []server.CommandOption } +// Drop uses the proxies to drop all network traffic to/from this node. +func (com Command) Drop(tb testing.TB) { + if com.proxies.http == nil { + tb.Fatal("can't drop traffic if cluster wasn't created with proxy support.") + } + err := com.proxies.http.Disable() + if err != nil { + tb.Fatalf("disabling http proxy: %v", err) + } + err = com.proxies.memberTCP.Disable() + if err != nil { + tb.Fatalf("disabling memberlist tcp proxy: %v", err) + } + com.proxies.memberUDP.Drop() +} + +// Undrop starts forwarding traffic to/from this node after a previous drop request. +func (com Command) Undrop(tb testing.TB) { + if com.proxies.http == nil { + tb.Fatal("can't drop traffic if cluster wasn't created with proxy support.") + } + err := com.proxies.http.Enable() + if err != nil { + tb.Fatalf("enabling http proxy: %v", err) + } + err = com.proxies.memberTCP.Enable() + if err != nil { + tb.Fatalf("enabling memberlist tcp proxy: %v", err) + } + com.proxies.memberUDP.Undrop() +} + +type commandProxies struct { + http *tox.Proxy + memberTCP *tox.Proxy + memberUDP *udproxy.Proxy +} + func OptAllowedOrigins(origins []string) server.CommandOption { return func(m *server.Command) error { m.Config.Handler.AllowedOrigins = origins @@ -232,7 +273,6 @@ type Cluster []*Command func (c Cluster) Start() error { var gossipSeeds = make([]string, len(c)) for i, cc := range c { - cc.Config.Gossip.Port = "0" cc.Config.Gossip.Seeds = gossipSeeds[:i] if err := cc.Start(); err != nil { return errors.Wrapf(err, "starting server %d", i) @@ -254,7 +294,17 @@ func (c Cluster) Close() error { // MustNewCluster creates a new cluster func MustNewCluster(tb testing.TB, size int, opts ...[]server.CommandOption) Cluster { - c, err := newCluster(size, opts...) + c, err := newCluster(size, false, opts...) + if err != nil { + tb.Fatalf("new cluster: %v", err) + } + return c +} + +// MustNewClusterWithProxy returns a test cluster which has all connections +// going through a proxy to allow for testing network partitions. +func MustNewClusterWithProxy(tb testing.TB, size int, opts ...[]server.CommandOption) Cluster { + c, err := newCluster(size, true, opts...) if err != nil { tb.Fatalf("new cluster: %v", err) } @@ -262,9 +312,7 @@ func MustNewCluster(tb testing.TB, size int, opts ...[]server.CommandOption) Clu } // newCluster creates a new cluster -func newCluster(size int, opts ...[]server.CommandOption) (Cluster, error) { - tclient := tox.NewClient(proxy) - +func newCluster(size int, withproxy bool, opts ...[]server.CommandOption) (cluster Cluster, err error) { if size == 0 { return nil, errors.New("cluster must contain at least one node") } @@ -272,33 +320,40 @@ func newCluster(size int, opts ...[]server.CommandOption) (Cluster, error) { return nil, errors.New("Slice of CommandOptions must be of length 0, 1, or equal to the number of cluster nodes") } - cluster := make(Cluster, size) + cluster = make(Cluster, size) for i := 0; i < size; i++ { var commandOpts []server.CommandOption if len(opts) > 0 { commandOpts = opts[i%len(opts)] } - aport := strconv.Itoa(int(ports.Next())) name := "node" + strconv.Itoa(i) m := NewCommandNode(i == 0, commandOpts...) m.Config.Bind = "localhost:" + strconv.Itoa(int(ports.Next())) - m.Config.Advertise = "localhost:" + aport - _, err := tclient.CreateProxy(name+aport, m.Config.Advertise, m.Config.Bind) - if err != nil { - return nil, errors.Wrap(err, "setting up toxiproxy") - } - gossipBindPort := strconv.Itoa(int(ports.Next())) - m.Config.Gossip.Port = gossipBindPort - gossipAdvertPort := strconv.Itoa(int(ports.Next())) - m.Config.Gossip.AdvertisePort = aport - _, err = tclient.CreateProxy(name+"-gossip"+gossipAdvertPort, "localhost:"+m.Config.Gossip.AdvertisePort, "localhost:"+m.Config.Gossip.Port) - if err != nil { - return nil, errors.Wrap(err, "setting up toxiproxy for gossip") - } - _, err = tclient.CreateProxy(name+"-gossipudp"+gossipAdvertPort, "localhost:"+m.Config.Gossip.AdvertisePort, "localhost:"+m.Config.Gossip.Port, tox.CreateWithProtocol("udp")) - if err != nil { - return nil, errors.Wrap(err, "setting up toxiproxy for udp gossip") + gossipBindPort := int(ports.Next()) + m.Config.Gossip.Port = strconv.Itoa(gossipBindPort) + + if withproxy { + tclient := tox.NewClient(proxy) + + aport := strconv.Itoa(int(ports.Next())) + m.Config.Advertise = "localhost:" + aport + m.proxies.http, err = tclient.CreateProxy(name+aport, m.Config.Advertise, m.Config.Bind) + if err != nil { + return nil, errors.Wrap(err, "setting up toxiproxy") + } + gossipAdvertPort := int(ports.Next()) + m.Config.Gossip.AdvertisePort = strconv.Itoa(gossipAdvertPort) + m.proxies.memberTCP, err = tclient.CreateProxy(name+"-gossip"+m.Config.Gossip.AdvertisePort, "localhost:"+m.Config.Gossip.AdvertisePort, "localhost:"+m.Config.Gossip.Port) + if err != nil { + return nil, errors.Wrap(err, "setting up toxiproxy for gossip") + } + m.proxies.memberUDP, err = udproxy.New("127.0.0.1", gossipAdvertPort, "127.0.0.1", gossipBindPort) + if err != nil { + return nil, errors.Wrap(err, "setting up proxy for udp gossip") + } + } + err = ioutil.WriteFile(path.Join(m.Config.DataDir, ".id"), []byte(name), 0600) if err != nil { return nil, errors.Wrap(err, "writing node id") @@ -310,8 +365,8 @@ func newCluster(size int, opts ...[]server.CommandOption) (Cluster, error) { } // runCluster creates and starts a new cluster -func runCluster(size int, opts ...[]server.CommandOption) (Cluster, error) { - cluster, err := newCluster(size, opts...) +func runCluster(size int, withproxy bool, opts ...[]server.CommandOption) (Cluster, error) { + cluster, err := newCluster(size, withproxy, opts...) if err != nil { return nil, errors.Wrap(err, "new cluster") } @@ -323,7 +378,7 @@ func runCluster(size int, opts ...[]server.CommandOption) (Cluster, error) { // MustRunCluster creates and starts a new cluster func MustRunCluster(tb testing.TB, size int, opts ...[]server.CommandOption) Cluster { - c, err := runCluster(size, opts...) + c, err := runCluster(size, false, opts...) if err != nil { tb.Fatalf("run cluster: %v", err) } From d86e9ed3ea11aa3d87963cd044d0ed0955a849c4 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 2 Nov 2018 14:41:42 -0500 Subject: [PATCH 06/33] add docker based cluster tests, remove proxy stuff, fix advertise --- Dockerfile-withgo | 22 ++++ Gopkg.lock | 11 +- ctl/server.go | 1 + internal/clustertests/Dockerfile | 3 + internal/clustertests/cluster_test.go | 138 +++++++++++++++++++++ internal/clustertests/docker-compose.yml | 53 ++++++++ internal/udproxy/udproxy.go | 151 ----------------------- internal/udproxy/udproxy_test.go | 83 ------------- server/server_test.go | 15 +-- test/pilosa.go | 124 ++----------------- 10 files changed, 236 insertions(+), 365 deletions(-) create mode 100644 Dockerfile-withgo create mode 100644 internal/clustertests/Dockerfile create mode 100644 internal/clustertests/cluster_test.go create mode 100644 internal/clustertests/docker-compose.yml delete mode 100644 internal/udproxy/udproxy.go delete mode 100644 internal/udproxy/udproxy_test.go diff --git a/Dockerfile-withgo b/Dockerfile-withgo new file mode 100644 index 000000000..5cb11e47b --- /dev/null +++ b/Dockerfile-withgo @@ -0,0 +1,22 @@ +FROM golang:1.11 + +LABEL maintainer "dev@pilosa.com" + +COPY . /go/src/github.com/pilosa/pilosa/ + +RUN cd /go/src/github.com/pilosa/pilosa \ + && CGO_ENABLED=0 make install-dep install FLAGS="-a" + +RUN wget https://github.com/alexei-led/pumba/releases/download/0.6.0/pumba_linux_amd64 -O /pumba +RUN chmod +x /pumba + +RUN cp /go/bin/pilosa /pilosa + +COPY LICENSE /LICENSE +COPY NOTICE /NOTICE + +EXPOSE 10101 +VOLUME /data + +ENTRYPOINT ["bash", "-c"] +CMD ["pilosa", "server", "--data-dir", "/data", "--bind", "http://0.0.0.0:10101"] diff --git a/Gopkg.lock b/Gopkg.lock index 4a6068840..280f6d056 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -189,6 +189,15 @@ revision = "c01d1270ff3e442a8a57cddc1c92dc1138598194" version = "v1.2.0" +[[projects]] + name = "github.com/pilosa/go-pilosa" + packages = [ + ".", + "gopilosa_pbuf" + ] + revision = "4e7807f5ad779407936744057cd17332046b6c3c" + version = "v1.1.0" + [[projects]] name = "github.com/pkg/errors" packages = ["."] @@ -324,6 +333,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "8290156ce8b4066c46ab83d743f4c81df0a17e148415bb1ee8409a51ac4c3ba4" + inputs-digest = "be318fa4f2a72e7e849b2faff1ce9300deaaf2d24cf76100f4b538abed86295b" solver-name = "gps-cdcl" solver-version = 1 diff --git a/ctl/server.go b/ctl/server.go index 7f384ec7b..90766c498 100644 --- a/ctl/server.go +++ b/ctl/server.go @@ -26,6 +26,7 @@ func BuildServerFlags(cmd *cobra.Command, srv *server.Command) { flags := cmd.Flags() flags.StringVarP(&srv.Config.DataDir, "data-dir", "d", srv.Config.DataDir, "Directory to store pilosa data files.") flags.StringVarP(&srv.Config.Bind, "bind", "b", srv.Config.Bind, "Default URI on which pilosa should listen.") + flags.StringVarP(&srv.Config.Advertise, "advertise", "a", "", "Address to broadcast to other hosts and clients to be contacted on.") flags.IntVarP(&srv.Config.MaxWritesPerRequest, "max-writes-per-request", "", srv.Config.MaxWritesPerRequest, "Number of write commands per request.") flags.StringVar(&srv.Config.LogPath, "log-path", srv.Config.LogPath, "Log path") flags.BoolVar(&srv.Config.Verbose, "verbose", srv.Config.Verbose, "Enable verbose logging") diff --git a/internal/clustertests/Dockerfile b/internal/clustertests/Dockerfile new file mode 100644 index 000000000..ecbfa7f21 --- /dev/null +++ b/internal/clustertests/Dockerfile @@ -0,0 +1,3 @@ +FROM ptest + +COPY . /go/src/github.com/pilosa/pilosa/internal/clustertests diff --git a/internal/clustertests/cluster_test.go b/internal/clustertests/cluster_test.go new file mode 100644 index 000000000..e63b95d0b --- /dev/null +++ b/internal/clustertests/cluster_test.go @@ -0,0 +1,138 @@ +package clustertest + +import ( + "fmt" + "io" + "os" + "os/exec" + "testing" + "time" + + "github.com/pilosa/go-pilosa" + pi "github.com/pilosa/pilosa" +) + +func TestLongPauses(t *testing.T) { + t.Skip() + cli := getPilosaClient(t) + + idx := pilosa.NewIndex("testidx") + err := cli.CreateIndex(idx) + if err != nil { + t.Fatalf("creating index: %v", err) + } + f := idx.Field("testf", pilosa.OptFieldTypeSet(pilosa.CacheTypeRanked, 10)) + err = cli.CreateField(f) + if err != nil { + t.Fatalf("creating field: %v", err) + } + + data := make([]pilosa.Column, 1000) + for i := range data { + data[i].RowID = 0 + data[i].ColumnID = uint64((i/10)*pi.ShardWidth + i%10) + } + + err = cli.ImportField(f, &colIterator{cols: data}, pilosa.OptImportBatchSize(1000)) + if err != nil { + t.Fatalf("importing: %v", err) + } + + r, err := cli.Query(idx.Count(f.Row(0))) + if err != nil { + t.Fatalf("count querying: %v", err) + } + if r.Result().Count() != 1000 { + t.Fatalf("count after import is %d", r.Result().Count()) + } + + pcmd := exec.Command("/pumba", "pause", "clustertests_pilosa3_1", "--duration", "30s") + pcmd.Stdout = os.Stdout + pcmd.Stderr = os.Stderr + fmt.Println("pausing pilosa3 for 30s") + err = pcmd.Start() + if err != nil { + t.Fatalf("starting pumba command: %v", err) + } + err = pcmd.Wait() + if err != nil { + t.Fatalf("waiting on pumba pause cmd: %v", err) + } + fmt.Println("done with pause, waiting for stability") + time.Sleep(time.Second * 400) + fmt.Println("done waiting") + + r, err = cli.Query(idx.Count(f.Row(0))) + if err != nil { + t.Fatalf("count querying: %v", err) + } + if r.Result().Count() != 1000 { + t.Fatalf("count after import is %d", r.Result().Count()) + } +} + +// Utils + +func getPilosaClient(t *testing.T) *pilosa.Client { + cli, err := pilosa.NewClient("pilosa1:10101") + if err != nil { + time.Sleep(time.Millisecond * 40) + } + time.Sleep(time.Second * 2) + // start := time.Now() + // for i := 0; true; i++ { + // s, err := cli.Status() + // if i > 800 { + // t.Fatalf("couldn't connect to cluster after %d attempts and %v: state: %s, err: %v", i, time.Since(start), s.State, err) + // } + // if err != nil { + // time.Sleep(time.Millisecond * 40) + // continue + // } + // if s.State == "NORMAL" { + // break + // } else { + // time.Sleep(time.Millisecond * 40) + // } + // } + + return cli +} + +type colIterator struct { + cols []pilosa.Column + i uint +} + +func (c *colIterator) NextRecord() (pilosa.Record, error) { + if int(c.i) >= len(c.cols) { + return nil, io.EOF + } + c.i++ + return c.cols[c.i-1], nil +} + +func TestColIterator(t *testing.T) { + data := make([]pilosa.Column, 3) + for i := range data { + data[i].RowID = 0 + data[i].ColumnID = uint64((i/10)*pi.ShardWidth + i%10) + } + + ci := colIterator{cols: data} + col := pilosa.Column{} + if rec, err := ci.NextRecord(); rec != col { + t.Fatalf("first record wrong: %v, err: %v", rec, err) + } + col.ColumnID = 1 + if rec, err := ci.NextRecord(); rec != col || err != nil { + t.Fatalf("second record wrong: %v, err: %v", rec, err) + } + col.ColumnID = 2 + if rec, err := ci.NextRecord(); rec != col || err != nil { + t.Fatalf("third record wrong: %v, err: %v", rec, err) + } + if rec, err := ci.NextRecord(); err != io.EOF { + t.Fatalf("should be EOF, but got %v, err: %v", rec, err) + } +} diff --git a/internal/clustertests/docker-compose.yml b/internal/clustertests/docker-compose.yml new file mode 100644 index 000000000..752c39177 --- /dev/null +++ b/internal/clustertests/docker-compose.yml @@ -0,0 +1,53 @@ +version: '2' +services: + pilosa1: + build: + context: ../.. + dockerfile: Dockerfile-withgo + image: ptest + ports: + - "33455:10101" + environment: + - PILOSA_CLUSTER_COORDINATOR=true + - PILOSA_GOSSIP_SEEDS=pilosa1:14000 + networks: + - pilosanet + command: + - "/pilosa server --bind pilosa1:10101" + pilosa2: + build: + context: ../.. + dockerfile: Dockerfile-withgo + image: ptest + ports: + - "33456:10101" + environment: + - PILOSA_GOSSIP_SEEDS=pilosa1:14000 + networks: + - pilosanet + command: + - "/pilosa server --bind pilosa2:10101" + pilosa3: + build: + context: . + image: ptest + ports: + - "33457:10101" + environment: + - PILOSA_GOSSIP_SEEDS=pilosa1:14000,pilosa2:14000 + networks: + - pilosanet + command: + - "/pilosa server --bind pilosa3:10101" + client1: + build: + context: ../.. + dockerfile: Dockerfile-withgo + networks: + - pilosanet + volumes: + - /var/run/docker.sock:/var/run/docker.sock + command: + - "go test -v github.com/pilosa/pilosa/internal/clustertests" +networks: + pilosanet: diff --git a/internal/udproxy/udproxy.go b/internal/udproxy/udproxy.go deleted file mode 100644 index a79c0121f..000000000 --- a/internal/udproxy/udproxy.go +++ /dev/null @@ -1,151 +0,0 @@ -package udproxy - -import ( - "bytes" - "io" - "net" - "sync" - "time" - - "github.com/pkg/errors" - "golang.org/x/sync/errgroup" -) - -type Proxy struct { - upstreamAddr *net.UDPAddr - conn *net.UDPConn - - drop bool - dropLock sync.Mutex - - quit chan struct{} - eg errgroup.Group - // map from client address to upstream connection. We must maintain a - // separate connection to upstream for each client connection so that we can - // differentiate data sent back from upstream. - upstreams map[*net.UDPAddr]*net.UDPConn - // TODO - need to track a per-connection timeout so that "upstreams" doesn't - // grow indefinitely. -} - -func New(listenIP string, listenPort int, upstreamIP string, upstreamPort int) (*Proxy, error) { - uc, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP(listenIP), Port: listenPort}) - if err != nil { - return nil, errors.Wrap(err, "listening") - } - p := &Proxy{ - conn: uc, - upstreamAddr: &net.UDPAddr{IP: net.ParseIP(upstreamIP), Port: upstreamPort}, - quit: make(chan struct{}), - upstreams: make(map[*net.UDPAddr]*net.UDPConn), - } - if p.upstreamAddr.IP == nil { - return nil, errors.Errorf("unable to parse upstream ip '%s'", upstreamIP) - } - p.eg.Go(p.run) - return p, nil -} - -func (p *Proxy) Drop() { - // sleep to attempt to ensure all traffic that was supposed to pass, did - // pass. - time.Sleep(time.Millisecond * 3) - p.dropLock.Lock() - p.drop = true - p.dropLock.Unlock() -} - -func (p *Proxy) Undrop() { - // this sleep is a cheap attempt to ensure everything that was supposed to - // be dropped was. - time.Sleep(time.Millisecond * 3) - p.dropLock.Lock() - p.drop = false - p.dropLock.Unlock() -} - -func (p *Proxy) dropping() bool { - p.dropLock.Lock() - d := p.drop - p.dropLock.Unlock() - return d -} - -func (p *Proxy) run() error { - buf := make([]byte, 65507) - for { - select { - case <-p.quit: - return nil - default: - } - err := p.conn.SetReadDeadline(time.Now().Add(time.Millisecond)) - if err != nil { - return errors.Wrap(err, "setting read deadline (run)") - } - n, addr, err := p.conn.ReadFromUDP(buf) - if err, ok := err.(net.Error); ok && err.Timeout() { - continue - } else if err != nil { - return errors.Wrap(err, "reading from udp conn") - } - upConn := p.upstreams[addr] - if upConn == nil { - p.upstreams[addr], err = net.DialUDP("udp", &net.UDPAddr{}, p.upstreamAddr) - if err != nil { - return errors.Wrap(err, "creating new connection to upstream") - } - p.eg.Go(func() error { - return p.proxyBack(addr, p.upstreams[addr]) - }) - upConn = p.upstreams[addr] - } - if !p.dropping() { - _, err = io.Copy(upConn, bytes.NewBuffer(buf[:n])) - if err != nil { - return errors.Wrap(err, "writing to upstream conn") - } - } - } -} - -func (p *Proxy) proxyBack(to *net.UDPAddr, from *net.UDPConn) error { - buf := make([]byte, 65507) - for { - select { - case <-p.quit: - return nil - default: - } - err := from.SetReadDeadline(time.Now().Add(time.Millisecond)) - if err != nil { - return errors.Wrap(err, "setting read deadline (proxyBack)") - } - n, _, err := from.ReadFromUDP(buf) - if err, ok := err.(net.Error); ok && err.Timeout() { - continue - } else if err != nil { - return errors.Wrap(err, "reading from upstream") - } - if !p.dropping() { - _, err = io.Copy(addrWriter{c: p.conn, a: to}, bytes.NewBuffer(buf[:n])) - if err != nil { - return errors.Wrap(err, "writing back to client") - } - } - } -} - -type addrWriter struct { - c *net.UDPConn - a *net.UDPAddr -} - -func (a addrWriter) Write(b []byte) (n int, err error) { - return a.c.WriteTo(b, a.a) -} - -func (p *Proxy) Close() error { - close(p.quit) - return p.eg.Wait() -} diff --git a/internal/udproxy/udproxy_test.go b/internal/udproxy/udproxy_test.go deleted file mode 100644 index be746ea72..000000000 --- a/internal/udproxy/udproxy_test.go +++ /dev/null @@ -1,83 +0,0 @@ -package udproxy_test - -import ( - "net" - "testing" - - "github.com/pilosa/pilosa/internal/udproxy" - "github.com/pkg/errors" - "golang.org/x/sync/errgroup" -) - -func TestUDProxy(t *testing.T) { - p, err := udproxy.New("127.0.0.1", 12345, "127.0.0.1", 12346) - if err != nil { - t.Fatalf("creating proxy: %v", err) - } - - uc, err := net.ListenUDP("udp", &net.UDPAddr{Port: 12346}) - if err != nil { - t.Fatalf("listening udp upstream: %v", err) - } - - resp := make([]byte, 8) - eg := errgroup.Group{} - eg.Go(func() error { - conn, err := net.DialUDP("udp", nil, &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 12345}) - if err != nil { - t.Fatalf("connecting to proxy: %v", err) - } - _, err = conn.Write([]byte("hello!")) - if err != nil { - return errors.Wrap(err, "writing to proxy") - } - _, err = conn.Read(resp) - if err != nil { - return errors.Wrap(err, "reading from proxy") - } - p.Drop() - _, err = conn.Write([]byte("hello2")) - if err != nil { - return errors.Wrap(err, "writing to dropping proxy") - } - p.Undrop() - _, err = conn.Write([]byte("hello3")) - if err != nil { - return errors.Wrap(err, "writing to undropping proxy") - } - return nil - }) - - req := make([]byte, 10) - _, addr, err := uc.ReadFrom(req) - if err != nil { - t.Fatalf("upstream reading from proxy: %v", err) - } - if string(req[:6]) != "hello!" { - t.Fatalf("got unexpected request %s", req) - } - _, err = uc.WriteTo([]byte("goodbye"), addr) - if err != nil { - t.Fatalf("writing response: %v", err) - } - - _, _, err = uc.ReadFrom(req) - if err != nil { - t.Fatalf("upstream reading from proxy: %v", err) - } - if string(req[:6]) != "hello3" { - t.Fatalf("got unexpected request %s", req) - } - - eg.Wait() - if string(resp[:7]) != "goodbye" { - t.Fatalf("got unexpected response '%v", resp) - } - - err = p.Close() - if err != nil { - t.Fatalf("err closing proxy: '%v'", err) - } -} - -// TODO test dropping diff --git a/server/server_test.go b/server/server_test.go index f30c02774..d62605ce0 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -379,11 +379,7 @@ func (p uint64Slice) Len() int { return len(p) } func (p uint64Slice) Less(i, j int) bool { return p[i] < p[j] } func TestClusteringNodesReplica1(t *testing.T) { - cluster := test.MustNewCluster(t, 3) - err := cluster.Start() - if err != nil { - t.Fatalf("starting cluster: %v", err) - } + cluster := test.MustRunCluster(t, 3) defer cluster.Close() var wait = true @@ -696,12 +692,3 @@ func TestClusterQueriesAfterRestart(t *testing.T) { } // TODO: confirm that things keep working if a node is hard-closed (no nodeLeave event) and immediately restarted with a different address. - -func TestClusterPartitioning(t *testing.T) { - cluster := test.MustNewClusterWithProxy(t, 3) - err := cluster.Start() - if err != nil { - t.Fatalf("starting cluster with proxy: %v", err) - } - -} diff --git a/test/pilosa.go b/test/pilosa.go index 3efecfe73..3bc43d593 100644 --- a/test/pilosa.go +++ b/test/pilosa.go @@ -19,100 +19,28 @@ import ( "context" "fmt" "io/ioutil" - "math/rand" gohttp "net/http" "os" "path" "strconv" "strings" - "sync/atomic" "testing" "time" "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/http" - "github.com/pilosa/pilosa/internal/udproxy" "github.com/pilosa/pilosa/server" "github.com/pkg/errors" - - "github.com/Shopify/toxiproxy" - tox "github.com/Shopify/toxiproxy/client" ) -type portAllocator struct { - port *uint32 -} - -var ports *portAllocator - -// Next generates a new port and returns it -func (n *portAllocator) Next() (nextPort uint32) { - return atomic.AddUint32(n.port, 1) -} - -var proxy string - -func init() { - r := rand.New(rand.NewSource(int64(time.Now().Nanosecond()))) - p := uint32(r.Uint32()%40000 + 20000) - ports = &portAllocator{ - port: &p, - } - - tport := strconv.Itoa(int(ports.Next())) - proxy = "localhost:" + tport - go toxiproxy.NewServer().Listen("localhost", tport) - -} - //////////////////////////////////////////////////////////////////////////////////// // Command represents a test wrapper for server.Command. type Command struct { *server.Command - proxies commandProxies - commandOptions []server.CommandOption } -// Drop uses the proxies to drop all network traffic to/from this node. -func (com Command) Drop(tb testing.TB) { - if com.proxies.http == nil { - tb.Fatal("can't drop traffic if cluster wasn't created with proxy support.") - } - err := com.proxies.http.Disable() - if err != nil { - tb.Fatalf("disabling http proxy: %v", err) - } - err = com.proxies.memberTCP.Disable() - if err != nil { - tb.Fatalf("disabling memberlist tcp proxy: %v", err) - } - com.proxies.memberUDP.Drop() -} - -// Undrop starts forwarding traffic to/from this node after a previous drop request. -func (com Command) Undrop(tb testing.TB) { - if com.proxies.http == nil { - tb.Fatal("can't drop traffic if cluster wasn't created with proxy support.") - } - err := com.proxies.http.Enable() - if err != nil { - tb.Fatalf("enabling http proxy: %v", err) - } - err = com.proxies.memberTCP.Enable() - if err != nil { - tb.Fatalf("enabling memberlist tcp proxy: %v", err) - } - com.proxies.memberUDP.Undrop() -} - -type commandProxies struct { - http *tox.Proxy - memberTCP *tox.Proxy - memberUDP *udproxy.Proxy -} - func OptAllowedOrigins(origins []string) server.CommandOption { return func(m *server.Command) error { m.Config.Handler.AllowedOrigins = origins @@ -346,6 +274,7 @@ func (c Cluster) CreateField(t testing.TB, index string, iopts pilosa.IndexOptio func (c Cluster) Start() error { var gossipSeeds = make([]string, len(c)) for i, cc := range c { + cc.Config.Gossip.Port = "0" cc.Config.Gossip.Seeds = gossipSeeds[:i] if err := cc.Start(); err != nil { return errors.Wrapf(err, "starting server %d", i) @@ -367,17 +296,7 @@ func (c Cluster) Close() error { // MustNewCluster creates a new cluster func MustNewCluster(tb testing.TB, size int, opts ...[]server.CommandOption) Cluster { - c, err := newCluster(size, false, opts...) - if err != nil { - tb.Fatalf("new cluster: %v", err) - } - return c -} - -// MustNewClusterWithProxy returns a test cluster which has all connections -// going through a proxy to allow for testing network partitions. -func MustNewClusterWithProxy(tb testing.TB, size int, opts ...[]server.CommandOption) Cluster { - c, err := newCluster(size, true, opts...) + c, err := newCluster(size, opts...) if err != nil { tb.Fatalf("new cluster: %v", err) } @@ -385,7 +304,7 @@ func MustNewClusterWithProxy(tb testing.TB, size int, opts ...[]server.CommandOp } // newCluster creates a new cluster -func newCluster(size int, withproxy bool, opts ...[]server.CommandOption) (cluster Cluster, err error) { +func newCluster(size int, opts ...[]server.CommandOption) (Cluster, error) { if size == 0 { return nil, errors.New("cluster must contain at least one node") } @@ -393,41 +312,14 @@ func newCluster(size int, withproxy bool, opts ...[]server.CommandOption) (clust return nil, errors.New("Slice of CommandOptions must be of length 0, 1, or equal to the number of cluster nodes") } - cluster = make(Cluster, size) + cluster := make(Cluster, size) for i := 0; i < size; i++ { var commandOpts []server.CommandOption if len(opts) > 0 { commandOpts = opts[i%len(opts)] } - name := "node" + strconv.Itoa(i) m := NewCommandNode(i == 0, commandOpts...) - m.Config.Bind = "localhost:" + strconv.Itoa(int(ports.Next())) - gossipBindPort := int(ports.Next()) - m.Config.Gossip.Port = strconv.Itoa(gossipBindPort) - - if withproxy { - tclient := tox.NewClient(proxy) - - aport := strconv.Itoa(int(ports.Next())) - m.Config.Advertise = "localhost:" + aport - m.proxies.http, err = tclient.CreateProxy(name+aport, m.Config.Advertise, m.Config.Bind) - if err != nil { - return nil, errors.Wrap(err, "setting up toxiproxy") - } - gossipAdvertPort := int(ports.Next()) - m.Config.Gossip.AdvertisePort = strconv.Itoa(gossipAdvertPort) - m.proxies.memberTCP, err = tclient.CreateProxy(name+"-gossip"+m.Config.Gossip.AdvertisePort, "localhost:"+m.Config.Gossip.AdvertisePort, "localhost:"+m.Config.Gossip.Port) - if err != nil { - return nil, errors.Wrap(err, "setting up toxiproxy for gossip") - } - m.proxies.memberUDP, err = udproxy.New("127.0.0.1", gossipAdvertPort, "127.0.0.1", gossipBindPort) - if err != nil { - return nil, errors.Wrap(err, "setting up proxy for udp gossip") - } - - } - - err = ioutil.WriteFile(path.Join(m.Config.DataDir, ".id"), []byte(name), 0600) + err := ioutil.WriteFile(path.Join(m.Config.DataDir, ".id"), []byte("node"+strconv.Itoa(i)), 0600) if err != nil { return nil, errors.Wrap(err, "writing node id") } @@ -438,8 +330,8 @@ func newCluster(size int, withproxy bool, opts ...[]server.CommandOption) (clust } // runCluster creates and starts a new cluster -func runCluster(size int, withproxy bool, opts ...[]server.CommandOption) (Cluster, error) { - cluster, err := newCluster(size, withproxy, opts...) +func runCluster(size int, opts ...[]server.CommandOption) (Cluster, error) { + cluster, err := newCluster(size, opts...) if err != nil { return nil, errors.Wrap(err, "new cluster") } @@ -451,7 +343,7 @@ func runCluster(size int, withproxy bool, opts ...[]server.CommandOption) (Clust // MustRunCluster creates and starts a new cluster func MustRunCluster(tb testing.TB, size int, opts ...[]server.CommandOption) Cluster { - c, err := runCluster(size, false, opts...) + c, err := runCluster(size, opts...) if err != nil { tb.Fatalf("run cluster: %v", err) } From 02aee931a1c8d80743cdefd57506ef05cd42f62d Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 2 Nov 2018 16:03:50 -0500 Subject: [PATCH 07/33] few small fixes --- Dockerfile-withgo | 4 ++++ internal/clustertests/cluster_test.go | 2 ++ 2 files changed, 6 insertions(+) diff --git a/Dockerfile-withgo b/Dockerfile-withgo index 5cb11e47b..306d16f3e 100644 --- a/Dockerfile-withgo +++ b/Dockerfile-withgo @@ -1,3 +1,6 @@ +# This Dockerfile is used for cluster testing - it produces a much larger image +# and includes all of Go as well as some utilities. + FROM golang:1.11 LABEL maintainer "dev@pilosa.com" @@ -7,6 +10,7 @@ COPY . /go/src/github.com/pilosa/pilosa/ RUN cd /go/src/github.com/pilosa/pilosa \ && CGO_ENABLED=0 make install-dep install FLAGS="-a" +# download pumba for fault injection RUN wget https://github.com/alexei-led/pumba/releases/download/0.6.0/pumba_linux_amd64 -O /pumba RUN chmod +x /pumba diff --git a/internal/clustertests/cluster_test.go b/internal/clustertests/cluster_test.go index e63b95d0b..10ffc638c 100644 --- a/internal/clustertests/cluster_test.go +++ b/internal/clustertests/cluster_test.go @@ -58,6 +58,7 @@ func TestLongPauses(t *testing.T) { if err != nil { t.Fatalf("waiting on pumba pause cmd: %v", err) } + // TODO change the sleep to wait for status to return to NORMAL or timeout once we have Status.State support in go-pilosa fmt.Println("done with pause, waiting for stability") time.Sleep(time.Second * 400) fmt.Println("done waiting") @@ -79,6 +80,7 @@ func getPilosaClient(t *testing.T) *pilosa.Client { time.Sleep(time.Millisecond * 40) } time.Sleep(time.Second * 2) + // TODO uncomment the following once we get the version of go-pilosa that has the State field on Status. // start := time.Now() // for i := 0; true; i++ { // s, err := cli.Status() From ec3982c0ff2ea1e0f20a8aba939ac2c631513c95 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 2 Nov 2018 16:10:39 -0500 Subject: [PATCH 08/33] pull out advertise URI changes --- ctl/server.go | 1 - gossip/gossip.go | 28 ++++------------------------ server.go | 16 ++++------------ server/config.go | 6 ------ server/server.go | 6 ------ server/server_test.go | 1 + uri.go | 17 ++++------------- 7 files changed, 13 insertions(+), 62 deletions(-) diff --git a/ctl/server.go b/ctl/server.go index 90766c498..7f384ec7b 100644 --- a/ctl/server.go +++ b/ctl/server.go @@ -26,7 +26,6 @@ func BuildServerFlags(cmd *cobra.Command, srv *server.Command) { flags := cmd.Flags() flags.StringVarP(&srv.Config.DataDir, "data-dir", "d", srv.Config.DataDir, "Directory to store pilosa data files.") flags.StringVarP(&srv.Config.Bind, "bind", "b", srv.Config.Bind, "Default URI on which pilosa should listen.") - flags.StringVarP(&srv.Config.Advertise, "advertise", "a", "", "Address to broadcast to other hosts and clients to be contacted on.") flags.IntVarP(&srv.Config.MaxWritesPerRequest, "max-writes-per-request", "", srv.Config.MaxWritesPerRequest, "Number of write commands per request.") flags.StringVar(&srv.Config.LogPath, "log-path", srv.Config.LogPath, "Log path") flags.BoolVar(&srv.Config.Verbose, "verbose", srv.Config.Verbose, "Enable verbose logging") diff --git a/gossip/gossip.go b/gossip/gossip.go index d03016792..789ed50b0 100644 --- a/gossip/gossip.go +++ b/gossip/gossip.go @@ -205,18 +205,8 @@ func NewMemberSet(cfg Config, api *pilosa.API, options ...memberSetOption) (*mem conf.Name = api.Node().ID conf.BindAddr = api.Node().URI.Host conf.BindPort = port - if cfg.AdvertisePort != "" { - port, err = strconv.Atoi(cfg.Port) - if err != nil { - return nil, fmt.Errorf("convert advertise port: %s", err) - } - } conf.AdvertisePort = port - if cfg.AdvertiseHost != "" { - conf.AdvertiseAddr = cfg.AdvertiseHost - } else { - conf.AdvertiseAddr = hostToIP(api.Node().URI.Host) - } + conf.AdvertiseAddr = hostToIP(api.Node().URI.Host) // conf.TCPTimeout = time.Duration(cfg.StreamTimeout) conf.SuspicionMult = cfg.SuspicionMult @@ -457,20 +447,10 @@ func newTransport(conf *memberlist.Config) (*memberlist.NetTransport, error) { // Config holds toml-friendly memberlist configuration. type Config struct { - // Host is the host gossip will bind to. If left blank it will be set to the - // host from Pilosa. - Host string `toml:"host"` // Port indicates the port to which pilosa should bind for internal state sharing. - Port string `toml:"port"` - // AdvertiseHost is the hostname or IP other nodes should use to connect to - // this host. If left blank, the value for Host will be used. This is useful - // in some proxy and NAT scenarios. - AdvertiseHost string `toml:"advertise-host` - // AdvertisePort is the port other nodes will use to connect to this one. - // Behaves like AdvertiseHost. - AdvertisePort string `toml:"advertise-port"` - Seeds []string `toml:"seeds"` - Key string `toml:"key"` + Port string `toml:"port"` + Seeds []string `toml:"seeds"` + Key string `toml:"key"` // StreamTimeout is the timeout for establishing a stream connection with // a remote node for a full state sync, and for stream read and write // operations. Maps to memberlist TCPTimeout. diff --git a/server.go b/server.go index 36190e315..a4cb8fc8b 100644 --- a/server.go +++ b/server.go @@ -62,7 +62,6 @@ type Server struct { // nolint: maligned nodeID string uri URI - advertiseURI URI antiEntropyInterval time.Duration metricInterval time.Duration diagnosticInterval time.Duration @@ -74,7 +73,7 @@ type Server struct { // nolint: maligned dataDir string } -// TODO (2.0): have this return an interface for Holder instead of concrete object? +// TODO: have this return an interface for Holder instead of concrete object? func (s *Server) Holder() *Holder { return s.holder } @@ -161,13 +160,6 @@ func OptServerInternalClient(c InternalClient) ServerOption { } } -func OptServerAdvertiseURI(u *URI) ServerOption { - return func(s *Server) error { - s.advertiseURI = *u - return nil - } -} - // DEPRECATED func OptServerPrimaryTranslateStore(store TranslateStore) ServerOption { return func(s *Server) error { @@ -304,7 +296,7 @@ func NewServer(opts ...ServerOption) (*Server, error) { // Set Cluster Node. node := &Node{ ID: s.nodeID, - URI: s.advertiseURI, + URI: s.uri, IsCoordinator: s.cluster.Coordinator == s.nodeID, } s.cluster.Node = node @@ -589,7 +581,7 @@ func (s *Server) SendSync(m Message) error { node := node s.logger.Printf("SendSync to: %s", node.URI) // Don't forward the message to ourselves. - if s.advertiseURI == node.URI { + if s.uri == node.URI { continue } @@ -684,7 +676,7 @@ func (s *Server) monitorDiagnostics() { s.diagnostics.Logger = s.logger s.diagnostics.SetVersion(Version) - s.diagnostics.Set("Host", s.advertiseURI.Host) + s.diagnostics.Set("Host", s.uri.Host) s.diagnostics.Set("Cluster", strings.Join(s.cluster.nodeIDs(), ",")) s.diagnostics.Set("NumNodes", len(s.cluster.nodes)) s.diagnostics.Set("NumCPU", runtime.NumCPU()) diff --git a/server/config.go b/server/config.go index 210a2e298..d255e4bb6 100644 --- a/server/config.go +++ b/server/config.go @@ -36,15 +36,9 @@ type Config struct { // DataDir is the directory where Pilosa stores both indexed data and // running state such as cluster topology information. DataDir string `toml:"data-dir"` - // Bind is the host:port on which Pilosa will listen. Bind string `toml:"bind"` - // Advertise is the host:port that this node will report as its address to - // others. If left blank (the default), this will be set to the bind address - // once it is listening. - Advertise string `toml:"advertise"` - // MaxWritesPerRequest limits the number of mutating commands that can be in // a single request to the server. This includes Set, Clear, // SetRowAttrs & SetColumnAttrs. diff --git a/server/server.go b/server/server.go index 8521abdec..010eb2f8a 100644 --- a/server/server.go +++ b/server/server.go @@ -248,11 +248,6 @@ func (m *Command) SetupServer() error { uri.SetPort(uint16(m.ln.Addr().(*net.TCPAddr).Port)) } - advertURI, err := pilosa.NewURIFromAddressWithDefault(m.Config.Advertise, uri) - if err != nil { - return errors.Wrapf(err, "processing avertise address '%s'", m.Config.Advertise) - } - c := http.GetHTTPClient(TLSConfig) // Primary store configuration is handled automatically now. @@ -281,7 +276,6 @@ func (m *Command) SetupServer() error { pilosa.OptServerGCNotifier(gcnotify.NewActiveGCNotifier()), pilosa.OptServerStatsClient(statsClient), pilosa.OptServerURI(uri), - pilosa.OptServerAdvertiseURI(advertURI), pilosa.OptServerInternalClient(http.NewInternalClientFromURI(uri, c)), pilosa.OptServerPrimaryTranslateStoreFunc(http.NewTranslateStore), pilosa.OptServerClusterDisabled(m.Config.Cluster.Disabled, m.Config.Cluster.Hosts), diff --git a/server/server_test.go b/server/server_test.go index d62605ce0..aa202a54c 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -405,6 +405,7 @@ func TestClusteringNodesReplica1(t *testing.T) { // Create new main with the same config. config := cluster[2].Command.Config config.Translation.MapSize = 100000 + // config.Bind = cluster[2].API.Node().URI.HostPort() // this isn't necessary, but makes the test run way faster config.Gossip.Port = strconv.Itoa(int(cluster[2].Command.GossipTransport().URI.Port)) diff --git a/uri.go b/uri.go index 7cf985f0b..231457691 100644 --- a/uri.go +++ b/uri.go @@ -82,10 +82,6 @@ func NewURIFromAddress(address string) (*URI, error) { return parseAddress(address) } -func NewURIFromAddressWithDefault(address string, base *URI) (*URI, error) { - return parseAddressWithDefault(address, base) -} - // setScheme sets the scheme of this URI. func (u *URI) setScheme(scheme string) error { m := schemeRegexp.FindStringSubmatch(scheme) @@ -158,20 +154,20 @@ func (u URI) Type() string { return "URI" } -func parseAddressWithDefault(address string, def *URI) (uri *URI, err error) { +func parseAddress(address string) (uri *URI, err error) { m := addressRegexp.FindStringSubmatch(address) if m == nil { return nil, errors.New("invalid address") } - scheme := def.Scheme + scheme := "http" if m[2] != "" { scheme = m[2] } - host := def.Host + host := "localhost" if m[3] != "" { host = m[3] } - var port = int(def.Port) + var port = 10101 if m[5] != "" { port, err = strconv.Atoi(m[5]) if err != nil { @@ -189,11 +185,6 @@ func parseAddressWithDefault(address string, def *URI) (uri *URI, err error) { return uri, nil } -func parseAddress(address string) (uri *URI, err error) { - u, err := parseAddressWithDefault(address, defaultURI()) - return u, err -} - // MarshalJSON marshals URI into a JSON-encoded byte slice. func (u *URI) MarshalJSON() ([]byte, error) { var output struct { From 6d821afed93d24b03e859759b6fc1d3c5e2159df Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 2 Nov 2018 16:13:03 -0500 Subject: [PATCH 09/33] add note on skipped cluster test --- internal/clustertests/cluster_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/clustertests/cluster_test.go b/internal/clustertests/cluster_test.go index 10ffc638c..23d27fcba 100644 --- a/internal/clustertests/cluster_test.go +++ b/internal/clustertests/cluster_test.go @@ -13,7 +13,7 @@ import ( ) func TestLongPauses(t *testing.T) { - t.Skip() + t.Skip() // TODO figure out how to only run in the docker-compose environment cli := getPilosaClient(t) idx := pilosa.NewIndex("testidx") From 44e436f5718a13080f6011dfa57a0eafce732d2f Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Fri, 9 Nov 2018 08:08:43 +0300 Subject: [PATCH 10/33] removed unused rule from peg grammar --- pql/pql.peg | 1 - pql/pql.peg.go | 418 ++++++++++++++++++++++++------------------------- 2 files changed, 204 insertions(+), 215 deletions(-) diff --git a/pql/pql.peg b/pql/pql.peg index f136ad764..15d33ac65 100644 --- a/pql/pql.peg +++ b/pql/pql.peg @@ -57,7 +57,6 @@ field <- { p.addField(buffer[begin:end]) } reserved <- ('_row' / '_col' / '_start' / '_end' / '_timestamp' / '_field') posfield <- { p.addPosStr("_field", buffer[begin:end]) } uint <- [1-9] [0-9]* / '0' -uintrow <- {p.addPosNum("_row", buffer[begin:end])} col <- ( {p.addPosNum("_col", buffer[begin:end])} / '\'' '\'' {p.addPosStr("_col", buffer[begin:end])} / '"' '"' {p.addPosStr("_col", buffer[begin:end])} diff --git a/pql/pql.peg.go b/pql/pql.peg.go index 5577142bf..5c3fb9ee9 100644 --- a/pql/pql.peg.go +++ b/pql/pql.peg.go @@ -37,7 +37,6 @@ const ( rulereserved ruleposfield ruleuint - ruleuintrow rulecol rulerow ruleopen @@ -102,7 +101,6 @@ const ( ruleAction48 ruleAction49 ruleAction50 - ruleAction51 ) var rul3s = [...]string{ @@ -128,7 +126,6 @@ var rul3s = [...]string{ "reserved", "posfield", "uint", - "uintrow", "col", "row", "open", @@ -193,7 +190,6 @@ var rul3s = [...]string{ "Action48", "Action49", "Action50", - "Action51", } type token32 struct { @@ -310,7 +306,7 @@ type PQL struct { Buffer string buffer []rune - rules [88]func() bool + rules [86]func() bool parse func(rule ...int) error reset func() Pretty bool @@ -492,20 +488,18 @@ func (p *PQL) Execute() { case ruleAction43: p.addPosStr("_field", buffer[begin:end]) case ruleAction44: - p.addPosNum("_row", buffer[begin:end]) - case ruleAction45: p.addPosNum("_col", buffer[begin:end]) + case ruleAction45: + p.addPosStr("_col", buffer[begin:end]) case ruleAction46: p.addPosStr("_col", buffer[begin:end]) case ruleAction47: - p.addPosStr("_col", buffer[begin:end]) - case ruleAction48: p.addPosNum("_row", buffer[begin:end]) + case ruleAction48: + p.addPosStr("_row", buffer[begin:end]) case ruleAction49: p.addPosStr("_row", buffer[begin:end]) case ruleAction50: - p.addPosStr("_row", buffer[begin:end]) - case ruleAction51: p.addPosStr("_timestamp", buffer[begin:end]) } @@ -667,7 +661,7 @@ func (p *PQL) Init() { add(rulePegText, position13) } { - add(ruleAction51, position) + add(ruleAction50, position) } add(ruletimestamp, position12) } @@ -753,7 +747,7 @@ func (p *PQL) Init() { add(rulePegText, position21) } { - add(ruleAction48, position) + add(ruleAction47, position) } goto l19 l20: @@ -774,7 +768,7 @@ func (p *PQL) Init() { } position++ { - add(ruleAction49, position) + add(ruleAction48, position) } goto l19 l23: @@ -795,7 +789,7 @@ func (p *PQL) Init() { } position++ { - add(ruleAction50, position) + add(ruleAction49, position) } } l19: @@ -2536,421 +2530,417 @@ func (p *PQL) Init() { position, tokenIndex = position243, tokenIndex243 return false }, - /* 21 uintrow <- <( Action44)> */ - nil, - /* 22 col <- <(( Action45) / ('\'' '\'' Action46) / ('"' '"' Action47))> */ + /* 21 col <- <(( Action44) / ('\'' '\'' Action45) / ('"' '"' Action46))> */ func() bool { - position250, tokenIndex250 := position, tokenIndex + position249, tokenIndex249 := position, tokenIndex { - position251 := position + position250 := position { - position252, tokenIndex252 := position, tokenIndex + position251, tokenIndex251 := position, tokenIndex { - position254 := position + position253 := position if !_rules[ruleuint]() { - goto l253 + goto l252 } - add(rulePegText, position254) + add(rulePegText, position253) } { - add(ruleAction45, position) + add(ruleAction44, position) } - goto l252 - l253: - position, tokenIndex = position252, tokenIndex252 + goto l251 + l252: + position, tokenIndex = position251, tokenIndex251 if buffer[position] != rune('\'') { - goto l256 + goto l255 } position++ { - position257 := position + position256 := position if !_rules[rulesinglequotedstring]() { - goto l256 + goto l255 } - add(rulePegText, position257) + add(rulePegText, position256) } if buffer[position] != rune('\'') { - goto l256 + goto l255 + } + position++ + { + add(ruleAction45, position) + } + goto l251 + l255: + position, tokenIndex = position251, tokenIndex251 + if buffer[position] != rune('"') { + goto l249 + } + position++ + { + position258 := position + if !_rules[ruledoublequotedstring]() { + goto l249 + } + add(rulePegText, position258) + } + if buffer[position] != rune('"') { + goto l249 } position++ { add(ruleAction46, position) } - goto l252 - l256: - position, tokenIndex = position252, tokenIndex252 - if buffer[position] != rune('"') { - goto l250 - } - position++ - { - position259 := position - if !_rules[ruledoublequotedstring]() { - goto l250 - } - add(rulePegText, position259) - } - if buffer[position] != rune('"') { - goto l250 - } - position++ - { - add(ruleAction47, position) - } } - l252: - add(rulecol, position251) + l251: + add(rulecol, position250) } return true - l250: - position, tokenIndex = position250, tokenIndex250 + l249: + position, tokenIndex = position249, tokenIndex249 return false }, - /* 23 row <- <(( Action48) / ('\'' '\'' Action49) / ('"' '"' Action50))> */ + /* 22 row <- <(( Action47) / ('\'' '\'' Action48) / ('"' '"' Action49))> */ nil, - /* 24 open <- <('(' sp)> */ + /* 23 open <- <('(' sp)> */ func() bool { - position262, tokenIndex262 := position, tokenIndex + position261, tokenIndex261 := position, tokenIndex { - position263 := position + position262 := position if buffer[position] != rune('(') { - goto l262 + goto l261 } position++ if !_rules[rulesp]() { - goto l262 + goto l261 } - add(ruleopen, position263) + add(ruleopen, position262) } return true - l262: - position, tokenIndex = position262, tokenIndex262 + l261: + position, tokenIndex = position261, tokenIndex261 return false }, - /* 25 close <- <(')' sp)> */ + /* 24 close <- <(')' sp)> */ func() bool { - position264, tokenIndex264 := position, tokenIndex + position263, tokenIndex263 := position, tokenIndex { - position265 := position + position264 := position if buffer[position] != rune(')') { - goto l264 + goto l263 } position++ if !_rules[rulesp]() { - goto l264 + goto l263 } - add(ruleclose, position265) + add(ruleclose, position264) } return true - l264: - position, tokenIndex = position264, tokenIndex264 + l263: + position, tokenIndex = position263, tokenIndex263 return false }, - /* 26 sp <- <(' ' / '\t' / '\n')*> */ + /* 25 sp <- <(' ' / '\t' / '\n')*> */ func() bool { { - position267 := position - l268: + position266 := position + l267: { - position269, tokenIndex269 := position, tokenIndex + position268, tokenIndex268 := position, tokenIndex { - position270, tokenIndex270 := position, tokenIndex + position269, tokenIndex269 := position, tokenIndex if buffer[position] != rune(' ') { + goto l270 + } + position++ + goto l269 + l270: + position, tokenIndex = position269, tokenIndex269 + if buffer[position] != rune('\t') { goto l271 } position++ - goto l270 + goto l269 l271: - position, tokenIndex = position270, tokenIndex270 - if buffer[position] != rune('\t') { - goto l272 - } - position++ - goto l270 - l272: - position, tokenIndex = position270, tokenIndex270 + position, tokenIndex = position269, tokenIndex269 if buffer[position] != rune('\n') { - goto l269 + goto l268 } position++ } - l270: - goto l268 l269: - position, tokenIndex = position269, tokenIndex269 + goto l267 + l268: + position, tokenIndex = position268, tokenIndex268 } - add(rulesp, position267) + add(rulesp, position266) } return true }, - /* 27 comma <- <(sp ',' sp)> */ + /* 26 comma <- <(sp ',' sp)> */ func() bool { - position273, tokenIndex273 := position, tokenIndex + position272, tokenIndex272 := position, tokenIndex { - position274 := position + position273 := position if !_rules[rulesp]() { - goto l273 + goto l272 } if buffer[position] != rune(',') { - goto l273 + goto l272 } position++ if !_rules[rulesp]() { - goto l273 + goto l272 } - add(rulecomma, position274) + add(rulecomma, position273) } return true - l273: - position, tokenIndex = position273, tokenIndex273 + l272: + position, tokenIndex = position272, tokenIndex272 return false }, - /* 28 lbrack <- <('[' sp)> */ + /* 27 lbrack <- <('[' sp)> */ nil, - /* 29 rbrack <- <(sp ']' sp)> */ + /* 28 rbrack <- <(sp ']' sp)> */ nil, - /* 30 IDENT <- <(([a-z] / [A-Z]) ([a-z] / [A-Z] / [0-9])*)> */ + /* 29 IDENT <- <(([a-z] / [A-Z]) ([a-z] / [A-Z] / [0-9])*)> */ nil, - /* 31 timestampbasicfmt <- <([0-9] [0-9] [0-9] [0-9] '-' ('0' / '1') [0-9] '-' [0-3] [0-9] 'T' [0-9] [0-9] ':' [0-9] [0-9])> */ + /* 30 timestampbasicfmt <- <([0-9] [0-9] [0-9] [0-9] '-' ('0' / '1') [0-9] '-' [0-3] [0-9] 'T' [0-9] [0-9] ':' [0-9] [0-9])> */ func() bool { - position278, tokenIndex278 := position, tokenIndex + position277, tokenIndex277 := position, tokenIndex { - position279 := position + position278 := position if c := buffer[position]; c < rune('0') || c > rune('9') { - goto l278 + goto l277 } position++ if c := buffer[position]; c < rune('0') || c > rune('9') { - goto l278 + goto l277 } position++ if c := buffer[position]; c < rune('0') || c > rune('9') { - goto l278 + goto l277 } position++ if c := buffer[position]; c < rune('0') || c > rune('9') { - goto l278 + goto l277 } position++ if buffer[position] != rune('-') { - goto l278 + goto l277 } position++ { - position280, tokenIndex280 := position, tokenIndex + position279, tokenIndex279 := position, tokenIndex if buffer[position] != rune('0') { - goto l281 + goto l280 } position++ - goto l280 - l281: - position, tokenIndex = position280, tokenIndex280 + goto l279 + l280: + position, tokenIndex = position279, tokenIndex279 if buffer[position] != rune('1') { - goto l278 + goto l277 } position++ } - l280: + l279: if c := buffer[position]; c < rune('0') || c > rune('9') { - goto l278 + goto l277 } position++ if buffer[position] != rune('-') { - goto l278 + goto l277 } position++ if c := buffer[position]; c < rune('0') || c > rune('3') { - goto l278 + goto l277 } position++ if c := buffer[position]; c < rune('0') || c > rune('9') { - goto l278 + goto l277 } position++ if buffer[position] != rune('T') { - goto l278 + goto l277 } position++ if c := buffer[position]; c < rune('0') || c > rune('9') { - goto l278 + goto l277 } position++ if c := buffer[position]; c < rune('0') || c > rune('9') { - goto l278 + goto l277 } position++ if buffer[position] != rune(':') { - goto l278 + goto l277 } position++ if c := buffer[position]; c < rune('0') || c > rune('9') { - goto l278 + goto l277 } position++ if c := buffer[position]; c < rune('0') || c > rune('9') { - goto l278 + goto l277 } position++ - add(ruletimestampbasicfmt, position279) + add(ruletimestampbasicfmt, position278) } return true - l278: - position, tokenIndex = position278, tokenIndex278 + l277: + position, tokenIndex = position277, tokenIndex277 return false }, - /* 32 timestampfmt <- <(('"' timestampbasicfmt '"') / ('\'' timestampbasicfmt '\'') / timestampbasicfmt)> */ + /* 31 timestampfmt <- <(('"' timestampbasicfmt '"') / ('\'' timestampbasicfmt '\'') / timestampbasicfmt)> */ func() bool { - position282, tokenIndex282 := position, tokenIndex + position281, tokenIndex281 := position, tokenIndex { - position283 := position + position282 := position { - position284, tokenIndex284 := position, tokenIndex + position283, tokenIndex283 := position, tokenIndex if buffer[position] != rune('"') { + goto l284 + } + position++ + if !_rules[ruletimestampbasicfmt]() { + goto l284 + } + if buffer[position] != rune('"') { + goto l284 + } + position++ + goto l283 + l284: + position, tokenIndex = position283, tokenIndex283 + if buffer[position] != rune('\'') { goto l285 } position++ if !_rules[ruletimestampbasicfmt]() { goto l285 } - if buffer[position] != rune('"') { + if buffer[position] != rune('\'') { goto l285 } position++ - goto l284 + goto l283 l285: - position, tokenIndex = position284, tokenIndex284 - if buffer[position] != rune('\'') { - goto l286 - } - position++ + position, tokenIndex = position283, tokenIndex283 if !_rules[ruletimestampbasicfmt]() { - goto l286 - } - if buffer[position] != rune('\'') { - goto l286 - } - position++ - goto l284 - l286: - position, tokenIndex = position284, tokenIndex284 - if !_rules[ruletimestampbasicfmt]() { - goto l282 + goto l281 } } - l284: - add(ruletimestampfmt, position283) + l283: + add(ruletimestampfmt, position282) } return true - l282: - position, tokenIndex = position282, tokenIndex282 + l281: + position, tokenIndex = position281, tokenIndex281 return false }, - /* 33 timestamp <- <( Action51)> */ + /* 32 timestamp <- <( Action50)> */ nil, - /* 35 Action0 <- <{p.startCall("Set")}> */ + /* 34 Action0 <- <{p.startCall("Set")}> */ nil, - /* 36 Action1 <- <{p.endCall()}> */ + /* 35 Action1 <- <{p.endCall()}> */ nil, - /* 37 Action2 <- <{p.startCall("SetRowAttrs")}> */ + /* 36 Action2 <- <{p.startCall("SetRowAttrs")}> */ nil, - /* 38 Action3 <- <{p.endCall()}> */ + /* 37 Action3 <- <{p.endCall()}> */ nil, - /* 39 Action4 <- <{p.startCall("SetColumnAttrs")}> */ + /* 38 Action4 <- <{p.startCall("SetColumnAttrs")}> */ nil, - /* 40 Action5 <- <{p.endCall()}> */ + /* 39 Action5 <- <{p.endCall()}> */ nil, - /* 41 Action6 <- <{p.startCall("Clear")}> */ + /* 40 Action6 <- <{p.startCall("Clear")}> */ nil, - /* 42 Action7 <- <{p.endCall()}> */ + /* 41 Action7 <- <{p.endCall()}> */ nil, - /* 43 Action8 <- <{p.startCall("ClearRow")}> */ + /* 42 Action8 <- <{p.startCall("ClearRow")}> */ nil, - /* 44 Action9 <- <{p.endCall()}> */ + /* 43 Action9 <- <{p.endCall()}> */ nil, - /* 45 Action10 <- <{p.startCall("Store")}> */ + /* 44 Action10 <- <{p.startCall("Store")}> */ nil, - /* 46 Action11 <- <{p.endCall()}> */ + /* 45 Action11 <- <{p.endCall()}> */ nil, - /* 47 Action12 <- <{p.startCall("TopN")}> */ + /* 46 Action12 <- <{p.startCall("TopN")}> */ nil, - /* 48 Action13 <- <{p.endCall()}> */ + /* 47 Action13 <- <{p.endCall()}> */ nil, - /* 49 Action14 <- <{p.startCall("Range")}> */ + /* 48 Action14 <- <{p.startCall("Range")}> */ nil, - /* 50 Action15 <- <{p.endCall()}> */ + /* 49 Action15 <- <{p.endCall()}> */ nil, nil, - /* 52 Action16 <- <{ p.startCall(buffer[begin:end] ) }> */ + /* 51 Action16 <- <{ p.startCall(buffer[begin:end] ) }> */ nil, - /* 53 Action17 <- <{ p.endCall() }> */ + /* 52 Action17 <- <{ p.endCall() }> */ nil, - /* 54 Action18 <- <{ p.addBTWN() }> */ + /* 53 Action18 <- <{ p.addBTWN() }> */ nil, - /* 55 Action19 <- <{ p.addLTE() }> */ + /* 54 Action19 <- <{ p.addLTE() }> */ nil, - /* 56 Action20 <- <{ p.addGTE() }> */ + /* 55 Action20 <- <{ p.addGTE() }> */ nil, - /* 57 Action21 <- <{ p.addEQ() }> */ + /* 56 Action21 <- <{ p.addEQ() }> */ nil, - /* 58 Action22 <- <{ p.addNEQ() }> */ + /* 57 Action22 <- <{ p.addNEQ() }> */ nil, - /* 59 Action23 <- <{ p.addLT() }> */ + /* 58 Action23 <- <{ p.addLT() }> */ nil, - /* 60 Action24 <- <{ p.addGT() }> */ + /* 59 Action24 <- <{ p.addGT() }> */ nil, - /* 61 Action25 <- <{p.startConditional()}> */ + /* 60 Action25 <- <{p.startConditional()}> */ nil, - /* 62 Action26 <- <{p.endConditional()}> */ + /* 61 Action26 <- <{p.endConditional()}> */ nil, - /* 63 Action27 <- <{p.condAdd(buffer[begin:end])}> */ + /* 62 Action27 <- <{p.condAdd(buffer[begin:end])}> */ nil, - /* 64 Action28 <- <{p.condAdd(buffer[begin:end])}> */ + /* 63 Action28 <- <{p.condAdd(buffer[begin:end])}> */ nil, - /* 65 Action29 <- <{p.condAdd(buffer[begin:end])}> */ + /* 64 Action29 <- <{p.condAdd(buffer[begin:end])}> */ nil, - /* 66 Action30 <- <{p.addPosStr("_start", buffer[begin:end])}> */ + /* 65 Action30 <- <{p.addPosStr("_start", buffer[begin:end])}> */ nil, - /* 67 Action31 <- <{p.addPosStr("_end", buffer[begin:end])}> */ + /* 66 Action31 <- <{p.addPosStr("_end", buffer[begin:end])}> */ nil, - /* 68 Action32 <- <{ p.startList() }> */ + /* 67 Action32 <- <{ p.startList() }> */ nil, - /* 69 Action33 <- <{ p.endList() }> */ + /* 68 Action33 <- <{ p.endList() }> */ nil, - /* 70 Action34 <- <{ p.addVal(nil) }> */ + /* 69 Action34 <- <{ p.addVal(nil) }> */ nil, - /* 71 Action35 <- <{ p.addVal(true) }> */ + /* 70 Action35 <- <{ p.addVal(true) }> */ nil, - /* 72 Action36 <- <{ p.addVal(false) }> */ + /* 71 Action36 <- <{ p.addVal(false) }> */ nil, - /* 73 Action37 <- <{ p.addNumVal(buffer[begin:end]) }> */ + /* 72 Action37 <- <{ p.addNumVal(buffer[begin:end]) }> */ nil, - /* 74 Action38 <- <{ p.addNumVal(buffer[begin:end]) }> */ + /* 73 Action38 <- <{ p.addNumVal(buffer[begin:end]) }> */ nil, - /* 75 Action39 <- <{ p.addVal(buffer[begin:end]) }> */ + /* 74 Action39 <- <{ p.addVal(buffer[begin:end]) }> */ nil, - /* 76 Action40 <- <{ s, _ := strconv.Unquote(buffer[begin:end]); p.addVal(s) }> */ + /* 75 Action40 <- <{ s, _ := strconv.Unquote(buffer[begin:end]); p.addVal(s) }> */ nil, - /* 77 Action41 <- <{ p.addVal(buffer[begin:end]) }> */ + /* 76 Action41 <- <{ p.addVal(buffer[begin:end]) }> */ nil, - /* 78 Action42 <- <{ p.addField(buffer[begin:end]) }> */ + /* 77 Action42 <- <{ p.addField(buffer[begin:end]) }> */ nil, - /* 79 Action43 <- <{ p.addPosStr("_field", buffer[begin:end]) }> */ + /* 78 Action43 <- <{ p.addPosStr("_field", buffer[begin:end]) }> */ nil, - /* 80 Action44 <- <{p.addPosNum("_row", buffer[begin:end])}> */ + /* 79 Action44 <- <{p.addPosNum("_col", buffer[begin:end])}> */ nil, - /* 81 Action45 <- <{p.addPosNum("_col", buffer[begin:end])}> */ + /* 80 Action45 <- <{p.addPosStr("_col", buffer[begin:end])}> */ nil, - /* 82 Action46 <- <{p.addPosStr("_col", buffer[begin:end])}> */ + /* 81 Action46 <- <{p.addPosStr("_col", buffer[begin:end])}> */ nil, - /* 83 Action47 <- <{p.addPosStr("_col", buffer[begin:end])}> */ + /* 82 Action47 <- <{p.addPosNum("_row", buffer[begin:end])}> */ nil, - /* 84 Action48 <- <{p.addPosNum("_row", buffer[begin:end])}> */ + /* 83 Action48 <- <{p.addPosStr("_row", buffer[begin:end])}> */ nil, - /* 85 Action49 <- <{p.addPosStr("_row", buffer[begin:end])}> */ + /* 84 Action49 <- <{p.addPosStr("_row", buffer[begin:end])}> */ nil, - /* 86 Action50 <- <{p.addPosStr("_row", buffer[begin:end])}> */ - nil, - /* 87 Action51 <- <{p.addPosStr("_timestamp", buffer[begin:end])}> */ + /* 85 Action50 <- <{p.addPosStr("_timestamp", buffer[begin:end])}> */ nil, } p.rules = _rules From deae8ce7c03336615a8df800b4dc97d3ad2d779e Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 9 Nov 2018 11:28:19 -0600 Subject: [PATCH 11/33] improvements to clustertests and fix cluster pause bug by state sharing --- Dockerfile-withgo | 2 +- Makefile | 16 ++ cluster.go | 51 ++++-- cluster_internal_test.go | 16 +- encoding/proto/proto.go | 2 + internal/clustertests/cluster_test.go | 112 +++++++------ internal/clustertests/docker-compose.yml | 7 +- internal/private.pb.go | 190 ++++++++++++++--------- internal/private.proto | 1 + server.go | 6 +- server/server.go | 2 +- 11 files changed, 260 insertions(+), 145 deletions(-) diff --git a/Dockerfile-withgo b/Dockerfile-withgo index 306d16f3e..834634b13 100644 --- a/Dockerfile-withgo +++ b/Dockerfile-withgo @@ -23,4 +23,4 @@ EXPOSE 10101 VOLUME /data ENTRYPOINT ["bash", "-c"] -CMD ["pilosa", "server", "--data-dir", "/data", "--bind", "http://0.0.0.0:10101"] +CMD ["/pilosa", "server", "--data-dir", "/data", "--bind", "http://0.0.0.0:10101"] diff --git a/Makefile b/Makefile index b403fde23..9cf4cffaf 100644 --- a/Makefile +++ b/Makefile @@ -68,6 +68,22 @@ release: check-clean $(MAKE) release-build GOOS=linux GOARCH=386 $(MAKE) release-build GOOS=linux GOARCH=386 ENTERPRISE=1 +# Run cluster integration tests using docker. Requires docker daemon to be +# running. This will catch changes to internal/clustertests/*.go, but if you +# make changes to Pilosa, you'll want to run clustertests-build to rebuild the +# pilosa image. +clustertests: + cd internal/clustertests;\ + docker-compose down;\ + docker-compose up; + + +# Like clustertests, but rebuilds all images. +clustertests-build: + cd internal/clustertests;\ + docker-compose down;\ + docker-compose up --build; + # Create prerelease builds prerelease: vendor $(MAKE) release-build GOOS=linux GOARCH=amd64 VERSION_ID=$$\(BRANCH_ID\) diff --git a/cluster.go b/cluster.go index c3bc67b4d..4b1ab9089 100644 --- a/cluster.go +++ b/cluster.go @@ -65,10 +65,11 @@ type Node struct { ID string `json:"id"` URI URI `json:"uri"` IsCoordinator bool `json:"isCoordinator"` + State string `json:"state"` } func (n Node) String() string { - return fmt.Sprintf("Node: %s", n.ID) + return fmt.Sprintf("Node:%s:%s:%s", n.URI, n.State, n.ID[:6]) } // Nodes represents a list of nodes. @@ -456,7 +457,19 @@ func (c *cluster) unprotectedSetState(state string) { } } +func (c *cluster) setMyNodeState(state string) { + c.mu.Lock() + defer c.mu.Unlock() + c.Node.State = state + for i, n := range c.nodes { + if n.ID == c.Node.ID { + c.nodes[i].State = state + } + } +} + func (c *cluster) setNodeState(state string) error { // nolint: unparam + c.setMyNodeState(state) if c.isCoordinator() { return c.receiveNodeState(c.Node.ID, state) } @@ -486,11 +499,23 @@ func (c *cluster) receiveNodeState(nodeID string, state string) error { } c.Topology.mu.Lock() - c.Topology.nodeStates[nodeID] = state + changed := false + if c.Topology.nodeStates[nodeID] != state { + changed = true + c.Topology.nodeStates[nodeID] = state + for i, n := range c.nodes { + if n.ID == nodeID { + c.nodes[i].State = state + } + } + } c.Topology.mu.Unlock() c.logger.Printf("received state %s (%s)", state, nodeID) - return c.unprotectedSetStateAndBroadcast(c.determineClusterState()) + if changed { + return c.unprotectedSetStateAndBroadcast(c.determineClusterState()) + } + return nil } // determineClusterState is unprotected. @@ -932,7 +957,6 @@ func (c *cluster) waitForStarted() error { <-c.joining c.logger.Printf("joining has completed") } - return nil } @@ -1043,8 +1067,9 @@ func (c *cluster) unprotectedSetStateAndBroadcast(state string) error { return nil } // Broadcast cluster status changes to the cluster. - c.logger.Printf("broadcasting ClusterStatus: %s", state) - return c.broadcaster.SendSync(c.unprotectedStatus()) // TODO fix c.Status + status := c.unprotectedStatus() + c.logger.Printf("broadcasting ClusterStatus: %s", status) + return c.broadcaster.SendSync(status) // TODO fix c.Status } @@ -1625,7 +1650,7 @@ func (c *cluster) ReceiveEvent(e *NodeEvent) (err error) { switch e.Event { case NodeJoin: - c.logger.Printf("received NodeJoin event: %v", e) + c.logger.Printf("nodeJoin of %s on %s", e.Node.URI, c.Node.URI) // Ignore the event if this is not the coordinator. if !c.isCoordinator() { return nil @@ -1660,6 +1685,7 @@ func (c *cluster) ReceiveEvent(e *NodeEvent) (err error) { func (c *cluster) nodeJoin(node *Node) error { c.mu.Lock() defer c.mu.Unlock() + c.logger.Printf("NodeJoin event on coordinator, node: %s, id: %s", node.URI, node.ID) if c.needTopologyAgreement() { // A host that is not part of the topology can't be added to the STARTING cluster. if !c.Topology.ContainsID(node.ID) { @@ -1688,11 +1714,10 @@ func (c *cluster) nodeJoin(node *Node) error { if c.haveTopologyAgreement() && c.allNodesReady() { return c.unprotectedSetStateAndBroadcast(ClusterStateNormal) - } else { - // Send the status to the remote node. This lets the remote node - // know that it can proceed with opening its Holder. - return c.sendTo(node, c.unprotectedStatus()) } + // Send the status to the remote node. This lets the remote node + // know that it can proceed with opening its Holder. + return c.sendTo(node, c.unprotectedStatus()) } // If the cluster already contains the node, just send it the cluster status. @@ -1796,6 +1821,10 @@ func (c *cluster) mergeClusterStatus(cs *ClusterStatus) error { // Add all nodes from the coordinator. for _, node := range officialNodes { + if node.ID == c.Node.ID && node.State != c.Node.State { + c.logger.Printf("mismatched state in mergeClusterStatus got %v have %v", node.State, c.Node.State) + go c.setNodeState(c.Node.State) + } if err := c.addNode(node); err != nil { return errors.Wrap(err, "adding node") } diff --git a/cluster_internal_test.go b/cluster_internal_test.go index a2b529fba..50602d385 100644 --- a/cluster_internal_test.go +++ b/cluster_internal_test.go @@ -185,8 +185,8 @@ func TestFragSources(t *testing.T) { "node0": {}, "node1": {}, "node2": { - {&Node{"node0", URI{"http", "host0", 10101}, false}, "i", "f", "standard", uint64(0)}, - {&Node{"node1", URI{"http", "host1", 10101}, false}, "i", "f", "standard", uint64(2)}, + {&Node{ID: "node0", URI: URI{"http", "host0", 10101}, IsCoordinator: false}, "i", "f", "standard", uint64(0)}, + {&Node{ID: "node1", URI: URI{"http", "host1", 10101}, IsCoordinator: false}, "i", "f", "standard", uint64(2)}, }, }, err: "", @@ -197,11 +197,11 @@ func TestFragSources(t *testing.T) { idx: idx, expected: map[string][]*ResizeSource{ "node0": { - {&Node{"node1", URI{"http", "host1", 10101}, false}, "i", "f", "standard", uint64(1)}, + {&Node{ID: "node1", URI: URI{"http", "host1", 10101}, IsCoordinator: false}, "i", "f", "standard", uint64(1)}, }, "node1": { - {&Node{"node0", URI{"http", "host0", 10101}, false}, "i", "f", "standard", uint64(0)}, - {&Node{"node0", URI{"http", "host0", 10101}, false}, "i", "f", "standard", uint64(2)}, + {&Node{ID: "node0", URI: URI{"http", "host0", 10101}, IsCoordinator: false}, "i", "f", "standard", uint64(0)}, + {&Node{ID: "node0", URI: URI{"http", "host0", 10101}, IsCoordinator: false}, "i", "f", "standard", uint64(2)}, }, }, err: "", @@ -212,11 +212,11 @@ func TestFragSources(t *testing.T) { idx: idx, expected: map[string][]*ResizeSource{ "node0": { - {&Node{"node2", URI{"http", "host2", 10101}, false}, "i", "f", "standard", uint64(0)}, - {&Node{"node2", URI{"http", "host2", 10101}, false}, "i", "f", "standard", uint64(2)}, + {&Node{ID: "node2", URI: URI{"http", "host2", 10101}, IsCoordinator: false}, "i", "f", "standard", uint64(0)}, + {&Node{ID: "node2", URI: URI{"http", "host2", 10101}, IsCoordinator: false}, "i", "f", "standard", uint64(2)}, }, "node1": { - {&Node{"node0", URI{"http", "host0", 10101}, false}, "i", "f", "standard", uint64(3)}, + {&Node{ID: "node0", URI: URI{"http", "host0", 10101}, IsCoordinator: false}, "i", "f", "standard", uint64(3)}, }, "node2": {}, }, diff --git a/encoding/proto/proto.go b/encoding/proto/proto.go index e8de4ea1a..9826c7ef9 100644 --- a/encoding/proto/proto.go +++ b/encoding/proto/proto.go @@ -506,6 +506,7 @@ func encodeNode(n *pilosa.Node) *internal.Node { ID: n.ID, URI: encodeURI(n.URI), IsCoordinator: n.IsCoordinator, + State: n.State, } } @@ -761,6 +762,7 @@ func decodeNode(node *internal.Node, m *pilosa.Node) { m.ID = node.ID decodeURI(node.URI, &m.URI) m.IsCoordinator = node.IsCoordinator + m.State = node.State } func decodeURI(i *internal.URI, m *pilosa.URI) { diff --git a/internal/clustertests/cluster_test.go b/internal/clustertests/cluster_test.go index 23d27fcba..21c9e35de 100644 --- a/internal/clustertests/cluster_test.go +++ b/internal/clustertests/cluster_test.go @@ -12,63 +12,77 @@ import ( pi "github.com/pilosa/pilosa" ) -func TestLongPauses(t *testing.T) { - t.Skip() // TODO figure out how to only run in the docker-compose environment +func TestClusterStuff(t *testing.T) { + if os.Getenv("ENABLE_PILOSA_CLUSTER_TESTS") != "1" { + t.Skip() + } cli := getPilosaClient(t) - idx := pilosa.NewIndex("testidx") - err := cli.CreateIndex(idx) - if err != nil { - t.Fatalf("creating index: %v", err) - } - f := idx.Field("testf", pilosa.OptFieldTypeSet(pilosa.CacheTypeRanked, 10)) - err = cli.CreateField(f) - if err != nil { - t.Fatalf("creating field: %v", err) - } + t.Run("long pause", func(t *testing.T) { + idx := pilosa.NewIndex("testidx") + err := cli.CreateIndex(idx) + if err != nil { + t.Fatalf("creating index: %v", err) + } + f := idx.Field("testf", pilosa.OptFieldTypeSet(pilosa.CacheTypeRanked, 10)) + err = cli.CreateField(f) + if err != nil { + t.Fatalf("creating field: %v", err) + } - data := make([]pilosa.Column, 1000) - for i := range data { - data[i].RowID = 0 - data[i].ColumnID = uint64((i/10)*pi.ShardWidth + i%10) - } + data := make([]pilosa.Column, 1000) + for i := range data { + data[i].RowID = 0 + data[i].ColumnID = uint64((i/10)*pi.ShardWidth + i%10) + } - err = cli.ImportField(f, &colIterator{cols: data}, pilosa.OptImportBatchSize(1000)) - if err != nil { - t.Fatalf("importing: %v", err) - } + err = cli.ImportField(f, &colIterator{cols: data}, pilosa.OptImportBatchSize(1000)) + if err != nil { + t.Fatalf("importing: %v", err) + } - r, err := cli.Query(idx.Count(f.Row(0))) - if err != nil { - t.Fatalf("count querying: %v", err) - } - if r.Result().Count() != 1000 { - t.Fatalf("count after import is %d", r.Result().Count()) - } + r, err := cli.Query(idx.Count(f.Row(0))) + if err != nil { + t.Fatalf("count querying: %v", err) + } + if r.Result().Count() != 1000 { + t.Fatalf("count after import is %d", r.Result().Count()) + } - pcmd := exec.Command("/pumba", "pause", "clustertests_pilosa3_1", "--duration", "30s") - pcmd.Stdout = os.Stdout - pcmd.Stderr = os.Stderr - fmt.Println("pausing pilosa3 for 30s") - err = pcmd.Start() - if err != nil { - t.Fatalf("starting pumba command: %v", err) - } - err = pcmd.Wait() - if err != nil { - t.Fatalf("waiting on pumba pause cmd: %v", err) - } - // TODO change the sleep to wait for status to return to NORMAL or timeout once we have Status.State support in go-pilosa - fmt.Println("done with pause, waiting for stability") - time.Sleep(time.Second * 400) - fmt.Println("done waiting") + pcmd := exec.Command("/pumba", "pause", "clustertests_pilosa3_1", "--duration", "10s") + pcmd.Stdout = os.Stdout + pcmd.Stderr = os.Stderr + fmt.Println("pausing pilosa3 for 10s") + err = pcmd.Start() + if err != nil { + t.Fatalf("starting pumba command: %v", err) + } + err = pcmd.Wait() + if err != nil { + t.Fatalf("waiting on pumba pause cmd: %v", err) + } + // TODO change the sleep to wait for status to return to NORMAL or timeout once we have Status.State support in go-pilosa + fmt.Println("done with pause, waiting for stability") + time.Sleep(time.Second * 3) + fmt.Println("done waiting") - r, err = cli.Query(idx.Count(f.Row(0))) + r, err = cli.Query(idx.Count(f.Row(0))) + if err != nil { + t.Fatalf("count querying: %v", err) + } else if r.Result().Count() != 1000 { + t.Fatalf("count after import is %d", r.Result().Count()) + } + + fmt.Println("at the bottom") + + }) + + down := exec.Command("/pumba", "stop", "clustertests_pilosa3_1", "clustertests_pilosa2_1", "clustertests_pilosa1_1") + down.Stdout = os.Stdout + down.Stderr = os.Stderr + err := down.Run() if err != nil { - t.Fatalf("count querying: %v", err) - } - if r.Result().Count() != 1000 { - t.Fatalf("count after import is %d", r.Result().Count()) + t.Logf("stopping Pilosa: %v", err) } } diff --git a/internal/clustertests/docker-compose.yml b/internal/clustertests/docker-compose.yml index 752c39177..7196ea0eb 100644 --- a/internal/clustertests/docker-compose.yml +++ b/internal/clustertests/docker-compose.yml @@ -41,13 +41,14 @@ services: - "/pilosa server --bind pilosa3:10101" client1: build: - context: ../.. - dockerfile: Dockerfile-withgo + context: . + environment: + - ENABLE_PILOSA_CLUSTER_TESTS=1 networks: - pilosanet volumes: - /var/run/docker.sock:/var/run/docker.sock command: - - "go test -v github.com/pilosa/pilosa/internal/clustertests" + - "go test -v -count=1 github.com/pilosa/pilosa/internal/clustertests" networks: pilosanet: diff --git a/internal/private.pb.go b/internal/private.pb.go index be4719e2f..d5e5d9251 100644 --- a/internal/private.pb.go +++ b/internal/private.pb.go @@ -540,6 +540,7 @@ type Node struct { ID string `protobuf:"bytes,1,opt,name=ID,proto3" json:"ID,omitempty"` URI *URI `protobuf:"bytes,2,opt,name=URI" json:"URI,omitempty"` IsCoordinator bool `protobuf:"varint,3,opt,name=IsCoordinator,proto3" json:"IsCoordinator,omitempty"` + State string `protobuf:"bytes,4,opt,name=State,proto3" json:"State,omitempty"` } func (m *Node) Reset() { *m = Node{} } @@ -568,6 +569,13 @@ func (m *Node) GetIsCoordinator() bool { return false } +func (m *Node) GetState() string { + if m != nil { + return m.State + } + return "" +} + type NodeStateMessage struct { NodeID string `protobuf:"bytes,1,opt,name=NodeID,proto3" json:"NodeID,omitempty"` State string `protobuf:"bytes,2,opt,name=State,proto3" json:"State,omitempty"` @@ -1749,6 +1757,12 @@ func (m *Node) MarshalTo(dAtA []byte) (int, error) { } i++ } + if len(m.State) > 0 { + dAtA[i] = 0x22 + i++ + i = encodeVarintPrivate(dAtA, i, uint64(len(m.State))) + i += copy(dAtA[i:], m.State) + } return i, nil } @@ -2675,6 +2689,10 @@ func (m *Node) Size() (n int) { if m.IsCoordinator { n += 2 } + l = len(m.State) + if l > 0 { + n += 1 + l + sovPrivate(uint64(l)) + } return n } @@ -5227,6 +5245,35 @@ func (m *Node) Unmarshal(dAtA []byte) error { } } m.IsCoordinator = bool(v != 0) + case 4: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field State", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPrivate + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= (uint64(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPrivate + } + postIndex := iNdEx + intStringLen + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.State = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex default: iNdEx = preIndex skippy, err := skipPrivate(dAtA[iNdEx:]) @@ -7399,75 +7446,76 @@ var ( func init() { proto.RegisterFile("private.proto", fileDescriptorPrivate) } var fileDescriptorPrivate = []byte{ - // 1113 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xac, 0x57, 0xdb, 0x6e, 0x1b, 0x45, - 0x18, 0x66, 0x0f, 0x76, 0xec, 0xdf, 0x75, 0x9a, 0x6c, 0x69, 0xd9, 0x02, 0x0a, 0x61, 0x54, 0xd1, - 0x50, 0x89, 0x50, 0xb5, 0x37, 0x9c, 0x2a, 0x95, 0xc4, 0xa1, 0x2c, 0x25, 0xa5, 0xcc, 0xa6, 0xb9, - 0xeb, 0xc5, 0xc4, 0x1e, 0x35, 0xab, 0xac, 0x77, 0xcc, 0xee, 0x6c, 0x12, 0xf7, 0x82, 0x5b, 0x90, - 0x78, 0x01, 0xc4, 0x93, 0xf0, 0x08, 0x5c, 0xf2, 0x08, 0x28, 0xbc, 0x08, 0x9a, 0x7f, 0x66, 0x76, - 0x37, 0x8e, 0x43, 0xa2, 0xc0, 0xdd, 0xfc, 0xdf, 0x7f, 0x3e, 0xae, 0x0d, 0xfd, 0x49, 0x9e, 0x1c, - 0x32, 0xc9, 0xd7, 0x27, 0xb9, 0x90, 0x22, 0xe8, 0x24, 0x99, 0xe4, 0x79, 0xc6, 0x52, 0xf2, 0x04, - 0xba, 0x51, 0x36, 0xe2, 0xc7, 0xdb, 0x5c, 0xb2, 0x20, 0x00, 0xff, 0x29, 0x9f, 0x16, 0xa1, 0xb7, - 0xea, 0xac, 0x75, 0x28, 0xbe, 0x83, 0x0f, 0x60, 0x71, 0x27, 0x67, 0xc3, 0x83, 0xad, 0xe3, 0xa4, - 0x90, 0x3c, 0x1b, 0xf2, 0xd0, 0x47, 0xee, 0x0c, 0x4a, 0x7e, 0x77, 0xe0, 0xda, 0x57, 0x09, 0x4f, - 0x47, 0xdf, 0x4d, 0x64, 0x22, 0xb2, 0x22, 0x78, 0x17, 0xba, 0x9b, 0x6c, 0xb8, 0xcf, 0x77, 0xa6, - 0x13, 0x8e, 0x16, 0xbb, 0xb4, 0x06, 0x2a, 0x6e, 0x9c, 0xbc, 0xd6, 0x16, 0xfb, 0xb4, 0x06, 0x82, - 0x55, 0xe8, 0xed, 0x24, 0x63, 0xfe, 0x7d, 0xc9, 0x32, 0x59, 0x8e, 0xc3, 0x16, 0x6a, 0x37, 0x21, - 0x15, 0x2a, 0x1a, 0xee, 0x20, 0x0b, 0xdf, 0xc1, 0x12, 0x78, 0xdb, 0x49, 0x16, 0x76, 0x57, 0x9d, - 0x35, 0x8f, 0xaa, 0x27, 0x22, 0xec, 0x38, 0x04, 0x83, 0xb0, 0xe3, 0x2a, 0xc5, 0x5e, 0x9d, 0x22, - 0x21, 0xb0, 0x18, 0x8d, 0x27, 0x22, 0x97, 0x94, 0x17, 0x13, 0x91, 0x15, 0x68, 0x69, 0x2b, 0xcf, - 0x43, 0x07, 0x8d, 0xab, 0x27, 0xf9, 0x11, 0x96, 0x36, 0x52, 0x31, 0x3c, 0x18, 0x30, 0xc9, 0x28, - 0xff, 0xa1, 0xe4, 0x85, 0x0c, 0xde, 0x84, 0x16, 0xd6, 0xce, 0xc8, 0x69, 0x42, 0xa1, 0x58, 0x87, - 0xd0, 0xd5, 0x28, 0x12, 0x0a, 0x45, 0x7d, 0xac, 0x84, 0x4f, 0x35, 0xa1, 0xd0, 0x78, 0x9f, 0xe5, - 0x23, 0xac, 0x80, 0x4f, 0x35, 0xa1, 0x62, 0xdc, 0x4d, 0xf8, 0x91, 0x49, 0x1b, 0xdf, 0x24, 0x82, - 0xe5, 0x86, 0x7f, 0x13, 0xe6, 0x2d, 0x68, 0x53, 0x71, 0x14, 0x0d, 0x8a, 0xd0, 0x59, 0xf5, 0xd6, - 0x7c, 0x6a, 0x28, 0x2c, 0xae, 0x48, 0xcb, 0x71, 0xa6, 0x58, 0x2e, 0xb2, 0x6a, 0x80, 0xdc, 0x86, - 0x16, 0x56, 0x5a, 0x65, 0x59, 0xeb, 0xaa, 0x27, 0xf9, 0xc9, 0x81, 0xee, 0x36, 0x3b, 0xc6, 0x30, - 0x8a, 0xe0, 0x11, 0x74, 0x62, 0xc9, 0xb2, 0x91, 0x0a, 0x50, 0x09, 0xf5, 0x1e, 0xbc, 0xbf, 0x6e, - 0x07, 0x67, 0xbd, 0x12, 0x5b, 0xb7, 0x32, 0x5b, 0x99, 0xcc, 0xa7, 0xb4, 0x52, 0x79, 0xfb, 0x73, - 0xe8, 0x9f, 0x62, 0x29, 0x7f, 0x07, 0x7c, 0x6a, 0xab, 0x7a, 0xc0, 0xa7, 0x2a, 0xff, 0x43, 0x96, - 0x96, 0x1c, 0x6b, 0xe5, 0x53, 0x4d, 0x7c, 0xe6, 0x7e, 0xe2, 0x90, 0x5d, 0x08, 0x36, 0x73, 0xce, - 0x24, 0x47, 0x27, 0xdb, 0xbc, 0x28, 0xd8, 0x2b, 0x7e, 0x7e, 0xc5, 0x75, 0x15, 0xdd, 0x66, 0x15, - 0xab, 0x3e, 0x78, 0x8d, 0x3e, 0x90, 0x7b, 0x10, 0x0c, 0x78, 0xca, 0x25, 0x37, 0x53, 0xff, 0x2f, - 0x76, 0x49, 0x6c, 0x63, 0xb8, 0x58, 0x36, 0xb8, 0x0b, 0xbe, 0x5a, 0x21, 0x0c, 0xa1, 0xf7, 0xe0, - 0x46, 0x5d, 0xa7, 0x6a, 0xbb, 0x28, 0x0a, 0x90, 0xd4, 0x1a, 0xc5, 0x78, 0x2e, 0x4c, 0x6c, 0xce, - 0x28, 0xdd, 0x33, 0xae, 0x3c, 0x74, 0x75, 0xab, 0x76, 0xd5, 0x5c, 0x3f, 0xe3, 0xed, 0xb1, 0x4d, - 0xf7, 0xaa, 0xde, 0xc8, 0x10, 0xde, 0xd1, 0x16, 0xbe, 0x3c, 0x64, 0x49, 0xca, 0xf6, 0xd2, 0x4b, - 0x76, 0x64, 0x4e, 0xe0, 0x21, 0x2c, 0xa0, 0x6e, 0x34, 0x30, 0x5b, 0x60, 0x49, 0xf2, 0xd2, 0xc8, - 0xab, 0xd1, 0x7f, 0xc6, 0xc6, 0xdc, 0x58, 0xc3, 0x77, 0x95, 0xaf, 0x7b, 0x71, 0xbe, 0xca, 0xb1, - 0x5a, 0x17, 0x75, 0xc2, 0x3c, 0xe5, 0x18, 0x09, 0xf2, 0x10, 0xda, 0xf1, 0x70, 0x9f, 0x8f, 0x59, - 0xf0, 0x21, 0x2c, 0x60, 0x84, 0xbc, 0x30, 0x13, 0x7d, 0x7d, 0xa6, 0x53, 0xd4, 0xf2, 0xc9, 0xc0, - 0x64, 0x36, 0x37, 0xa6, 0xbb, 0xd0, 0x46, 0xef, 0x45, 0xe8, 0xcf, 0x9a, 0x41, 0x9c, 0x1a, 0x36, - 0xd9, 0x02, 0xef, 0x05, 0x8d, 0xd4, 0xa6, 0x62, 0x04, 0xd6, 0x8a, 0xa1, 0x94, 0xed, 0xaf, 0x45, - 0x21, 0x4d, 0x9d, 0xf0, 0xad, 0xb0, 0xe7, 0x22, 0x97, 0x58, 0xa3, 0x3e, 0xc5, 0x37, 0x79, 0x09, - 0xfe, 0x33, 0x31, 0xe2, 0xc1, 0x22, 0xb8, 0xd1, 0xc0, 0xd8, 0x70, 0xa3, 0x41, 0xf0, 0x1e, 0x9a, - 0x37, 0xa5, 0xe9, 0xd7, 0x41, 0xbc, 0xa0, 0x11, 0x45, 0xc7, 0x77, 0xa0, 0x1f, 0x15, 0x9b, 0x42, - 0xe4, 0xa3, 0x24, 0x63, 0x52, 0xe4, 0xe6, 0xb6, 0x9f, 0x06, 0xc9, 0x63, 0x58, 0x52, 0xe6, 0x63, - 0xc9, 0x24, 0xb7, 0x9d, 0xbd, 0x05, 0x6d, 0x85, 0x55, 0xee, 0x0c, 0x85, 0xdb, 0xa6, 0xe4, 0x6c, - 0x6f, 0x91, 0x20, 0xdf, 0x6a, 0x0b, 0x5b, 0x87, 0x3c, 0x93, 0x8d, 0xd9, 0x40, 0x1a, 0x0d, 0xf4, - 0xa9, 0x26, 0x02, 0xa2, 0x53, 0x31, 0x31, 0x2f, 0xd6, 0x31, 0x2b, 0x94, 0x22, 0x8f, 0xfc, 0xe2, - 0x00, 0xd8, 0x80, 0xca, 0xa2, 0x52, 0x71, 0xce, 0x57, 0x09, 0xd6, 0x6c, 0x8f, 0xcd, 0x5e, 0x2c, - 0xd5, 0x52, 0x1a, 0xa7, 0x76, 0x06, 0x3e, 0xae, 0x67, 0x40, 0x37, 0xef, 0xe6, 0xcc, 0x0c, 0x68, - 0xaf, 0xf5, 0x24, 0x3c, 0x87, 0x5e, 0x03, 0x9f, 0x3b, 0x0f, 0x1f, 0x55, 0xf3, 0xe0, 0xce, 0x9a, - 0x44, 0xdc, 0x98, 0xb4, 0x53, 0xf1, 0x14, 0x7a, 0x0d, 0x78, 0xae, 0xc5, 0x35, 0xb8, 0x7e, 0x7a, - 0xe3, 0xec, 0x25, 0x9f, 0x85, 0x49, 0x02, 0xfd, 0xcd, 0xb4, 0x2c, 0x24, 0xcf, 0x8d, 0x39, 0x75, - 0xfe, 0x35, 0x50, 0x35, 0xaf, 0x06, 0xe6, 0xf7, 0x2f, 0xb8, 0x03, 0x2d, 0x55, 0x46, 0xbd, 0x38, - 0x67, 0x6b, 0xac, 0x99, 0x64, 0x17, 0x3a, 0x1b, 0x71, 0xf4, 0x24, 0x17, 0xe5, 0x64, 0x6e, 0xd0, - 0xf6, 0xab, 0xec, 0x9e, 0xfd, 0x2a, 0x7b, 0x67, 0xbe, 0xca, 0x7e, 0xf5, 0x55, 0x26, 0x31, 0x2c, - 0xeb, 0xa3, 0xa8, 0xf6, 0xf5, 0x2a, 0xa7, 0xc5, 0x7e, 0x32, 0xbd, 0xc6, 0x27, 0x33, 0x86, 0x65, - 0x7d, 0xb9, 0xfe, 0x4f, 0xa3, 0xbf, 0xb9, 0xb0, 0x4c, 0x79, 0x91, 0xbc, 0xe6, 0x51, 0x56, 0xc8, - 0xbc, 0x1c, 0xaa, 0xeb, 0xa3, 0xf4, 0xbf, 0x11, 0x7b, 0xa6, 0xda, 0x1e, 0xd5, 0xc4, 0x65, 0x26, - 0x3d, 0xb8, 0x0f, 0xbd, 0xd9, 0xed, 0x3c, 0x2b, 0xda, 0x14, 0x09, 0xee, 0xc3, 0x42, 0x2c, 0xca, - 0x7c, 0x58, 0x8d, 0x6f, 0xe3, 0x22, 0xea, 0xc8, 0x34, 0x9b, 0x5a, 0xb1, 0xc6, 0x6a, 0xb4, 0x2e, - 0x58, 0x8d, 0x47, 0x33, 0xa3, 0x14, 0xb6, 0x51, 0xe1, 0xad, 0x5a, 0xe1, 0x14, 0x9b, 0x9e, 0x96, - 0x26, 0x3f, 0x3b, 0x70, 0xad, 0x19, 0xc2, 0xa5, 0x16, 0xb7, 0xea, 0x88, 0x3b, 0xb7, 0x23, 0xde, - 0xbc, 0x8e, 0xf8, 0x75, 0x47, 0xea, 0xaf, 0x7f, 0xab, 0xf1, 0xf5, 0x27, 0x07, 0x70, 0xfb, 0x4c, - 0x9b, 0x36, 0xc5, 0x78, 0xa2, 0xe6, 0xe1, 0x3f, 0xb4, 0x4b, 0x9d, 0xb4, 0x3c, 0x37, 0x8d, 0xea, - 0x52, 0x4d, 0x90, 0x4f, 0xe1, 0x66, 0xcc, 0x65, 0xa3, 0x49, 0x76, 0xda, 0x56, 0xc1, 0x7b, 0xc6, - 0x8f, 0xce, 0x49, 0x5f, 0xb1, 0xc8, 0x17, 0x10, 0xbe, 0x98, 0x8c, 0x98, 0xe4, 0x57, 0xd2, 0xde, - 0x80, 0xce, 0x8e, 0x98, 0x88, 0x54, 0xbc, 0x9a, 0x5e, 0xb0, 0xf5, 0x21, 0x2c, 0xe8, 0xfb, 0xad, - 0xcf, 0x48, 0x97, 0x5a, 0x92, 0xdc, 0x50, 0x03, 0x3d, 0x64, 0xe9, 0xb0, 0x4c, 0x55, 0x18, 0xea, - 0x97, 0x61, 0xb1, 0xb1, 0xf4, 0xc7, 0xc9, 0x8a, 0xf3, 0xe7, 0xc9, 0x8a, 0xf3, 0xd7, 0xc9, 0x8a, - 0xf3, 0xeb, 0xdf, 0x2b, 0x6f, 0xec, 0xb5, 0xf1, 0x9f, 0xc3, 0xc3, 0x7f, 0x02, 0x00, 0x00, 0xff, - 0xff, 0xba, 0x1b, 0x62, 0x68, 0x4a, 0x0c, 0x00, 0x00, + // 1121 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xac, 0x57, 0xdd, 0x6e, 0x1b, 0xc5, + 0x17, 0xff, 0xef, 0x87, 0x1d, 0xfb, 0xb8, 0x4e, 0x93, 0xed, 0xbf, 0x61, 0x0b, 0x28, 0x84, 0x51, + 0x45, 0x43, 0x25, 0x42, 0xd5, 0xde, 0xf0, 0x55, 0xa9, 0x24, 0x0e, 0x65, 0x29, 0x09, 0x65, 0x9c, + 0xe4, 0x8e, 0x8b, 0x89, 0x3d, 0x6a, 0x56, 0x59, 0xef, 0x98, 0xdd, 0xd9, 0x24, 0xee, 0x05, 0xb7, + 0x20, 0xf1, 0x02, 0x88, 0x27, 0xe1, 0x11, 0xb8, 0xe4, 0x11, 0x50, 0x78, 0x11, 0x34, 0x67, 0x66, + 0x76, 0x37, 0x8e, 0x83, 0xa3, 0xc0, 0xdd, 0x9c, 0xdf, 0x99, 0xf9, 0x9d, 0xef, 0xb3, 0x36, 0x74, + 0xc7, 0x59, 0x7c, 0xc2, 0x24, 0xdf, 0x18, 0x67, 0x42, 0x8a, 0xa0, 0x15, 0xa7, 0x92, 0x67, 0x29, + 0x4b, 0xc8, 0x73, 0x68, 0x47, 0xe9, 0x90, 0x9f, 0xed, 0x70, 0xc9, 0x82, 0x00, 0xfc, 0x17, 0x7c, + 0x92, 0x87, 0xde, 0x9a, 0xb3, 0xde, 0xa2, 0x78, 0x0e, 0xde, 0x83, 0xc5, 0xbd, 0x8c, 0x0d, 0x8e, + 0xb7, 0xcf, 0xe2, 0x5c, 0xf2, 0x74, 0xc0, 0x43, 0x1f, 0xb5, 0x53, 0x28, 0xf9, 0xcd, 0x81, 0x5b, + 0x5f, 0xc4, 0x3c, 0x19, 0x7e, 0x33, 0x96, 0xb1, 0x48, 0xf3, 0xe0, 0x6d, 0x68, 0x6f, 0xb1, 0xc1, + 0x11, 0xdf, 0x9b, 0x8c, 0x39, 0x32, 0xb6, 0x69, 0x05, 0x94, 0xda, 0x7e, 0xfc, 0x5a, 0x33, 0x76, + 0x69, 0x05, 0x04, 0x6b, 0xd0, 0xd9, 0x8b, 0x47, 0xfc, 0xdb, 0x82, 0xa5, 0xb2, 0x18, 0x85, 0x0d, + 0x7c, 0x5d, 0x87, 0x94, 0xab, 0x48, 0xdc, 0x42, 0x15, 0x9e, 0x83, 0x25, 0xf0, 0x76, 0xe2, 0x34, + 0x6c, 0xaf, 0x39, 0xeb, 0x1e, 0x55, 0x47, 0x44, 0xd8, 0x59, 0x08, 0x06, 0x61, 0x67, 0x65, 0x88, + 0x9d, 0x2a, 0x44, 0x42, 0x60, 0x31, 0x1a, 0x8d, 0x45, 0x26, 0x29, 0xcf, 0xc7, 0x22, 0xcd, 0x91, + 0x69, 0x3b, 0xcb, 0x42, 0x07, 0xc9, 0xd5, 0x91, 0xfc, 0x00, 0x4b, 0x9b, 0x89, 0x18, 0x1c, 0xf7, + 0x98, 0x64, 0x94, 0x7f, 0x5f, 0xf0, 0x5c, 0x06, 0xff, 0x87, 0x06, 0xe6, 0xce, 0xdc, 0xd3, 0x82, + 0x42, 0x31, 0x0f, 0xa1, 0xab, 0x51, 0x14, 0x14, 0x8a, 0xef, 0x31, 0x13, 0x3e, 0xd5, 0x82, 0x42, + 0xfb, 0x47, 0x2c, 0x1b, 0x62, 0x06, 0x7c, 0xaa, 0x05, 0xe5, 0xe3, 0x41, 0xcc, 0x4f, 0x4d, 0xd8, + 0x78, 0x26, 0x11, 0x2c, 0xd7, 0xec, 0x1b, 0x37, 0x57, 0xa0, 0x49, 0xc5, 0x69, 0xd4, 0xcb, 0x43, + 0x67, 0xcd, 0x5b, 0xf7, 0xa9, 0x91, 0x30, 0xb9, 0x22, 0x29, 0x46, 0xa9, 0x52, 0xb9, 0xa8, 0xaa, + 0x00, 0x72, 0x0f, 0x1a, 0x98, 0x69, 0x15, 0x65, 0xf5, 0x56, 0x1d, 0xc9, 0x8f, 0x0e, 0xb4, 0x77, + 0xd8, 0x19, 0xba, 0x91, 0x07, 0x4f, 0xa1, 0xd5, 0x97, 0x2c, 0x1d, 0x2a, 0x07, 0xd5, 0xa5, 0xce, + 0xe3, 0x77, 0x37, 0x6c, 0xe3, 0x6c, 0x94, 0xd7, 0x36, 0xec, 0x9d, 0xed, 0x54, 0x66, 0x13, 0x5a, + 0x3e, 0x79, 0xf3, 0x53, 0xe8, 0x5e, 0x50, 0x29, 0x7b, 0xc7, 0x7c, 0x62, 0xb3, 0x7a, 0xcc, 0x27, + 0x2a, 0xfe, 0x13, 0x96, 0x14, 0x1c, 0x73, 0xe5, 0x53, 0x2d, 0x7c, 0xe2, 0x7e, 0xe4, 0x90, 0x03, + 0x08, 0xb6, 0x32, 0xce, 0x24, 0x47, 0x23, 0x3b, 0x3c, 0xcf, 0xd9, 0x2b, 0x7e, 0x75, 0xc6, 0x75, + 0x16, 0xdd, 0x7a, 0x16, 0xcb, 0x3a, 0x78, 0xb5, 0x3a, 0x90, 0x87, 0x10, 0xf4, 0x78, 0xc2, 0x25, + 0x37, 0x5d, 0xff, 0x0f, 0xbc, 0xa4, 0x6f, 0x7d, 0x98, 0x7f, 0x37, 0x78, 0x00, 0xbe, 0x1a, 0x21, + 0x74, 0xa1, 0xf3, 0xf8, 0x4e, 0x95, 0xa7, 0x72, 0xba, 0x28, 0x5e, 0x20, 0x89, 0x25, 0x45, 0x7f, + 0xe6, 0x06, 0x36, 0xa3, 0x95, 0x1e, 0x1a, 0x53, 0x1e, 0x9a, 0x5a, 0xa9, 0x4c, 0xd5, 0xc7, 0xcf, + 0x58, 0x7b, 0x66, 0xc3, 0xbd, 0xa9, 0x35, 0x32, 0x80, 0xb7, 0x34, 0xc3, 0xe7, 0x27, 0x2c, 0x4e, + 0xd8, 0x61, 0x72, 0xcd, 0x8a, 0xcc, 0x70, 0x3c, 0x84, 0x05, 0x7c, 0x1b, 0xf5, 0xcc, 0x14, 0x58, + 0x91, 0x7c, 0x67, 0xee, 0xab, 0xd6, 0xdf, 0x65, 0x23, 0x6e, 0xd8, 0xf0, 0x5c, 0xc6, 0xeb, 0xce, + 0x8f, 0x57, 0x19, 0x56, 0xe3, 0xa2, 0x56, 0x98, 0xa7, 0x0c, 0xa3, 0x40, 0x9e, 0x40, 0xb3, 0x3f, + 0x38, 0xe2, 0x23, 0x16, 0xbc, 0x0f, 0x0b, 0xe8, 0x21, 0xcf, 0x4d, 0x47, 0xdf, 0x9e, 0xaa, 0x14, + 0xb5, 0x7a, 0xd2, 0x33, 0x91, 0xcd, 0xf4, 0xe9, 0x01, 0x34, 0xd1, 0x7a, 0x1e, 0xfa, 0xd3, 0x34, + 0x88, 0x53, 0xa3, 0x26, 0xdb, 0xe0, 0xed, 0xd3, 0x48, 0x4d, 0x2a, 0x7a, 0x60, 0x59, 0x8c, 0xa4, + 0xb8, 0xbf, 0x14, 0xb9, 0x34, 0x79, 0xc2, 0xb3, 0xc2, 0x5e, 0x8a, 0x4c, 0x62, 0x8e, 0xba, 0x14, + 0xcf, 0x24, 0x07, 0x7f, 0x57, 0x0c, 0x79, 0xb0, 0x08, 0x6e, 0xd4, 0x33, 0x1c, 0x6e, 0xd4, 0x0b, + 0xde, 0x41, 0x7a, 0x93, 0x9a, 0x6e, 0xe5, 0xc4, 0x3e, 0x8d, 0x28, 0x1a, 0xbe, 0x0f, 0xdd, 0x28, + 0xdf, 0x12, 0x22, 0x1b, 0xc6, 0x29, 0x93, 0x22, 0x33, 0xbb, 0xfd, 0x22, 0x88, 0x13, 0x24, 0x99, + 0xd4, 0x9b, 0xb8, 0x4d, 0xb5, 0x40, 0x9e, 0xc1, 0x92, 0x32, 0x8a, 0x82, 0xad, 0xf7, 0x0a, 0x34, + 0x15, 0x56, 0x3a, 0x61, 0xa4, 0x8a, 0xc1, 0xad, 0x33, 0x7c, 0xad, 0x19, 0xb6, 0x4f, 0x78, 0x2a, + 0x6b, 0x1d, 0x83, 0x32, 0x12, 0x74, 0xa9, 0x16, 0x02, 0xa2, 0x03, 0x34, 0x91, 0x2c, 0x56, 0x91, + 0x28, 0x94, 0xa2, 0x8e, 0xfc, 0xec, 0x00, 0x58, 0x87, 0x8a, 0xbc, 0x7c, 0xe2, 0x5c, 0xfd, 0x24, + 0x58, 0xb7, 0x95, 0x37, 0xd3, 0xb2, 0x54, 0xdd, 0xd2, 0x38, 0xb5, 0x9d, 0xf1, 0x61, 0xd5, 0x19, + 0xba, 0xa4, 0x77, 0xa7, 0x3a, 0x43, 0x5b, 0xad, 0xfa, 0xe3, 0x25, 0x74, 0x6a, 0xf8, 0xcc, 0x2e, + 0xf9, 0xa0, 0xec, 0x12, 0x77, 0x9a, 0x12, 0x71, 0x43, 0x69, 0x7b, 0xe5, 0x05, 0x74, 0x6a, 0xf0, + 0x4c, 0xc6, 0x75, 0xb8, 0x7d, 0x71, 0x0e, 0xed, 0x7e, 0x9f, 0x86, 0x49, 0x0c, 0xdd, 0xad, 0xa4, + 0xc8, 0x25, 0xcf, 0x0c, 0x9d, 0xfa, 0x28, 0x68, 0xa0, 0x2c, 0x5e, 0x05, 0xcc, 0xae, 0x5f, 0x70, + 0x1f, 0x1a, 0x2a, 0x8d, 0x7a, 0x9c, 0x2e, 0xe7, 0x58, 0x2b, 0xc9, 0x01, 0xb4, 0x36, 0xfb, 0xd1, + 0xf3, 0x4c, 0x14, 0xe3, 0x99, 0x4e, 0xdb, 0x6f, 0xb5, 0x7b, 0xf9, 0x5b, 0xed, 0x5d, 0xfa, 0x56, + 0xfb, 0xe5, 0xb7, 0x9a, 0xf4, 0x61, 0x59, 0xaf, 0x4a, 0x35, 0xc5, 0x37, 0x59, 0x38, 0xf6, 0x43, + 0xea, 0xd5, 0x3e, 0xa4, 0x7d, 0x58, 0xd6, 0xfb, 0xec, 0xbf, 0x24, 0xfd, 0xd5, 0x85, 0x65, 0xca, + 0xf3, 0xf8, 0x35, 0x8f, 0xd2, 0x5c, 0x66, 0xc5, 0x40, 0xed, 0x24, 0xf5, 0xfe, 0x2b, 0x71, 0x68, + 0xb2, 0xed, 0x51, 0x2d, 0x5c, 0xa7, 0xd3, 0x83, 0x47, 0xd0, 0x99, 0x9e, 0xd9, 0xcb, 0x57, 0xeb, + 0x57, 0x82, 0x47, 0xb0, 0xd0, 0x17, 0x45, 0x36, 0x28, 0xdb, 0xb7, 0xb6, 0x27, 0xb5, 0x67, 0x5a, + 0x4d, 0xed, 0xb5, 0xda, 0x68, 0x34, 0xe6, 0x8c, 0xc6, 0xd3, 0xa9, 0x56, 0x0a, 0x9b, 0xf8, 0xe0, + 0x8d, 0xea, 0xc1, 0x05, 0x35, 0xbd, 0x78, 0x9b, 0xfc, 0xe4, 0xc0, 0xad, 0xba, 0x0b, 0xd7, 0x1a, + 0xdc, 0xb2, 0x22, 0xee, 0xcc, 0x8a, 0x78, 0xb3, 0x2a, 0xe2, 0x57, 0x15, 0xa9, 0x7e, 0x13, 0x34, + 0x6a, 0xbf, 0x09, 0xc8, 0x31, 0xdc, 0xbb, 0x54, 0xa6, 0x2d, 0x31, 0x1a, 0xab, 0x7e, 0xf8, 0x17, + 0xe5, 0x52, 0x2b, 0x2d, 0xcb, 0x4c, 0xa1, 0xda, 0x54, 0x0b, 0xe4, 0x63, 0xb8, 0xdb, 0xe7, 0xb2, + 0x56, 0x24, 0xdb, 0x6d, 0x6b, 0xe0, 0xed, 0xf2, 0xd3, 0x2b, 0xc2, 0x57, 0x2a, 0xf2, 0x19, 0x84, + 0xfb, 0xe3, 0x21, 0x93, 0xfc, 0x46, 0xaf, 0x37, 0xa1, 0xb5, 0x27, 0xc6, 0x22, 0x11, 0xaf, 0x26, + 0x73, 0xa6, 0x3e, 0x84, 0x05, 0xbd, 0xbf, 0xf5, 0x1a, 0x69, 0x53, 0x2b, 0x92, 0x3b, 0xaa, 0xa1, + 0x07, 0x2c, 0x19, 0x14, 0x89, 0x72, 0x43, 0xfd, 0x5e, 0xcc, 0x37, 0x97, 0x7e, 0x3f, 0x5f, 0x75, + 0xfe, 0x38, 0x5f, 0x75, 0xfe, 0x3c, 0x5f, 0x75, 0x7e, 0xf9, 0x6b, 0xf5, 0x7f, 0x87, 0x4d, 0xfc, + 0x3f, 0xf1, 0xe4, 0xef, 0x00, 0x00, 0x00, 0xff, 0xff, 0x46, 0x93, 0xc0, 0xc1, 0x60, 0x0c, 0x00, + 0x00, } diff --git a/internal/private.proto b/internal/private.proto index 57b98f62c..2e484b037 100644 --- a/internal/private.proto +++ b/internal/private.proto @@ -99,6 +99,7 @@ message Node { string ID = 1; URI URI = 2; bool IsCoordinator = 3; + string State = 4; } message NodeStateMessage { diff --git a/server.go b/server.go index a4cb8fc8b..5e6f61087 100644 --- a/server.go +++ b/server.go @@ -298,6 +298,7 @@ func NewServer(opts ...ServerOption) (*Server, error) { ID: s.nodeID, URI: s.uri, IsCoordinator: s.cluster.Coordinator == s.nodeID, + State: nodeStateDown, } s.cluster.Node = node if s.clusterDisabled { @@ -561,7 +562,10 @@ func (s *Server) receiveMessage(m Message) error { case *RecalculateCaches: s.holder.recalculateCaches() case *NodeEvent: - s.cluster.ReceiveEvent(obj) + err := s.cluster.ReceiveEvent(obj) + if err != nil { + return errors.Wrapf(err, "cluster receiving NodeEvent %v", obj) + } case *NodeStatus: s.handleRemoteStatus(obj) } diff --git a/server/server.go b/server/server.go index 010eb2f8a..79d25d270 100644 --- a/server/server.go +++ b/server/server.go @@ -11,7 +11,7 @@ // 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 server contains the `pilosa server` subcommand which runs Pilosa // itself. The purpose of this package is to define an easily tested Command // object which handles interpreting configuration and setting up all the From dd4685d43b4994c6bda995037fd087d00ba33b7a Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 9 Nov 2018 11:28:53 -0600 Subject: [PATCH 12/33] msg type stringer --- broadcast.go | 12 ++++++++---- msgtype_string.go | 16 ++++++++++++++++ server.go | 6 +++--- 3 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 msgtype_string.go diff --git a/broadcast.go b/broadcast.go index 6f2245992..d18cdc937 100644 --- a/broadcast.go +++ b/broadcast.go @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +//go:generate stringer -type=msgType + package pilosa import ( @@ -53,7 +55,7 @@ func (nopBroadcaster) SendTo(*Node, Message) error { return nil } // Broadcast message types. const ( - messageTypeCreateShard = iota + messageTypeCreateShard msgType = iota messageTypeCreateIndex messageTypeDeleteIndex messageTypeCreateField @@ -71,6 +73,8 @@ const ( messageTypeNodeStatus ) +type msgType byte + // MarshalInternalMessage serializes the pilosa message and adds pilosa internal // type info which is used by the internal messaging stuff. func MarshalInternalMessage(m Message, s Serializer) ([]byte, error) { @@ -79,11 +83,11 @@ func MarshalInternalMessage(m Message, s Serializer) ([]byte, error) { if err != nil { return nil, errors.Wrap(err, "marshaling") } - return append([]byte{typ}, buf...), nil + return append([]byte{byte(typ)}, buf...), nil } func getMessage(typ byte) Message { - switch typ { + switch msgType(typ) { case messageTypeCreateShard: return &CreateShardMessage{} case messageTypeCreateIndex: @@ -121,7 +125,7 @@ func getMessage(typ byte) Message { } } -func getMessageType(m Message) byte { +func getMessageType(m Message) msgType { switch m.(type) { case *CreateShardMessage: return messageTypeCreateShard diff --git a/msgtype_string.go b/msgtype_string.go new file mode 100644 index 000000000..d4c101bbe --- /dev/null +++ b/msgtype_string.go @@ -0,0 +1,16 @@ +// Code generated by "stringer -type=msgType"; DO NOT EDIT. + +package pilosa + +import "strconv" + +const _msgType_name = "messageTypeCreateShardmessageTypeCreateIndexmessageTypeDeleteIndexmessageTypeCreateFieldmessageTypeDeleteFieldmessageTypeCreateViewmessageTypeDeleteViewmessageTypeClusterStatusmessageTypeResizeInstructionmessageTypeResizeInstructionCompletemessageTypeSetCoordinatormessageTypeUpdateCoordinatormessageTypeNodeStatemessageTypeRecalculateCachesmessageTypeNodeEventmessageTypeNodeStatus" + +var _msgType_index = [...]uint16{0, 22, 44, 66, 88, 110, 131, 152, 176, 204, 240, 265, 293, 313, 341, 361, 382} + +func (i msgType) String() string { + if i >= msgType(len(_msgType_index)-1) { + return "msgType(" + strconv.FormatInt(int64(i), 10) + ")" + } + return _msgType_name[_msgType_index[i]:_msgType_index[i+1]] +} diff --git a/server.go b/server.go index 5e6f61087..fbbf3fa42 100644 --- a/server.go +++ b/server.go @@ -580,7 +580,7 @@ func (s *Server) SendSync(m Message) error { if err != nil { return fmt.Errorf("marshaling message: %v", err) } - msg = append([]byte{getMessageType(m)}, msg...) + msg = append([]byte{byte(getMessageType(m))}, msg...) for _, node := range s.cluster.nodes { node := node s.logger.Printf("SendSync to: %s", node.URI) @@ -604,12 +604,12 @@ func (s *Server) SendAsync(m Message) error { // SendTo represents an implementation of Broadcaster. func (s *Server) SendTo(to *Node, m Message) error { - s.logger.Printf("SendTo: %s", to.URI) + s.logger.Printf("SendTo: %s, type: %s", to.URI, getMessageType(m)) msg, err := s.serializer.Marshal(m) if err != nil { return fmt.Errorf("marshaling message: %v", err) } - msg = append([]byte{getMessageType(m)}, msg...) + msg = append([]byte{byte(getMessageType(m))}, msg...) return s.defaultClient.SendMessage(context.Background(), &to.URI, msg) } From 8cd53bf2c2a41f21864ed3d3580804f046d9c2fd Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Fri, 9 Nov 2018 11:34:32 -0600 Subject: [PATCH 13/33] Revert "msg type stringer" This reverts commit dd4685d43b4994c6bda995037fd087d00ba33b7a. --- broadcast.go | 12 ++++-------- msgtype_string.go | 16 ---------------- server.go | 6 +++--- 3 files changed, 7 insertions(+), 27 deletions(-) delete mode 100644 msgtype_string.go diff --git a/broadcast.go b/broadcast.go index d18cdc937..6f2245992 100644 --- a/broadcast.go +++ b/broadcast.go @@ -12,8 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:generate stringer -type=msgType - package pilosa import ( @@ -55,7 +53,7 @@ func (nopBroadcaster) SendTo(*Node, Message) error { return nil } // Broadcast message types. const ( - messageTypeCreateShard msgType = iota + messageTypeCreateShard = iota messageTypeCreateIndex messageTypeDeleteIndex messageTypeCreateField @@ -73,8 +71,6 @@ const ( messageTypeNodeStatus ) -type msgType byte - // MarshalInternalMessage serializes the pilosa message and adds pilosa internal // type info which is used by the internal messaging stuff. func MarshalInternalMessage(m Message, s Serializer) ([]byte, error) { @@ -83,11 +79,11 @@ func MarshalInternalMessage(m Message, s Serializer) ([]byte, error) { if err != nil { return nil, errors.Wrap(err, "marshaling") } - return append([]byte{byte(typ)}, buf...), nil + return append([]byte{typ}, buf...), nil } func getMessage(typ byte) Message { - switch msgType(typ) { + switch typ { case messageTypeCreateShard: return &CreateShardMessage{} case messageTypeCreateIndex: @@ -125,7 +121,7 @@ func getMessage(typ byte) Message { } } -func getMessageType(m Message) msgType { +func getMessageType(m Message) byte { switch m.(type) { case *CreateShardMessage: return messageTypeCreateShard diff --git a/msgtype_string.go b/msgtype_string.go deleted file mode 100644 index d4c101bbe..000000000 --- a/msgtype_string.go +++ /dev/null @@ -1,16 +0,0 @@ -// Code generated by "stringer -type=msgType"; DO NOT EDIT. - -package pilosa - -import "strconv" - -const _msgType_name = "messageTypeCreateShardmessageTypeCreateIndexmessageTypeDeleteIndexmessageTypeCreateFieldmessageTypeDeleteFieldmessageTypeCreateViewmessageTypeDeleteViewmessageTypeClusterStatusmessageTypeResizeInstructionmessageTypeResizeInstructionCompletemessageTypeSetCoordinatormessageTypeUpdateCoordinatormessageTypeNodeStatemessageTypeRecalculateCachesmessageTypeNodeEventmessageTypeNodeStatus" - -var _msgType_index = [...]uint16{0, 22, 44, 66, 88, 110, 131, 152, 176, 204, 240, 265, 293, 313, 341, 361, 382} - -func (i msgType) String() string { - if i >= msgType(len(_msgType_index)-1) { - return "msgType(" + strconv.FormatInt(int64(i), 10) + ")" - } - return _msgType_name[_msgType_index[i]:_msgType_index[i+1]] -} diff --git a/server.go b/server.go index fbbf3fa42..5e6f61087 100644 --- a/server.go +++ b/server.go @@ -580,7 +580,7 @@ func (s *Server) SendSync(m Message) error { if err != nil { return fmt.Errorf("marshaling message: %v", err) } - msg = append([]byte{byte(getMessageType(m))}, msg...) + msg = append([]byte{getMessageType(m)}, msg...) for _, node := range s.cluster.nodes { node := node s.logger.Printf("SendSync to: %s", node.URI) @@ -604,12 +604,12 @@ func (s *Server) SendAsync(m Message) error { // SendTo represents an implementation of Broadcaster. func (s *Server) SendTo(to *Node, m Message) error { - s.logger.Printf("SendTo: %s, type: %s", to.URI, getMessageType(m)) + s.logger.Printf("SendTo: %s", to.URI) msg, err := s.serializer.Marshal(m) if err != nil { return fmt.Errorf("marshaling message: %v", err) } - msg = append([]byte{byte(getMessageType(m))}, msg...) + msg = append([]byte{getMessageType(m)}, msg...) return s.defaultClient.SendMessage(context.Background(), &to.URI, msg) } From 90c5f64b194f827bb68fb23301f6c793c1d1c714 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Mon, 12 Nov 2018 14:01:57 -0600 Subject: [PATCH 14/33] filter memberlist debug and info logs, use t.Log instead of fmt in cluster tests --- gossip/gossip.go | 15 ++++++++++++++- internal/clustertests/cluster_test.go | 10 ++++------ server/server.go | 24 +++++++++++++++++++++++- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/gossip/gossip.go b/gossip/gossip.go index 789ed50b0..ecd663ecf 100644 --- a/gossip/gossip.go +++ b/gossip/gossip.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "fmt" + "io" "io/ioutil" "log" "net" @@ -49,6 +50,7 @@ type memberSet struct { Logger pilosa.Logger logger *log.Logger + logOutput io.Writer transport *Transport eventReceiver *eventReceiver @@ -156,6 +158,13 @@ func WithLogger(logger *log.Logger) memberSetOption { } } +func WithLogOutput(o io.Writer) memberSetOption { + return func(g *memberSet) error { + g.logOutput = o + return nil + } +} + // NewMemberSet returns a new instance of GossipMemberSet based on options. func NewMemberSet(cfg Config, api *pilosa.API, options ...memberSetOption) (*memberSet, error) { host := api.Node().URI.Host @@ -220,7 +229,11 @@ func NewMemberSet(cfg Config, api *pilosa.API, options ...memberSetOption) (*mem conf.Delegate = g conf.SecretKey = gossipKey conf.Events = ger - conf.Logger = g.logger + if g.logOutput != nil { + conf.LogOutput = g.logOutput + } else { + conf.Logger = g.logger + } g.config = &config{ memberlistConfig: conf, diff --git a/internal/clustertests/cluster_test.go b/internal/clustertests/cluster_test.go index 21c9e35de..7ced51042 100644 --- a/internal/clustertests/cluster_test.go +++ b/internal/clustertests/cluster_test.go @@ -1,7 +1,6 @@ package clustertest import ( - "fmt" "io" "os" "os/exec" @@ -19,6 +18,7 @@ func TestClusterStuff(t *testing.T) { cli := getPilosaClient(t) t.Run("long pause", func(t *testing.T) { + idx := pilosa.NewIndex("testidx") err := cli.CreateIndex(idx) if err != nil { @@ -52,7 +52,7 @@ func TestClusterStuff(t *testing.T) { pcmd := exec.Command("/pumba", "pause", "clustertests_pilosa3_1", "--duration", "10s") pcmd.Stdout = os.Stdout pcmd.Stderr = os.Stderr - fmt.Println("pausing pilosa3 for 10s") + t.Log("pausing pilosa3 for 10s") err = pcmd.Start() if err != nil { t.Fatalf("starting pumba command: %v", err) @@ -62,9 +62,9 @@ func TestClusterStuff(t *testing.T) { t.Fatalf("waiting on pumba pause cmd: %v", err) } // TODO change the sleep to wait for status to return to NORMAL or timeout once we have Status.State support in go-pilosa - fmt.Println("done with pause, waiting for stability") + t.Log("done with pause, waiting for stability") time.Sleep(time.Second * 3) - fmt.Println("done waiting") + t.Log("done waiting for stability") r, err = cli.Query(idx.Count(f.Row(0))) if err != nil { @@ -73,8 +73,6 @@ func TestClusterStuff(t *testing.T) { t.Fatalf("count after import is %d", r.Result().Count()) } - fmt.Println("at the bottom") - }) down := exec.Command("/pumba", "stop", "clustertests_pilosa3_1", "clustertests_pilosa2_1", "clustertests_pilosa1_1") diff --git a/server/server.go b/server/server.go index 79d25d270..6cdc43883 100644 --- a/server/server.go +++ b/server/server.go @@ -20,6 +20,7 @@ package server import ( + "bytes" "crypto/tls" "io" "log" @@ -336,7 +337,7 @@ func (m *Command) setupNetworking() error { gossipMemberSet, err := gossip.NewMemberSet( m.Config.Gossip, m.API, - gossip.WithLogger(m.logger.Logger()), + gossip.WithLogOutput(&filteredWriter{logOutput: m.logOutput, v: m.Config.Verbose}), gossip.WithTransport(m.gossipTransport), ) if err != nil { @@ -407,3 +408,24 @@ func getListener(uri pilosa.URI, tlsconf *tls.Config) (ln net.Listener, err erro return ln, nil } + +type filteredWriter struct { + v bool + logOutput io.Writer +} + +// Write forwards the write to logOutput if verbose is true, or it doesn't +// contain [DEBUG] or [INFO]. This implementation isn't technically correct +// since Write could be called with only part of a log line, but I don't think +// that actually happens, so until it becomes a problem, I don't think it's +// worth dealing with the extra complexity. (jaffee) +func (f *filteredWriter) Write(p []byte) (n int, err error) { + if bytes.Contains(p, []byte("[DEBUG]")) || bytes.Contains(p, []byte("[INFO]")) { + if f.v { + return f.logOutput.Write(p) + } + } else { + return f.logOutput.Write(p) + } + return len(p), nil +} From 37ac8b7a93adea7325c6b728bd7e315f9d0253f6 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Mon, 12 Nov 2018 17:44:23 -0600 Subject: [PATCH 15/33] better use of docker-compose opts per code review --- Makefile | 11 +++++------ internal/clustertests/cluster_test.go | 9 --------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/Makefile b/Makefile index 9cf4cffaf..34f97965e 100644 --- a/Makefile +++ b/Makefile @@ -73,16 +73,15 @@ release: check-clean # make changes to Pilosa, you'll want to run clustertests-build to rebuild the # pilosa image. clustertests: - cd internal/clustertests;\ - docker-compose down;\ - docker-compose up; + docker-compose -f internal/clustertests/docker-compose.yml down + docker-compose -f internal/clustertests/docker-compose.yml build client1 + docker-compose -f internal/clustertests/docker-compose.yml up --exit-code-from=client1 # Like clustertests, but rebuilds all images. clustertests-build: - cd internal/clustertests;\ - docker-compose down;\ - docker-compose up --build; + docker-compose -f internal/clustertests/docker-compose.yml down + docker-compose -f internal/clustertests/docker-compose.yml up --exit-code-from=client1 --build # Create prerelease builds prerelease: vendor diff --git a/internal/clustertests/cluster_test.go b/internal/clustertests/cluster_test.go index 7ced51042..402a8c14c 100644 --- a/internal/clustertests/cluster_test.go +++ b/internal/clustertests/cluster_test.go @@ -18,7 +18,6 @@ func TestClusterStuff(t *testing.T) { cli := getPilosaClient(t) t.Run("long pause", func(t *testing.T) { - idx := pilosa.NewIndex("testidx") err := cli.CreateIndex(idx) if err != nil { @@ -72,16 +71,8 @@ func TestClusterStuff(t *testing.T) { } else if r.Result().Count() != 1000 { t.Fatalf("count after import is %d", r.Result().Count()) } - }) - down := exec.Command("/pumba", "stop", "clustertests_pilosa3_1", "clustertests_pilosa2_1", "clustertests_pilosa1_1") - down.Stdout = os.Stdout - down.Stderr = os.Stderr - err := down.Run() - if err != nil { - t.Logf("stopping Pilosa: %v", err) - } } // Utils From 0d4a46af97f53bc719daebc902c8e3ce0cf131e6 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 13 Nov 2018 09:24:35 -0600 Subject: [PATCH 16/33] use internal client instead of go-pilosa, use ADD instead of wget --- Dockerfile-withgo | 2 +- internal/clustertests/cluster_test.go | 120 +++++++------------------- 2 files changed, 30 insertions(+), 92 deletions(-) diff --git a/Dockerfile-withgo b/Dockerfile-withgo index 834634b13..c60fda576 100644 --- a/Dockerfile-withgo +++ b/Dockerfile-withgo @@ -11,7 +11,7 @@ RUN cd /go/src/github.com/pilosa/pilosa \ && CGO_ENABLED=0 make install-dep install FLAGS="-a" # download pumba for fault injection -RUN wget https://github.com/alexei-led/pumba/releases/download/0.6.0/pumba_linux_amd64 -O /pumba +ADD https://github.com/alexei-led/pumba/releases/download/0.6.0/pumba_linux_amd64 /pumba RUN chmod +x /pumba RUN cp /go/bin/pilosa /pilosa diff --git a/internal/clustertests/cluster_test.go b/internal/clustertests/cluster_test.go index 402a8c14c..f01835c10 100644 --- a/internal/clustertests/cluster_test.go +++ b/internal/clustertests/cluster_test.go @@ -1,51 +1,54 @@ package clustertest import ( - "io" + "context" "os" "os/exec" "testing" "time" - "github.com/pilosa/go-pilosa" - pi "github.com/pilosa/pilosa" + "github.com/pilosa/pilosa" + picli "github.com/pilosa/pilosa/http" ) func TestClusterStuff(t *testing.T) { if os.Getenv("ENABLE_PILOSA_CLUSTER_TESTS") != "1" { t.Skip() } - cli := getPilosaClient(t) + cli, err := picli.NewInternalClient("pilosa1:10101", picli.GetHTTPClient(nil)) + if err != nil { + t.Fatalf("getting client: %v", err) + } t.Run("long pause", func(t *testing.T) { - idx := pilosa.NewIndex("testidx") - err := cli.CreateIndex(idx) + err := cli.CreateIndex(context.Background(), "testidx", pilosa.IndexOptions{}) if err != nil { t.Fatalf("creating index: %v", err) } - f := idx.Field("testf", pilosa.OptFieldTypeSet(pilosa.CacheTypeRanked, 10)) - err = cli.CreateField(f) + err = cli.CreateFieldWithOptions(context.Background(), "testidx", "testf", pilosa.FieldOptions{CacheType: pilosa.CacheTypeRanked, CacheSize: 100}) if err != nil { t.Fatalf("creating field: %v", err) } - data := make([]pilosa.Column, 1000) - for i := range data { - data[i].RowID = 0 - data[i].ColumnID = uint64((i/10)*pi.ShardWidth + i%10) + data := make([]pilosa.Bit, 10) + for i := 0; i < 1000; i++ { + data[i%10].RowID = 0 + data[i%10].ColumnID = uint64((i/10)*pilosa.ShardWidth + i%10) + shard := uint64(i / 10) + if i%10 == 9 { + err = cli.Import(context.Background(), "testidx", "testf", shard, data) + if err != nil { + t.Fatalf("importing: %v", err) + } + } } - err = cli.ImportField(f, &colIterator{cols: data}, pilosa.OptImportBatchSize(1000)) - if err != nil { - t.Fatalf("importing: %v", err) - } - - r, err := cli.Query(idx.Count(f.Row(0))) + r, err := cli.Query(context.Background(), "testidx", &pilosa.QueryRequest{Index: "testidx", Query: "Count(Row(testf=0))"}) if err != nil { t.Fatalf("count querying: %v", err) } - if r.Result().Count() != 1000 { - t.Fatalf("count after import is %d", r.Result().Count()) + if r.Results[0].(uint64) != 1000 { + t.Fatalf("count after import is %d", r.Results[0].(uint64)) } pcmd := exec.Command("/pumba", "pause", "clustertests_pilosa3_1", "--duration", "10s") @@ -60,84 +63,19 @@ func TestClusterStuff(t *testing.T) { if err != nil { t.Fatalf("waiting on pumba pause cmd: %v", err) } - // TODO change the sleep to wait for status to return to NORMAL or timeout once we have Status.State support in go-pilosa + + // TODO change the sleep to wait for status to return to NORMAL - need support in internal client for getting status t.Log("done with pause, waiting for stability") time.Sleep(time.Second * 3) t.Log("done waiting for stability") - r, err = cli.Query(idx.Count(f.Row(0))) + r, err = cli.Query(context.Background(), "testidx", &pilosa.QueryRequest{Index: "testidx", Query: "Count(Row(testf=0))"}) if err != nil { t.Fatalf("count querying: %v", err) - } else if r.Result().Count() != 1000 { - t.Fatalf("count after import is %d", r.Result().Count()) + } + if r.Results[0].(uint64) != 1000 { + t.Fatalf("count after import is %d", r.Results[0].(uint64)) } }) } - -// Utils - -func getPilosaClient(t *testing.T) *pilosa.Client { - cli, err := pilosa.NewClient("pilosa1:10101") - if err != nil { - time.Sleep(time.Millisecond * 40) - } - time.Sleep(time.Second * 2) - // TODO uncomment the following once we get the version of go-pilosa that has the State field on Status. - // start := time.Now() - // for i := 0; true; i++ { - // s, err := cli.Status() - // if i > 800 { - // t.Fatalf("couldn't connect to cluster after %d attempts and %v: state: %s, err: %v", i, time.Since(start), s.State, err) - // } - // if err != nil { - // time.Sleep(time.Millisecond * 40) - // continue - // } - // if s.State == "NORMAL" { - // break - // } else { - // time.Sleep(time.Millisecond * 40) - // } - // } - - return cli -} - -type colIterator struct { - cols []pilosa.Column - i uint -} - -func (c *colIterator) NextRecord() (pilosa.Record, error) { - if int(c.i) >= len(c.cols) { - return nil, io.EOF - } - c.i++ - return c.cols[c.i-1], nil -} - -func TestColIterator(t *testing.T) { - data := make([]pilosa.Column, 3) - for i := range data { - data[i].RowID = 0 - data[i].ColumnID = uint64((i/10)*pi.ShardWidth + i%10) - } - - ci := colIterator{cols: data} - col := pilosa.Column{} - if rec, err := ci.NextRecord(); rec != col { - t.Fatalf("first record wrong: %v, err: %v", rec, err) - } - col.ColumnID = 1 - if rec, err := ci.NextRecord(); rec != col || err != nil { - t.Fatalf("second record wrong: %v, err: %v", rec, err) - } - col.ColumnID = 2 - if rec, err := ci.NextRecord(); rec != col || err != nil { - t.Fatalf("third record wrong: %v, err: %v", rec, err) - } - if rec, err := ci.NextRecord(); err != io.EOF { - t.Fatalf("should be EOF, but got %v, err: %v", rec, err) - } -} From bc8b9912203463cfba46b67f9b5c0ee9d3a154c5 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 13 Nov 2018 09:31:16 -0600 Subject: [PATCH 17/33] rename Dockerfile-withgo to Dockerfile-clustertests --- Dockerfile-withgo => Dockerfile-clustertests | 0 internal/clustertests/docker-compose.yml | 7 ++++--- 2 files changed, 4 insertions(+), 3 deletions(-) rename Dockerfile-withgo => Dockerfile-clustertests (100%) diff --git a/Dockerfile-withgo b/Dockerfile-clustertests similarity index 100% rename from Dockerfile-withgo rename to Dockerfile-clustertests diff --git a/internal/clustertests/docker-compose.yml b/internal/clustertests/docker-compose.yml index 7196ea0eb..8586eb08f 100644 --- a/internal/clustertests/docker-compose.yml +++ b/internal/clustertests/docker-compose.yml @@ -3,7 +3,7 @@ services: pilosa1: build: context: ../.. - dockerfile: Dockerfile-withgo + dockerfile: Dockerfile-clustertests image: ptest ports: - "33455:10101" @@ -17,7 +17,7 @@ services: pilosa2: build: context: ../.. - dockerfile: Dockerfile-withgo + dockerfile: Dockerfile-clustertests image: ptest ports: - "33456:10101" @@ -29,7 +29,8 @@ services: - "/pilosa server --bind pilosa2:10101" pilosa3: build: - context: . + context: ../.. + dockerfile: Dockerfile-clustertests image: ptest ports: - "33457:10101" From 26cd50339309dc9981d2ead1116c0c73c368025f Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 13 Nov 2018 09:51:22 -0600 Subject: [PATCH 18/33] try to run clustertests in CI --- .circleci/config.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index bd1ca86cd..131ee934e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -22,6 +22,7 @@ jobs: - persist_to_workspace: root: . paths: "*" + - setup_remote_docker linter: <<: *defaults steps: @@ -44,6 +45,11 @@ jobs: <<: *base-test environment: GOARCH: 386 + cluster-tests: + <<: *defaults + sets: + - *fast-checkout + - run: make clustertests-build prerelease: <<: *base-test steps: From 08431f6e762bac0b691be2e2f06b1200dd29784f Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 13 Nov 2018 09:53:44 -0600 Subject: [PATCH 19/33] iterate on ci config --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 131ee934e..3e302ecac 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -47,7 +47,7 @@ jobs: GOARCH: 386 cluster-tests: <<: *defaults - sets: + steps: - *fast-checkout - run: make clustertests-build prerelease: From a9d108200b75bb92d73f322332b5de82e6a1493f Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 13 Nov 2018 11:52:08 -0600 Subject: [PATCH 20/33] update circle ci config with cody's feedback --- .circleci/config.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 3e302ecac..605990e5b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -22,7 +22,6 @@ jobs: - persist_to_workspace: root: . paths: "*" - - setup_remote_docker linter: <<: *defaults steps: @@ -49,6 +48,7 @@ jobs: <<: *defaults steps: - *fast-checkout + - setup_remote_docker - run: make clustertests-build prerelease: <<: *base-test @@ -105,6 +105,9 @@ workflows: - test-golang-1.10-386: requires: - build + - cluster-tests: + requires: + - build - prerelease: requires: - linter From 1cd7ebdd2c9889998cdf7fd4e1157b7db9028fa6 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Tue, 13 Nov 2018 12:32:43 -0600 Subject: [PATCH 21/33] Remove TravisCI, add CircleCI shield --- .travis.yml | 52 ------------------------------------------------- CONTRIBUTING.md | 2 +- README.md | 2 +- 3 files changed, 2 insertions(+), 54 deletions(-) delete mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 6c77fe0fc..000000000 --- a/.travis.yml +++ /dev/null @@ -1,52 +0,0 @@ -language: go -go: - - "1.10" # Use string, as 1.10==1.1 if interpreted as float. - - master -env: - global: # AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY - - secure: "VnBFmFfBOrrf7ONLN9WpAFCcV8SEt5G5VPnnHv97TP7PlJG8LWR6k6O+vRJOvf8V4vDMfKCTDonwWLgbssVf3yygo3C8ZoftY2phehEkWGffCgsd9ML/YBNbGq4LYLSE5HKvBqrZjQaOrVby71BAsP8W7RhC6hqzFQ00M/z8dZVfwaQQFwew2eEcSxLEaaDFS8Wgc3/UuwxDRPBq6u3cCN5RxfB+q70HvGVq4TT+0dqS4eCvz688+Z0GIGYx9olNjh0F2Kc8R2Po0lnUNa0GiHrZ21zeQ1DxIK04QABrWWmjL4h+bx3VHNKPFR4GYSKDf+pj1kfaqbfrAg6rMAJdGejgoS+QyjhgCoN4d3qRp8s+1nrxtp0TvezEdjwyxt4quGHbP5TxWUszssbGhWqf4mx6OeJ8MmdTaJjfu0f3NWJXMycqT6J73WKORk4rHeIqF9CIdxdmcpkwYj8rk0TEMTPTsd7WA8w2HIDsCz/jQnRmEgLUiNnTAofYc/uUi/Wg/T2hllkp+oBDTzxk9NTelkqx8TJ0bDmYYL9JWUi1siFHTHiVYTJgyirSfGNpe61u8OLmT0Hak/D399IfL7qgFLlMXk8q92typfO2xEduq6G+8KygeqiOMSsOY+xcDvZf5xtcEihYd21vjtrxRSqFsup/o8DIxEurQnfXBx1B+WA=" - - secure: "U4fpHWDVOG4viqZsiVgUDW7OW1JW60uPOZy0q9pfbs86iHvmZq0PaScsZ+YdlYaN2GETVr7endDf6DCcZs1PWfg0F6VQfkOXcShX8HVS9O58lUZA5tyvbDVql9DQs4PbnkZo+ktz+Z0YaXqq2RdtMDOUz4bgZwspLPMA14if+N6w0tqCFpB7bEtpptTGsdbIQPG1n07yvSeNmK4mvrEEs77tWmhulN5iilpOqhpIvD39bJvtCYVALuJpzLd/OjLTPV9l/fl+hJkMXSj+X5ilO1DHINAcCM648iEX2phXAIWmi0O0Rbg2cI4kV9T5ysOIw8ux+YCm9bZDGTCt+VGBW5Fg+Z5iaXXexyKYCGiHleOJ7kCj9kXxh2u8NiYVNgb19dGJV5/HgQ6pcGWjeVEqr8yY1546zMjpTX+SYGQF+XZe+uggEjeAsk53ueXa0pyZTrlrqSvR7BBtWPx47s/dTg2L19FQYv3XpGMxEXLw92RplExQKi1h7QgihRxFpjGgURHhrt7d9eiNiNqBt3ZsHjmh2AkXZHnaDjlgSnFFWaMqP3UtDBWIuO+2BMbZUJVfP+gpQGBZ4gtpUSmV2JDCHgZgX5OAnLD4usxh+ATQ4rvUXF/tf8nMqEKHlGKd8hxpYSyMX21BoqfSfY4/IA0ejVE9BITqlrvqewqkP1yxe7o=" - matrix: - - GOARCH=386 - - GOARCH=386 ENTERPRISE=1 - - GOARCH=amd64 - - GOARCH=amd64 ENTERPRISE=1 -cache: - directories: - vendor -install: - - make -B install-dep vendor -script: make test -jobs: - include: - - stage: metalinter - install: - - make -B install-dep vendor install-gometalinter - script: make gometalinter - env: - - GOARCH=amd64 - - stage: deploy - script: skip - go: "1.10" - env: - - GOARCH=amd64 - deploy: - - provider: script - script: pip install awscli --user `whoami` && make -B prerelease - skip_cleanup: true - on: - all_branches: true - repo: pilosa/pilosa - tags: false -stages: - - metalinter - - test - - deploy -matrix: - # Excluding or allowing failures on non-primary matrix configurations due to long running times. - fast_finish: true - allow_failures: - - go: master -notifications: - slack: - secure: "SceWannxoGzeSu9PlEhl6icQFGuTmwax870k20nB2ZGYLjo77UEcwYoFwWvFsdYPa/HCo3JorMTYvMJ15VDJcnKEfzDr+kyXbHWBzUumclIOU/Im3ArEN6waQgyGbbWUQhvJjy4ATaxiOlmCyDV+KhKC9P3+WB33/OQtM3ngjAdTXYHAkfEcpeoOP75um+KsQgbi+hlnqfZdgDa6yIkFjaS3KZEJW1vmcOYYzNsXOA1Ip8j1NY6AjjWZlQorZJ/SYFqdhIv8ST3+a6cQk12u3t6TwZdcr3wmm1qmiW/SaK7UesWlT/YfElIuK8BBq9w1oZHxNKoAmLWTOe7MMisdItmtwgA14eMGl1rvNFlVf9sjsxs4AAzFvSZBZdDfx9XeLCBU5I2WUc/PKUgNQBPMVChxA7gEhtZLndsDdye7LsZASD2yYqjlVlgoZpzRexee/cJgCqUcNKDBHF39ZJYxV4KtZ0prjcSnVmLvuapplzTV4LZ+LyFapCyhiuM/oMJvxgmd7jTtFb5e5EkaHBPN1XwQWZw87yCjKsunTlTe1f1a5qoH/xvJHNpqE/jxOHU3DTLDgTxhb+FwC1Qj9a8bp+UYLw5F4P46ZnHlBGc2O74klv17EqvUMn3JhzASUtyxLGOgJulJ+o83rxJvhSiWt3GQIfkExVPzmz11641ElJI=" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2ad9ab13d..3939e927e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -22,7 +22,7 @@ If you want to help but you aren't sure where to start, check out our [github la ### Development Environment -- Ensure you have a recent version of [Go](https://golang.org/doc/install) installed. Pilosa generally supports the current and previous minor versions; check our [travis file](../master/.travis.yml) for the most up-to-date information. +- Ensure you have a recent version of [Go](https://golang.org/doc/install) installed. Pilosa generally supports the current and previous minor versions; check our [CircleCI config file](../master/.circleci/config.yml) for the most up-to-date information. - Make sure `$GOPATH` environment variable points to your Go working directory and `$PATH` incudes `$GOPATH/bin`, as described [here](https://golang.org/doc/code.html#GOPATH). diff --git a/README.md b/README.md index 7c1c7cfda..8ccae2b91 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@

-[![Build Status](https://travis-ci.org/pilosa/pilosa.svg?branch=master)](https://travis-ci.org/pilosa/pilosa) +[![CircleCI](https://circleci.com/gh/pilosa/pilosa/tree/master.svg?style=shield)](https://circleci.com/gh/pilosa/pilosa/tree/master) [![GoDoc](https://godoc.org/github.com/pilosa/pilosa?status.svg)](https://godoc.org/github.com/pilosa/pilosa) [![Go Report Card](https://goreportcard.com/badge/github.com/pilosa/pilosa)](https://goreportcard.com/report/github.com/pilosa/pilosa) [![license](https://img.shields.io/github/license/pilosa/pilosa.svg)](https://github.com/pilosa/pilosa/blob/master/LICENSE) From 70f85211d97118331ba95c06ec3af26f0fb40aff Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Thu, 15 Nov 2018 22:06:21 +0300 Subject: [PATCH 22/33] prevent panic in Bitmap.UnmarshalBinary when there is no data --- api.go | 4 ++++ fragment.go | 4 +--- roaring/roaring.go | 4 ++++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/api.go b/api.go index dcedc3d92..03b3bbcb0 100644 --- a/api.go +++ b/api.go @@ -268,6 +268,10 @@ func setUpImportOptions(opts ...ImportOption) (*ImportOptions, error) { // of the rows in this shard of this field concatenated together in one long // bitmap. func (api *API) ImportRoaring(ctx context.Context, indexName, fieldName string, shard uint64, remote bool, data []byte, opts ...ImportOption) (err error) { + if len(data) == 0 { + return errors.New("no data to import") + } + if err = api.validate(apiField); err != nil { return errors.Wrap(err, "validating api method") } diff --git a/fragment.go b/fragment.go index dd1d874de..359628ab5 100644 --- a/fragment.go +++ b/fragment.go @@ -25,6 +25,7 @@ import ( "hash" "io" "io/ioutil" + "math" "os" "sort" "sync" @@ -33,9 +34,6 @@ import ( "unsafe" "github.com/cespare/xxhash" - - "math" - "github.com/gogo/protobuf/proto" "github.com/pilosa/pilosa/internal" "github.com/pilosa/pilosa/pql" diff --git a/roaring/roaring.go b/roaring/roaring.go index ed7be6be1..ba258526f 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -3465,6 +3465,10 @@ func readOfficialHeader(buf []byte) (size uint32, containerTyper func(index uint // UnmarshalBinary decodes b from a binary-encoded byte slice. data can be in // either official roaring format or Pilosa's roaring format. func (b *Bitmap) UnmarshalBinary(data []byte) error { + if data == nil { + // Nothing to unmarshal + return nil + } fileMagic := uint32(binary.LittleEndian.Uint16(data[0:2])) if fileMagic == magicNumber { // if pilosa roaring return errors.Wrap(b.unmarshalPilosaRoaring(data), "unmarshaling as pilosa roaring") From a203313143de80ea0c350e5a33881f0064298dd2 Mon Sep 17 00:00:00 2001 From: Seebs Date: Wed, 14 Nov 2018 22:44:27 -0600 Subject: [PATCH 23/33] move Logger and Stats to their own packages I'd like to add stat tracking to Roaring, which means it has to be able to import the stats package, which means stats has to be a package rather than part of the pilosa package. If stats stops being in pilosa, it still needs a way to import logger, so logger also has to leave the pilosa package. Then everything using them needs to import them and use package selectors on their names. This doesn't actually add the stats support to roaring, it just makes it so there's a way to import the stats code from something in the roaring package. --- api.go | 3 +- cache.go | 27 +++++++++-------- cluster.go | 9 +++--- diagnostics.go | 5 ++-- field.go | 10 ++++--- fragment.go | 12 ++++---- gossip/gossip.go | 5 ++-- holder.go | 12 ++++---- http/handler.go | 7 +++-- index.go | 10 ++++--- logger.go => logger/logger.go | 6 ++-- server.go | 10 ++++--- server/server.go | 14 +++++---- stats.go => stats/stats.go | 12 ++++---- stats_test.go => stats/stats_test.go | 44 +++++++++++++++------------- statsd/statsd.go | 13 ++++---- translate.go | 7 +++-- view.go | 10 ++++--- 18 files changed, 121 insertions(+), 95 deletions(-) rename logger.go => logger/logger.go (91%) rename stats.go => stats/stats.go (96%) rename stats_test.go => stats/stats_test.go (80%) diff --git a/api.go b/api.go index 03b3bbcb0..7a95ce76e 100644 --- a/api.go +++ b/api.go @@ -28,6 +28,7 @@ import ( "github.com/pilosa/pilosa/pql" "github.com/pilosa/pilosa/roaring" + "github.com/pilosa/pilosa/stats" "github.com/pkg/errors" "golang.org/x/sync/errgroup" ) @@ -930,7 +931,7 @@ func (api *API) AvailableShardsByIndex(_ context.Context) map[string]*roaring.Bi // StatsWithTags returns an instance of whatever implementation of StatsClient // pilosa is using with the given tags. -func (api *API) StatsWithTags(tags []string) StatsClient { +func (api *API) StatsWithTags(tags []string) stats.StatsClient { if api.holder == nil || api.cluster == nil { return nil } diff --git a/cache.go b/cache.go index df82a5802..40509ab64 100644 --- a/cache.go +++ b/cache.go @@ -23,6 +23,7 @@ import ( "time" "github.com/pilosa/pilosa/lru" + "github.com/pilosa/pilosa/stats" ) const ( @@ -50,14 +51,14 @@ type cache interface { Top() []bitmapPair // SetStats defines the stats client used in the cache. - SetStats(s StatsClient) + SetStats(s stats.StatsClient) } // lruCache represents a least recently used Cache implementation. type lruCache struct { cache *lru.Cache counts map[uint64]uint64 - stats StatsClient + stats stats.StatsClient } // newLRUCache returns a new instance of LRUCache. @@ -65,7 +66,7 @@ func newLRUCache(maxEntries uint32) *lruCache { c := &lruCache{ cache: lru.New(int(maxEntries)), counts: make(map[uint64]uint64), - stats: NopStatsClient, + stats: stats.NopStatsClient, } c.cache.OnEvicted = c.onEvicted return c @@ -122,7 +123,7 @@ func (c *lruCache) Top() []bitmapPair { } // SetStats defines the stats client used in the cache. -func (c *lruCache) SetStats(s StatsClient) { +func (c *lruCache) SetStats(s stats.StatsClient) { c.stats = s } @@ -150,7 +151,7 @@ type rankCache struct { // thresholdValue is the value of the last item in the cache thresholdValue uint64 - stats StatsClient + stats stats.StatsClient } // NewRankCache returns a new instance of RankCache. @@ -159,7 +160,7 @@ func NewRankCache(maxEntries uint32) *rankCache { maxEntries: maxEntries, thresholdBuffer: int(thresholdFactor * float64(maxEntries)), entries: make(map[uint64]uint64), - stats: NopStatsClient, + stats: stats.NopStatsClient, } } @@ -279,7 +280,7 @@ func (c *rankCache) recalculate() { } // SetStats defines the stats client used in the cache. -func (c *rankCache) SetStats(s StatsClient) { +func (c *rankCache) SetStats(s stats.StatsClient) { c.stats = s } @@ -458,12 +459,12 @@ func (s *simpleCache) Add(id uint64, b *Row) { // nopCache represents a no-op Cache implementation. type nopCache struct { - stats StatsClient + stats stats.StatsClient } // Ensure NopCache implements Cache. var globalNopCache cache = nopCache{ - stats: NopStatsClient, + stats: stats.NopStatsClient, } func (c nopCache) Add(uint64, uint64) {} @@ -471,10 +472,10 @@ func (c nopCache) BulkAdd(uint64, uint64) {} func (c nopCache) Get(uint64) uint64 { return 0 } func (c nopCache) IDs() []uint64 { return []uint64{} } -func (c nopCache) Invalidate() {} -func (c nopCache) Len() int { return 0 } -func (c nopCache) Recalculate() {} -func (c nopCache) SetStats(StatsClient) {} +func (c nopCache) Invalidate() {} +func (c nopCache) Len() int { return 0 } +func (c nopCache) Recalculate() {} +func (c nopCache) SetStats(stats.StatsClient) {} func (c nopCache) Top() []bitmapPair { return []bitmapPair{} diff --git a/cluster.go b/cluster.go index 4b1ab9089..e22781a25 100644 --- a/cluster.go +++ b/cluster.go @@ -31,6 +31,7 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pilosa/pilosa/internal" + "github.com/pilosa/pilosa/logger" "github.com/pilosa/pilosa/roaring" "github.com/pkg/errors" uuid "github.com/satori/go.uuid" @@ -216,7 +217,7 @@ type cluster struct { // nolint: maligned wg sync.WaitGroup closing chan struct{} - logger Logger + logger logger.Logger InternalClient InternalClient } @@ -235,7 +236,7 @@ func newCluster() *cluster { InternalClient: newNopInternalClient(), - logger: NopLogger, + logger: logger.NopLogger, } } @@ -1379,7 +1380,7 @@ type resizeJob struct { mu sync.RWMutex state string - Logger Logger + Logger logger.Logger } // newResizeJob returns a new instance of resizeJob. @@ -1411,7 +1412,7 @@ func newResizeJob(existingNodes []*Node, node *Node, action string) *resizeJob { IDs: ids, action: action, result: make(chan string), - Logger: NopLogger, + Logger: logger.NopLogger, } } diff --git a/diagnostics.go b/diagnostics.go index 673fb0c7f..5ed97940a 100644 --- a/diagnostics.go +++ b/diagnostics.go @@ -24,6 +24,7 @@ import ( "sync" "time" + "github.com/pilosa/pilosa/logger" "github.com/pkg/errors" ) @@ -51,7 +52,7 @@ type diagnosticsCollector struct { client *http.Client - Logger Logger + Logger logger.Logger server *Server } @@ -65,7 +66,7 @@ func newDiagnosticsCollector(host string) *diagnosticsCollector { // nolint: unp start: time.Now(), client: &http.Client{Timeout: 10 * time.Second}, metrics: make(map[string]interface{}), - Logger: NopLogger, + Logger: logger.NopLogger, } } diff --git a/field.go b/field.go index bc656fa54..4189e55a4 100644 --- a/field.go +++ b/field.go @@ -28,8 +28,10 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pilosa/pilosa/internal" + "github.com/pilosa/pilosa/logger" "github.com/pilosa/pilosa/pql" "github.com/pilosa/pilosa/roaring" + "github.com/pilosa/pilosa/stats" "github.com/pkg/errors" ) @@ -69,7 +71,7 @@ type Field struct { rowAttrStore AttrStore broadcaster broadcaster - Stats StatsClient + Stats stats.StatsClient // Field options. options FieldOptions @@ -79,7 +81,7 @@ type Field struct { // Shards with data on any node in the cluster, according to this node. remoteAvailableShards *roaring.Bitmap - logger Logger + logger logger.Logger } // FieldOption is a functional option type for pilosa.fieldOptions. @@ -196,13 +198,13 @@ func newField(path, index, name string, opts FieldOption) (*Field, error) { rowAttrStore: nopStore, broadcaster: NopBroadcaster, - Stats: NopStatsClient, + Stats: stats.NopStatsClient, options: applyDefaultOptions(fo), remoteAvailableShards: roaring.NewBitmap(), - logger: NopLogger, + logger: logger.NopLogger, } return f, nil } diff --git a/fragment.go b/fragment.go index 359628ab5..805112039 100644 --- a/fragment.go +++ b/fragment.go @@ -36,8 +36,10 @@ import ( "github.com/cespare/xxhash" "github.com/gogo/protobuf/proto" "github.com/pilosa/pilosa/internal" + "github.com/pilosa/pilosa/logger" "github.com/pilosa/pilosa/pql" "github.com/pilosa/pilosa/roaring" + "github.com/pilosa/pilosa/stats" "github.com/pkg/errors" ) @@ -116,7 +118,7 @@ type fragment struct { MaxOpN int // Logger used for out-of-band log entries. - Logger Logger + Logger logger.Logger // Row attribute storage. // This is set by the parent field unless overridden for testing. @@ -126,7 +128,7 @@ type fragment struct { // existing value (to clear) prior to setting a new value. mutexVector vector - stats StatsClient + stats stats.StatsClient } // newFragment returns a new instance of Fragment. @@ -140,10 +142,10 @@ func newFragment(path, index, field, view string, shard uint64) *fragment { CacheType: DefaultCacheType, CacheSize: DefaultCacheSize, - Logger: NopLogger, + Logger: logger.NopLogger, MaxOpN: defaultFragmentMaxOpN, - stats: NopStatsClient, + stats: stats.NopStatsClient, } } @@ -1718,7 +1720,7 @@ func (f *fragment) Snapshot() error { defer f.mu.Unlock() return f.snapshot() } -func track(start time.Time, message string, stats StatsClient, logger Logger) { +func track(start time.Time, message string, stats stats.StatsClient, logger logger.Logger) { elapsed := time.Since(start) logger.Printf("%s took %s", message, elapsed) stats.Histogram("snapshot", elapsed.Seconds(), 1.0) diff --git a/gossip/gossip.go b/gossip/gossip.go index ecd663ecf..8f19203f5 100644 --- a/gossip/gossip.go +++ b/gossip/gossip.go @@ -29,6 +29,7 @@ import ( "github.com/hashicorp/memberlist" "github.com/pilosa/pilosa" + "github.com/pilosa/pilosa/logger" "github.com/pilosa/pilosa/roaring" "github.com/pilosa/pilosa/toml" "github.com/pkg/errors" @@ -47,7 +48,7 @@ type memberSet struct { papi *pilosa.API config *config - Logger pilosa.Logger + Logger logger.Logger logger *log.Logger logOutput io.Writer @@ -170,7 +171,7 @@ func NewMemberSet(cfg Config, api *pilosa.API, options ...memberSetOption) (*mem host := api.Node().URI.Host g := &memberSet{ papi: api, - Logger: pilosa.NopLogger, + Logger: logger.NopLogger, } // options diff --git a/holder.go b/holder.go index 0a1af820b..68643915e 100644 --- a/holder.go +++ b/holder.go @@ -27,7 +27,9 @@ import ( "syscall" "time" + "github.com/pilosa/pilosa/logger" "github.com/pilosa/pilosa/roaring" + "github.com/pilosa/pilosa/stats" "github.com/pkg/errors" uuid "github.com/satori/go.uuid" ) @@ -66,7 +68,7 @@ type Holder struct { closing chan struct{} // Stats - Stats StatsClient + Stats stats.StatsClient // Data directory path. Path string @@ -74,7 +76,7 @@ type Holder struct { // The interval at which the cached row ids are persisted to disk. cacheFlushInterval time.Duration - Logger Logger + Logger logger.Logger } // NewHolder returns a new instance of Holder. @@ -89,13 +91,13 @@ func NewHolder() *Holder { NewPrimaryTranslateStore: newNopTranslateStore, broadcaster: NopBroadcaster, - Stats: NopStatsClient, + Stats: stats.NopStatsClient, NewAttrStore: newNopAttrStore, cacheFlushInterval: defaultCacheFlushInterval, - Logger: NopLogger, + Logger: logger.NopLogger, } } @@ -605,7 +607,7 @@ type holderSyncer struct { Cluster *cluster // Stats - Stats StatsClient + Stats stats.StatsClient // Signals that the sync should stop. Closing <-chan struct{} diff --git a/http/handler.go b/http/handler.go index 81e31a82b..1ecbd2fb1 100644 --- a/http/handler.go +++ b/http/handler.go @@ -36,6 +36,7 @@ import ( "github.com/gorilla/handlers" "github.com/gorilla/mux" "github.com/pilosa/pilosa" + "github.com/pilosa/pilosa/logger" "github.com/pkg/errors" ) @@ -44,7 +45,7 @@ import ( type Handler struct { Handler http.Handler - logger pilosa.Logger + logger logger.Logger // Keeps the query argument validators for each handler validators map[string]*queryValidationSpec @@ -95,7 +96,7 @@ func OptHandlerAPI(api *pilosa.API) handlerOption { } } -func OptHandlerLogger(logger pilosa.Logger) handlerOption { +func OptHandlerLogger(logger logger.Logger) handlerOption { return func(h *Handler) error { h.logger = logger return nil @@ -121,7 +122,7 @@ func OptHandlerCloseTimeout(d time.Duration) handlerOption { // NewHandler returns a new instance of Handler with a default logger. func NewHandler(opts ...handlerOption) (*Handler, error) { handler := &Handler{ - logger: pilosa.NopLogger, + logger: logger.NopLogger, closeTimeout: time.Second * 30, } handler.Handler = newRouter(handler) diff --git a/index.go b/index.go index 297c02f20..e06ce1732 100644 --- a/index.go +++ b/index.go @@ -25,7 +25,9 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pilosa/pilosa/internal" + "github.com/pilosa/pilosa/logger" "github.com/pilosa/pilosa/roaring" + "github.com/pilosa/pilosa/stats" "github.com/pkg/errors" ) @@ -49,9 +51,9 @@ type Index struct { columnAttrs AttrStore broadcaster broadcaster - Stats StatsClient + Stats stats.StatsClient - logger Logger + logger logger.Logger } // NewIndex returns a new instance of Index. @@ -70,8 +72,8 @@ func NewIndex(path, name string) (*Index, error) { columnAttrs: nopStore, broadcaster: NopBroadcaster, - Stats: NopStatsClient, - logger: NopLogger, + Stats: stats.NopStatsClient, + logger: logger.NopLogger, trackExistence: true, }, nil } diff --git a/logger.go b/logger/logger.go similarity index 91% rename from logger.go rename to logger/logger.go index 074da8a36..ed5a2dc26 100644 --- a/logger.go +++ b/logger/logger.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package pilosa +package logger import ( "io" @@ -39,7 +39,7 @@ func (n *nopLogger) Printf(format string, v ...interface{}) {} // Debugf is a no-op implementation of the Logger Debugf method. func (n *nopLogger) Debugf(format string, v ...interface{}) {} -// standardLogger is a basic implementation of pilosa.Logger based on log.Logger. +// standardLogger is a basic implementation of Logger based on log.Logger. type standardLogger struct { logger *log.Logger } @@ -60,7 +60,7 @@ func (s *standardLogger) Logger() *log.Logger { return s.logger } -// verboseLogger is an implementation of pilosa.Logger which includes debug messages. +// verboseLogger is an implementation of Logger which includes debug messages. type verboseLogger struct { logger *log.Logger } diff --git a/server.go b/server.go index 5e6f61087..24385c4ea 100644 --- a/server.go +++ b/server.go @@ -27,7 +27,9 @@ import ( "sync" "time" + "github.com/pilosa/pilosa/logger" "github.com/pilosa/pilosa/roaring" + "github.com/pilosa/pilosa/stats" "github.com/pkg/errors" "golang.org/x/sync/errgroup" ) @@ -58,7 +60,7 @@ type Server struct { // nolint: maligned // External systemInfo SystemInfo gcNotifier GCNotifier - logger Logger + logger logger.Logger nodeID string uri URI @@ -81,7 +83,7 @@ func (s *Server) Holder() *Holder { // ServerOption is a functional option type for pilosa.Server type ServerOption func(s *Server) error -func OptServerLogger(l Logger) ServerOption { +func OptServerLogger(l logger.Logger) ServerOption { return func(s *Server) error { s.logger = l return nil @@ -176,7 +178,7 @@ func OptServerPrimaryTranslateStoreFunc(tf func(interface{}) TranslateStore) Ser } } -func OptServerStatsClient(sc StatsClient) ServerOption { +func OptServerStatsClient(sc stats.StatsClient) ServerOption { return func(s *Server) error { s.holder.Stats = sc return nil @@ -258,7 +260,7 @@ func NewServer(opts ...ServerOption) (*Server, error) { metricInterval: 0, diagnosticInterval: 0, - logger: NopLogger, + logger: logger.NopLogger, } s.executor = newExecutor(optExecutorInternalQueryClient(s.defaultClient)) s.cluster.InternalClient = s.defaultClient diff --git a/server/server.go b/server/server.go index 6cdc43883..1e140d1f0 100644 --- a/server/server.go +++ b/server/server.go @@ -41,12 +41,14 @@ import ( "github.com/pilosa/pilosa/gopsutil" "github.com/pilosa/pilosa/gossip" "github.com/pilosa/pilosa/http" + "github.com/pilosa/pilosa/logger" + "github.com/pilosa/pilosa/stats" "github.com/pilosa/pilosa/statsd" "github.com/pkg/errors" ) type loggerLogger interface { - pilosa.Logger + logger.Logger Logger() *log.Logger } @@ -185,9 +187,9 @@ func (m *Command) setupLogger() error { } if m.Config.Verbose { - m.logger = pilosa.NewVerboseLogger(m.logOutput) + m.logger = logger.NewVerboseLogger(m.logOutput) } else { - m.logger = pilosa.NewStandardLogger(m.logOutput) + m.logger = logger.NewStandardLogger(m.logOutput) } return nil } @@ -375,14 +377,14 @@ func (m *Command) Close() error { } // newStatsClient creates a stats client from the config -func newStatsClient(name string, host string) (pilosa.StatsClient, error) { +func newStatsClient(name string, host string) (stats.StatsClient, error) { switch name { case "expvar": - return pilosa.NewExpvarStatsClient(), nil + return stats.NewExpvarStatsClient(), nil case "statsd": return statsd.NewStatsClient(host) case "nop", "none": - return pilosa.NopStatsClient, nil + return stats.NopStatsClient, nil default: return nil, errors.Errorf("'%v' not a valid stats client, choose from [expvar, statsd, none].", name) } diff --git a/stats.go b/stats/stats.go similarity index 96% rename from stats.go rename to stats/stats.go index 8f23c77aa..169df0d6d 100644 --- a/stats.go +++ b/stats/stats.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package pilosa +package stats import ( "expvar" @@ -20,6 +20,8 @@ import ( "strings" "sync" "time" + + "github.com/pilosa/pilosa/logger" ) // Expvar global expvar map. @@ -52,7 +54,7 @@ type StatsClient interface { Timing(name string, value time.Duration, rate float64) // SetLogger Set the logger output type - SetLogger(logger Logger) + SetLogger(logger logger.Logger) // Starts the service Open() @@ -74,7 +76,7 @@ func (c *nopStatsClient) Gauge(name string, value float64, rate float64) func (c *nopStatsClient) Histogram(name string, value float64, rate float64) {} func (c *nopStatsClient) Set(name string, value string, rate float64) {} func (c *nopStatsClient) Timing(name string, value time.Duration, rate float64) {} -func (c *nopStatsClient) SetLogger(logger Logger) {} +func (c *nopStatsClient) SetLogger(logger logger.Logger) {} func (c *nopStatsClient) Open() {} func (c *nopStatsClient) Close() error { return nil } @@ -149,7 +151,7 @@ func (c *expvarStatsClient) Timing(name string, value time.Duration, rate float6 } // SetLogger has no logger. -func (c *expvarStatsClient) SetLogger(logger Logger) { +func (c *expvarStatsClient) SetLogger(logger logger.Logger) { } // Open no-op. @@ -221,7 +223,7 @@ func (a MultiStatsClient) Timing(name string, value time.Duration, rate float64) } // SetLogger Sets the StatsD logger output type. -func (a MultiStatsClient) SetLogger(logger Logger) { +func (a MultiStatsClient) SetLogger(logger logger.Logger) { for _, c := range a { c.SetLogger(logger) } diff --git a/stats_test.go b/stats/stats_test.go similarity index 80% rename from stats_test.go rename to stats/stats_test.go index 067cfa991..3da83ce70 100644 --- a/stats_test.go +++ b/stats/stats_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package pilosa_test +package stats_test import ( "context" @@ -23,6 +23,8 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/http" + "github.com/pilosa/pilosa/logger" + "github.com/pilosa/pilosa/stats" "github.com/pilosa/pilosa/test" ) @@ -32,51 +34,51 @@ func TestMultiStatClient_Expvar(t *testing.T) { hldr := test.MustOpenHolder() defer hldr.Close() - c := pilosa.NewExpvarStatsClient() - ms := make(pilosa.MultiStatsClient, 1) + c := stats.NewExpvarStatsClient() + ms := make(stats.MultiStatsClient, 1) ms[0] = c hldr.Stats = ms hldr.SetBit("d", "f", 0, 0) hldr.SetBit("d", "f", 0, 1) - hldr.SetBit("d", "f", 0, ShardWidth) - hldr.SetBit("d", "f", 0, ShardWidth+2) + hldr.SetBit("d", "f", 0, pilosa.ShardWidth) + hldr.SetBit("d", "f", 0, pilosa.ShardWidth+2) hldr.ClearBit("d", "f", 0, 1) - if pilosa.Expvar.String() != `{"index:d": {"field:f": {"view:standard": {"shard:0": {"clearBit": 1, "rows": 0, "setBit": 2}, "shard:1": {"rows": 0, "setBit": 2}}}}}` { - t.Fatalf("unexpected expvar : %s", pilosa.Expvar.String()) + if stats.Expvar.String() != `{"index:d": {"field:f": {"view:standard": {"shard:0": {"clearBit": 1, "rows": 0, "setBit": 2}, "shard:1": {"rows": 0, "setBit": 2}}}}}` { + t.Fatalf("unexpected expvar : %s", stats.Expvar.String()) } hldr.Stats.CountWithCustomTags("cc", 1, 1.0, []string{"foo:bar"}) - if pilosa.Expvar.String() != `{"cc": 1, "index:d": {"field:f": {"view:standard": {"shard:0": {"clearBit": 1, "rows": 0, "setBit": 2}, "shard:1": {"rows": 0, "setBit": 2}}}}}` { - t.Fatalf("unexpected expvar : %s", pilosa.Expvar.String()) + if stats.Expvar.String() != `{"cc": 1, "index:d": {"field:f": {"view:standard": {"shard:0": {"clearBit": 1, "rows": 0, "setBit": 2}, "shard:1": {"rows": 0, "setBit": 2}}}}}` { + t.Fatalf("unexpected expvar : %s", stats.Expvar.String()) } // Gauge creates a unique key, subsequent Gauge calls will overwrite hldr.Stats.Gauge("g", 5, 1.0) hldr.Stats.Gauge("g", 8, 1.0) - if pilosa.Expvar.String() != `{"cc": 1, "g": 8, "index:d": {"field:f": {"view:standard": {"shard:0": {"clearBit": 1, "rows": 0, "setBit": 2}, "shard:1": {"rows": 0, "setBit": 2}}}}}` { - t.Fatalf("unexpected expvar : %s", pilosa.Expvar.String()) + if stats.Expvar.String() != `{"cc": 1, "g": 8, "index:d": {"field:f": {"view:standard": {"shard:0": {"clearBit": 1, "rows": 0, "setBit": 2}, "shard:1": {"rows": 0, "setBit": 2}}}}}` { + t.Fatalf("unexpected expvar : %s", stats.Expvar.String()) } // Set creates a unique key, subsequent sets will overwrite hldr.Stats.Set("s", "4", 1.0) hldr.Stats.Set("s", "7", 1.0) - if pilosa.Expvar.String() != `{"cc": 1, "g": 8, "index:d": {"field:f": {"view:standard": {"shard:0": {"clearBit": 1, "rows": 0, "setBit": 2}, "shard:1": {"rows": 0, "setBit": 2}}}}, "s": "7"}` { - t.Fatalf("unexpected expvar : %s", pilosa.Expvar.String()) + if stats.Expvar.String() != `{"cc": 1, "g": 8, "index:d": {"field:f": {"view:standard": {"shard:0": {"clearBit": 1, "rows": 0, "setBit": 2}, "shard:1": {"rows": 0, "setBit": 2}}}}, "s": "7"}` { + t.Fatalf("unexpected expvar : %s", stats.Expvar.String()) } // Record timing duration and a uniquely Set key/value dur, _ := time.ParseDuration("123us") hldr.Stats.Timing("tt", dur, 1.0) - if pilosa.Expvar.String() != `{"cc": 1, "g": 8, "index:d": {"field:f": {"view:standard": {"shard:0": {"clearBit": 1, "rows": 0, "setBit": 2}, "shard:1": {"rows": 0, "setBit": 2}}}}, "s": "7", "tt": 123µs}` { - t.Fatalf("unexpected expvar : %s", pilosa.Expvar.String()) + if stats.Expvar.String() != `{"cc": 1, "g": 8, "index:d": {"field:f": {"view:standard": {"shard:0": {"clearBit": 1, "rows": 0, "setBit": 2}, "shard:1": {"rows": 0, "setBit": 2}}}}, "s": "7", "tt": 123µs}` { + t.Fatalf("unexpected expvar : %s", stats.Expvar.String()) } // Expvar histogram is implemented as a gauge hldr.Stats.Histogram("hh", 3, 1.0) - if pilosa.Expvar.String() != `{"cc": 1, "g": 8, "hh": 3, "index:d": {"field:f": {"view:standard": {"shard:0": {"clearBit": 1, "rows": 0, "setBit": 2}, "shard:1": {"rows": 0, "setBit": 2}}}}, "s": "7", "tt": 123µs}` { - t.Fatalf("unexpected expvar : %s", pilosa.Expvar.String()) + if stats.Expvar.String() != `{"cc": 1, "g": 8, "hh": 3, "index:d": {"field:f": {"view:standard": {"shard:0": {"clearBit": 1, "rows": 0, "setBit": 2}, "shard:1": {"rows": 0, "setBit": 2}}}}, "s": "7", "tt": 123µs}` { + t.Fatalf("unexpected expvar : %s", stats.Expvar.String()) } // Expvar should ignore earlier set tags from setbit @@ -92,8 +94,8 @@ func TestStatsCount_TopN(t *testing.T) { hldr.SetBit("d", "f", 0, 0) hldr.SetBit("d", "f", 0, 1) - hldr.SetBit("d", "f", 0, ShardWidth) - hldr.SetBit("d", "f", 0, ShardWidth+2) + hldr.SetBit("d", "f", 0, pilosa.ShardWidth) + hldr.SetBit("d", "f", 0, pilosa.ShardWidth+2) // Execute query. called := false @@ -311,11 +313,11 @@ func (s *MockStats) CountWithCustomTags(name string, value int64, rate float64, } func (c *MockStats) Tags() []string { return nil } -func (c *MockStats) WithTags(tags ...string) pilosa.StatsClient { return c } +func (c *MockStats) WithTags(tags ...string) stats.StatsClient { return c } func (c *MockStats) Gauge(name string, value float64, rate float64) {} func (c *MockStats) Histogram(name string, value float64, rate float64) {} func (c *MockStats) Set(name string, value string, rate float64) {} func (c *MockStats) Timing(name string, value time.Duration, rate float64) {} -func (c *MockStats) SetLogger(logger pilosa.Logger) {} +func (c *MockStats) SetLogger(logger logger.Logger) {} func (c *MockStats) Open() {} func (c *MockStats) Close() error { return nil } diff --git a/statsd/statsd.go b/statsd/statsd.go index 7a9ae6c14..eaf8facf1 100644 --- a/statsd/statsd.go +++ b/statsd/statsd.go @@ -19,7 +19,8 @@ import ( "time" "github.com/DataDog/datadog-go/statsd" - "github.com/pilosa/pilosa" + "github.com/pilosa/pilosa/logger" + "github.com/pilosa/pilosa/stats" ) // StatsD protocol wrapper using the DataDog library that added Tags to the StatsD protocol @@ -34,13 +35,13 @@ const ( ) // Ensure client implements interface. -var _ pilosa.StatsClient = &statsClient{} +var _ stats.StatsClient = &statsClient{} // statsClient represents a StatsD implementation of pilosa.statsClient. type statsClient struct { client *statsd.Client tags []string - logger pilosa.Logger + logger logger.Logger } // NewStatsClient returns a new instance of StatsClient. @@ -52,7 +53,7 @@ func NewStatsClient(host string) (*statsClient, error) { return &statsClient{ client: c, - logger: pilosa.NopLogger, + logger: logger.NopLogger, }, nil } @@ -70,7 +71,7 @@ func (c *statsClient) Tags() []string { } // WithTags returns a new client with additional tags appended. -func (c *statsClient) WithTags(tags ...string) pilosa.StatsClient { +func (c *statsClient) WithTags(tags ...string) stats.StatsClient { return &statsClient{ client: c.client, tags: unionStringSlice(c.tags, tags), @@ -122,7 +123,7 @@ func (c *statsClient) Timing(name string, value time.Duration, rate float64) { } // SetLogger sets the logger for client. -func (c *statsClient) SetLogger(logger pilosa.Logger) { +func (c *statsClient) SetLogger(logger logger.Logger) { c.logger = logger } diff --git a/translate.go b/translate.go index 86ed6c914..669e1e323 100644 --- a/translate.go +++ b/translate.go @@ -15,6 +15,7 @@ import ( "time" "github.com/cespare/xxhash" + "github.com/pilosa/pilosa/logger" "github.com/pkg/errors" ) @@ -68,7 +69,7 @@ type TranslateFile struct { Path string mapSize int - logger Logger + logger logger.Logger // If non-nil, data is streamed from a primary and this is a read-only store. PrimaryTranslateStore TranslateStore primaryID string // unique ID used to identify the primary store @@ -89,7 +90,7 @@ func OptTranslateFileMapSize(mapSize int) TranslateFileOption { return nil } } -func OptTranslateFileLogger(l Logger) TranslateFileOption { +func OptTranslateFileLogger(l logger.Logger) TranslateFileOption { return func(s *TranslateFile) error { s.logger = l return nil @@ -116,7 +117,7 @@ func NewTranslateFile(opts ...TranslateFileOption) *TranslateFile { mapSize: defaultMapSize, - logger: NopLogger, + logger: logger.NopLogger, replicationClosing: make(chan struct{}), primaryStoreEvents: make(chan primaryStoreEvent), diff --git a/view.go b/view.go index a0abb0e9c..128e3b828 100644 --- a/view.go +++ b/view.go @@ -22,8 +22,10 @@ import ( "strings" "sync" + "github.com/pilosa/pilosa/logger" "github.com/pilosa/pilosa/pql" "github.com/pilosa/pilosa/roaring" + "github.com/pilosa/pilosa/stats" "github.com/pkg/errors" ) @@ -50,9 +52,9 @@ type view struct { fragments map[uint64]*fragment broadcaster broadcaster - stats StatsClient + stats stats.StatsClient rowAttrStore AttrStore - logger Logger + logger logger.Logger } // newView returns a new instance of View. @@ -70,8 +72,8 @@ func newView(path, index, field, name string, fieldOptions FieldOptions) *view { fragments: make(map[uint64]*fragment), broadcaster: NopBroadcaster, - stats: NopStatsClient, - logger: NopLogger, + stats: stats.NopStatsClient, + logger: logger.NopLogger, } } From 33add4f1e000343b4909c333350037ededdddd19 Mon Sep 17 00:00:00 2001 From: Seebs Date: Wed, 14 Nov 2018 23:01:48 -0600 Subject: [PATCH 24/33] proof of concept for stats This commit adds some trivial stat-tracking which can be observed at localhost:10101/debug/vars. However, writes to a locking data structure aren't cheap, so the stat-tracking is by default not compiled. To build it, add the build tag `roaringstats`, which will cause the `statsHit` function to actually do something. Otherwise, it's an empty and inlineable function, meaning the compiler throws it away entirely. This would, in principle, let us get additional visibility into edge cases and which code paths are hot. This is not the same thing as profiling for overall performance; the stat counts aren't affected by whether a particular code path is using a large amount of CPU time, just reporting how often it happens at all. --- roaring/containers.go | 2 + roaring/roaring.go | 71 ++++++++++++++++++++++++++++++++++++ roaring/roaring_nop_stats.go | 8 ++++ roaring/roaring_stats.go | 15 ++++++++ 4 files changed, 96 insertions(+) create mode 100644 roaring/roaring_nop_stats.go create mode 100644 roaring/roaring_stats.go diff --git a/roaring/containers.go b/roaring/containers.go index ed745a915..3fe0814cc 100644 --- a/roaring/containers.go +++ b/roaring/containers.go @@ -64,6 +64,7 @@ func (sc *sliceContainers) PutContainerValues(key uint64, containerType byte, n } func (sc *sliceContainers) Remove(key uint64) { + statsHit("sliceContainers/Remove") i := search64(sc.keys, key) if i < 0 { return @@ -73,6 +74,7 @@ func (sc *sliceContainers) Remove(key uint64) { } func (sc *sliceContainers) insertAt(key uint64, c *Container, i int) { + statsHit("sliceContainers/insertAt") sc.keys = append(sc.keys, 0) copy(sc.keys[i+1:], sc.keys[i:]) sc.keys[i] = key diff --git a/roaring/roaring.go b/roaring/roaring.go index ba258526f..3a4050cea 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1027,6 +1027,7 @@ func (iv interval16) runlen() int32 { // newContainer returns a new instance of container. func NewContainer() *Container { + statsHit("NewContainer") return &Container{containerType: containerArray} } @@ -1194,6 +1195,7 @@ func (c *Container) add(v uint16) (added bool) { func (c *Container) arrayAdd(v uint16) bool { // Optimize appending to the end of an array container. if c.n > 0 && c.n < ArrayMaxSize && c.isArray() && c.array[c.n-1] < v { + statsHit("arrayAdd/append") c.unmap() c.array = append(c.array, v) return true @@ -1207,11 +1209,13 @@ func (c *Container) arrayAdd(v uint16) bool { // Convert to a bitmap container if too many values are in an array container. if c.n >= ArrayMaxSize { + statsHit("arrayAdd/arrayToBitmap") c.arrayToBitmap() return c.bitmapAdd(v) } // Otherwise insert into array. + statsHit("arrayAdd/insert") c.unmap() i = -i - 1 c.array = append(c.array, 0) @@ -1325,6 +1329,7 @@ func (c *Container) countRuns() (r int32) { // amount of space. func (c *Container) optimize() { if c.n == 0 { + statsHit("optimize/empty") return } runs := c.countRuns() @@ -1341,21 +1346,33 @@ func (c *Container) optimize() { // Then convert accordingly. if c.isArray() { if newType == containerBitmap { + statsHit("optimize/arrayToBitmap") c.arrayToBitmap() } else if newType == containerRun { + statsHit("optimize/arrayToRun") c.arrayToRun() + } else { + statsHit("optimize/arrayUnchanged") } } else if c.isBitmap() { if newType == containerArray { + statsHit("optimize/bitmapToArray") c.bitmapToArray() } else if newType == containerRun { + statsHit("optimize/bitmapToRun") c.bitmapToRun() + } else { + statsHit("optimize/bitmapUnchanged") } } else if c.isRun() { if newType == containerBitmap { + statsHit("optimize/runToBitmap") c.runToBitmap() } else if newType == containerArray { + statsHit("optimize/runToArray") c.runToArray() + } else { + statsHit("optimize/runUnchanged") } } } @@ -1425,6 +1442,7 @@ func (c *Container) bitmapRemove(v uint16) bool { // Convert to array if we go below the threshold. if c.n == ArrayMaxSize { + statsHit("bitmapRemove/bitmapToArray") c.bitmapToArray() } return true @@ -1492,6 +1510,7 @@ func (c *Container) runMax() uint16 { // bitmapToArray converts from bitmap format to array format. func (c *Container) bitmapToArray() { + statsHit("bitmapToArray") c.array = make([]uint16, 0, c.n) c.containerType = containerArray @@ -1515,6 +1534,7 @@ func (c *Container) bitmapToArray() { // arrayToBitmap converts from array format to bitmap format. func (c *Container) arrayToBitmap() { + statsHit("arrayToBitmap") c.bitmap = make([]uint64, bitmapN) c.containerType = containerBitmap @@ -1534,6 +1554,7 @@ func (c *Container) arrayToBitmap() { // runToBitmap converts from RLE format to bitmap format. func (c *Container) runToBitmap() { + statsHit("runToBitmap") c.bitmap = make([]uint64, bitmapN) c.containerType = containerBitmap @@ -1557,6 +1578,7 @@ func (c *Container) runToBitmap() { // bitmapToRun converts from bitmap format to RLE format. func (c *Container) bitmapToRun() { + statsHit("bitmapToRun") c.containerType = containerRun // return early if empty if c.n == 0 { @@ -1613,6 +1635,7 @@ func (c *Container) bitmapToRun() { // arrayToRun converts from array format to RLE format. func (c *Container) arrayToRun() { + statsHit("arrayToRun") c.containerType = containerRun // return early if empty if c.n == 0 { @@ -1640,6 +1663,7 @@ func (c *Container) arrayToRun() { // runToArray converts from RLE format to array format. func (c *Container) runToArray() { + statsHit("runToArray") c.containerType = containerArray c.array = make([]uint16, 0, c.n) @@ -1661,16 +1685,20 @@ func (c *Container) runToArray() { // Clone returns a copy of c. func (c *Container) Clone() *Container { + statsHit("Container/Clone") other := &Container{n: c.n, containerType: c.containerType} switch c.containerType { case containerArray: + statsHit("Container/Clone/Array") other.array = make([]uint16, len(c.array)) copy(other.array, c.array) case containerBitmap: + statsHit("Container/Clone/Bitmap") other.bitmap = make([]uint64, len(c.bitmap)) copy(other.bitmap, c.bitmap) case containerRun: + statsHit("Container/Clone/Run") other.runs = make([]interval16, len(c.runs)) copy(other.runs, c.runs) } @@ -1689,6 +1717,7 @@ func (c *Container) WriteTo(w io.Writer) (n int64, err error) { } func (c *Container) arrayWriteTo(w io.Writer) (n int64, err error) { + statsHit("Container/arrayWriteTo") if len(c.array) == 0 { return 0, nil } @@ -1705,12 +1734,14 @@ func (c *Container) arrayWriteTo(w io.Writer) (n int64, err error) { } func (c *Container) bitmapWriteTo(w io.Writer) (n int64, err error) { + statsHit("Container/bitmapWriteTo") // Write sizeof(uint64) * bitmapN bytes. nn, err := w.Write((*[0xFFFFFFF]byte)(unsafe.Pointer(&c.bitmap[0]))[:(8 * bitmapN)]) return int64(nn), err } func (c *Container) runWriteTo(w io.Writer) (n int64, err error) { + statsHit("Container/runWriteTo") if len(c.runs) == 0 { return 0, nil } @@ -1815,6 +1846,7 @@ func flip(a *Container) *Container { // nolint: deadcode } func flipArray(b *Container) *Container { + statsHit("flipArray") // TODO: actually implement this x := b.Clone() x.arrayToBitmap() @@ -1822,6 +1854,7 @@ func flipArray(b *Container) *Container { } func flipBitmap(b *Container) *Container { + statsHit("flipBitmap") other := &Container{bitmap: make([]uint64, bitmapN), containerType: containerBitmap} for i, bitmap := range b.bitmap { @@ -1833,6 +1866,7 @@ func flipBitmap(b *Container) *Container { } func flipRun(b *Container) *Container { + statsHit("flipRun") // TODO: actually implement this x := b.Clone() x.runToBitmap() @@ -1868,6 +1902,7 @@ func intersectionCount(a, b *Container) int32 { } func intersectionCountArrayArray(a, b *Container) (n int32) { + statsHit("intersectionCount/ArrayArray") na, nb := len(a.array), len(b.array) for i, j := 0, 0; i < na && j < nb; { va, vb := a.array[i], b.array[j] @@ -1884,6 +1919,7 @@ func intersectionCountArrayArray(a, b *Container) (n int32) { } func intersectionCountArrayRun(a, b *Container) (n int32) { + statsHit("intersectionCount/ArrayRun") na, nb := len(a.array), len(b.runs) for i, j := 0, 0; i < na && j < nb; { va, vb := a.array[i], b.runs[j] @@ -1900,6 +1936,7 @@ func intersectionCountArrayRun(a, b *Container) (n int32) { } func intersectionCountRunRun(a, b *Container) (n int32) { + statsHit("intersectionCount/RunRun") na, nb := len(a.runs), len(b.runs) for i, j := 0, 0; i < na && j < nb; { va, vb := a.runs[i], b.runs[j] @@ -1931,6 +1968,7 @@ func intersectionCountRunRun(a, b *Container) (n int32) { } func intersectionCountBitmapRun(a, b *Container) (n int32) { + statsHit("intersectionCount/BitmapRun") for _, iv := range b.runs { n += a.bitmapCountRange(int32(iv.start), int32(iv.last)+1) } @@ -1938,6 +1976,7 @@ func intersectionCountBitmapRun(a, b *Container) (n int32) { } func intersectionCountArrayBitmap(a, b *Container) (n int32) { + statsHit("intersectionCount/ArrayBitmap") ln := len(b.bitmap) for _, val := range a.array { i := int(val >> 6) @@ -1951,6 +1990,7 @@ func intersectionCountArrayBitmap(a, b *Container) (n int32) { } func intersectionCountBitmapBitmap(a, b *Container) (n int32) { + statsHit("intersectionCount/BitmapBitmap") return int32(popcountAndSlice(a.bitmap, b.bitmap)) } @@ -1983,6 +2023,7 @@ func intersect(a, b *Container) *Container { } func intersectArrayArray(a, b *Container) *Container { + statsHit("intersect/ArrayArray") output := &Container{containerType: containerArray} na, nb := len(a.array), len(b.array) for i, j := 0, 0; i < na && j < nb; { @@ -2004,6 +2045,7 @@ func intersectArrayArray(a, b *Container) *Container { // container. The return is always an array container (since it's guaranteed to // be low-cardinality) func intersectArrayRun(a, b *Container) *Container { + statsHit("intersect/ArrayRun") output := &Container{containerType: containerArray} na, nb := len(a.array), len(b.runs) for i, j := 0, 0; i < na && j < nb; { @@ -2023,6 +2065,7 @@ func intersectArrayRun(a, b *Container) *Container { // intersectRunRun computes the intersect of two run containers. func intersectRunRun(a, b *Container) *Container { + statsHit("intersect/RunRun") output := &Container{containerType: containerRun} na, nb := len(a.runs), len(b.runs) for i, j := 0, 0; i < na && j < nb; { @@ -2062,6 +2105,7 @@ func intersectRunRun(a, b *Container) *Container { // intersectBitmapRun returns an array container if the run container's // cardinality is < ArrayMaxSize. Otherwise it returns a bitmap container. func intersectBitmapRun(a, b *Container) *Container { + statsHit("intersect/BitmapRun") var output *Container if b.n < ArrayMaxSize { // output is array container @@ -2125,6 +2169,7 @@ func intersectBitmapRun(a, b *Container) *Container { } func intersectArrayBitmap(a, b *Container) *Container { + statsHit("intersect/ArrayBitmap") output := &Container{containerType: containerArray} for _, va := range a.array { bmidx := va / 64 @@ -2140,6 +2185,7 @@ func intersectArrayBitmap(a, b *Container) *Container { } func intersectBitmapBitmap(a, b *Container) *Container { + statsHit("intersect/BitmapBitmap") // local variables added to prevent BCE checks in loop // see https://go101.org/article/bounds-check-elimination.html var ( @@ -2191,6 +2237,7 @@ func union(a, b *Container) *Container { } func unionArrayArray(a, b *Container) *Container { + statsHit("union/ArrayArray") output := &Container{containerType: containerArray} na, nb := len(a.array), len(b.array) for i, j := 0, 0; ; { @@ -2224,6 +2271,7 @@ func unionArrayArray(a, b *Container) *Container { // unionArrayRun optimistically assumes that the result will be a run container, // and converts to a bitmap or array container afterwards if necessary. func unionArrayRun(a, b *Container) *Container { + statsHit("union/ArrayRun") if b.n == maxContainerVal+1 { return b.Clone() } @@ -2281,6 +2329,7 @@ func (c *Container) runAppendInterval(v interval16) int32 { } func unionRunRun(a, b *Container) *Container { + statsHit("union/RunRun") if a.n == maxContainerVal+1 { return a.Clone() } @@ -2315,6 +2364,7 @@ func unionRunRun(a, b *Container) *Container { } func unionBitmapRun(a, b *Container) *Container { + statsHit("union/BitmapRun") if b.n == maxContainerVal+1 { return b.Clone() } @@ -2503,6 +2553,7 @@ func difference(a, b *Container) *Container { // differenceArrayArray computes the difference bween two arrays. func differenceArrayArray(a, b *Container) *Container { + statsHit("difference/ArrayArray") output := &Container{containerType: containerArray} na, nb := len(a.array), len(b.array) for i, j := 0, 0; i < na; { @@ -2528,6 +2579,7 @@ func differenceArrayArray(a, b *Container) *Container { // differenceArrayRun computes the difference of an array from a run. func differenceArrayRun(a, b *Container) *Container { + statsHit("difference/ArrayRun") // func (ac *arrayContainer) iandNotRun16(rc *runContainer16) container { if a.n == 0 || b.n == 0 { @@ -2585,6 +2637,7 @@ func differenceArrayRun(a, b *Container) *Container { // differenceBitmapRun computes the difference of an bitmap from a run. func differenceBitmapRun(a, b *Container) *Container { + statsHit("difference/BitmapRun") if a.n == 0 || b.n == 0 { return a.Clone() } @@ -2599,6 +2652,7 @@ func differenceBitmapRun(a, b *Container) *Container { // differenceRunArray subtracts the bits in an array container from a run // container. func differenceRunArray(a, b *Container) *Container { + statsHit("difference/RunArray") if a.n == 0 || b.n == 0 { return a.Clone() } @@ -2654,6 +2708,7 @@ RUNLOOP: // differenceRunBitmap computes the difference of an run from a bitmap. func differenceRunBitmap(a, b *Container) *Container { + statsHit("difference/RunBitmap") // If a is full, difference is the flip of b. if len(a.runs) > 0 && a.runs[0].start == 0 && a.runs[0].last == 65535 { return flipBitmap(b) @@ -2711,6 +2766,7 @@ func differenceRunBitmap(a, b *Container) *Container { // differenceRunRun computes the difference of two runs. func differenceRunRun(a, b *Container) *Container { + statsHit("difference/RunRun") if a.n == 0 || b.n == 0 { return a.Clone() } @@ -2774,6 +2830,7 @@ func differenceRunRun(a, b *Container) *Container { } func differenceArrayBitmap(a, b *Container) *Container { + statsHit("difference/ArrayBitmap") output := &Container{containerType: containerArray} for _, va := range a.array { bmidx := va / 64 @@ -2790,6 +2847,7 @@ func differenceArrayBitmap(a, b *Container) *Container { } func differenceBitmapArray(a, b *Container) *Container { + statsHit("difference/BitmapArray") output := a.Clone() for _, v := range b.array { @@ -2805,6 +2863,7 @@ func differenceBitmapArray(a, b *Container) *Container { } func differenceBitmapBitmap(a, b *Container) *Container { + statsHit("difference/BitmapBitmap") // local variables added to prevent BCE checks in loop // see https://go101.org/article/bounds-check-elimination.html @@ -2862,6 +2921,7 @@ func xor(a, b *Container) *Container { } func xorArrayArray(a, b *Container) *Container { + statsHit("xor/ArrayArray") output := &Container{containerType: containerArray} na, nb := len(a.array), len(b.array) for i, j := 0, 0; i < na || j < nb; { @@ -2891,6 +2951,7 @@ func xorArrayArray(a, b *Container) *Container { } func xorArrayBitmap(a, b *Container) *Container { + statsHit("xor/ArrayBitmap") output := b.Clone() for _, v := range a.array { if b.bitmapContains(v) { @@ -2910,6 +2971,7 @@ func xorArrayBitmap(a, b *Container) *Container { } func xorBitmapBitmap(a, b *Container) *Container { + statsHit("xor/BitmapBitmap") // local variables added to prevent BCE checks in loop // see https://go101.org/article/bounds-check-elimination.html @@ -2987,6 +3049,7 @@ func (op *op) UnmarshalBinary(data []byte) error { if len(data) < op.size() { return fmt.Errorf("op data out of bounds: len=%d", len(data)) } + statsHit("op/UnmarshalBinary") // Verify checksum. h := fnv.New32a() @@ -3011,6 +3074,7 @@ func lowbits(v uint64) uint16 { return uint16(v & 0xFFFF) } // search32 returns the index of value in a. If value is not found, it works the // same way as search64. func search32(a []uint16, value uint16) int32 { + statsHit("search32") // Optimize for elements and the last element. n := int32(len(a)) if n == 0 { @@ -3054,6 +3118,7 @@ func search32(a []uint16, value uint16) int32 { // since negative 0 is no different from positive 0, we offset the returned // negative indices by 1. See the test for this function for examples. func search64(a []uint64, value uint64) int { + statsHit("search64") // Optimize for elements and the last element. n := len(a) if n == 0 { @@ -3132,6 +3197,7 @@ func (a *ErrorList) AppendWithPrefix(err error, prefix string) { // xorArrayRun computes the exclusive or of an array and a run container. func xorArrayRun(a, b *Container) *Container { + statsHit("xor/ArrayRun") output := &Container{containerType: containerRun} na, nb := len(a.array), len(b.runs) var vb interval16 @@ -3290,6 +3356,7 @@ type xorstm struct { // xorRunRun computes the exclusive or of two run containers. func xorRunRun(a, b *Container) *Container { + statsHit("xor/RunRun") na, nb := len(a.runs), len(b.runs) if na == 0 { return b.Clone() @@ -3338,6 +3405,7 @@ func xorRunRun(a, b *Container) *Container { // xorRunRun computes the exclusive or of a bitmap and a run container. func xorBitmapRun(a, b *Container) *Container { + statsHit("xor/BitmapRun") output := a.Clone() for j := 0; j < len(b.runs); j++ { output.bitmapXorRange(uint64(b.runs[j].start), uint64(b.runs[j].last)+1) @@ -3352,6 +3420,7 @@ func xorBitmapRun(a, b *Container) *Container { } func bitmapsEqual(b, c *Bitmap) error { // nolint: deadcode + statsHit("bitmapsEqual") if b.OpWriter != c.OpWriter { return errors.New("opWriters not equal") } @@ -3404,6 +3473,7 @@ const ( ) func readOfficialHeader(buf []byte) (size uint32, containerTyper func(index uint, card int) byte, header, pos int, haveRuns bool, err error) { + statsHit("readOfficialHeader") if len(buf) < 8 { err = fmt.Errorf("buffer too small, expecting at least 8 bytes, was %d", len(buf)) return size, containerTyper, header, pos, haveRuns, err @@ -3469,6 +3539,7 @@ func (b *Bitmap) UnmarshalBinary(data []byte) error { // Nothing to unmarshal return nil } + statsHit("Bitmap/UnmarshalBinary") fileMagic := uint32(binary.LittleEndian.Uint16(data[0:2])) if fileMagic == magicNumber { // if pilosa roaring return errors.Wrap(b.unmarshalPilosaRoaring(data), "unmarshaling as pilosa roaring") diff --git a/roaring/roaring_nop_stats.go b/roaring/roaring_nop_stats.go new file mode 100644 index 000000000..c9e029ab7 --- /dev/null +++ b/roaring/roaring_nop_stats.go @@ -0,0 +1,8 @@ +// +build !roaringstats + +package roaring + +// statsCount does nothing, because you aren't building with +// the "roaringstats" build tag. +func statsHit(string) { +} diff --git a/roaring/roaring_stats.go b/roaring/roaring_stats.go new file mode 100644 index 000000000..fd8ade91f --- /dev/null +++ b/roaring/roaring_stats.go @@ -0,0 +1,15 @@ +// +build roaringstats + +package roaring + +import ( + "github.com/pilosa/pilosa/stats" +) + +var statsEv = stats.NewExpvarStatsClient() + +// statsHit increments the given stat, so we can tell how often we've hit +// that particular event. +func statsHit(name string) { + statsEv.Count(name, 1, 1) +} From 8e270f9201822605ab1424254920547121dd8eb0 Mon Sep 17 00:00:00 2001 From: Seebs Date: Thu, 15 Nov 2018 14:58:07 -0600 Subject: [PATCH 25/33] provide commented-out test case for bug in dead code bitmapEquals isn't currently being called ever, but it has an arcane edge-case bug, so I've made the test case for it and commented it out for future reference. --- roaring/roaring_internal_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index a4c162629..266bf39e7 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -3261,3 +3261,26 @@ func TestUnmarshalOfficialRoaring(t *testing.T) { } } + +/* +// This function exercises an arcane edge case in dead code. +// It doesn't need to be run right now. +func TestEquals(t *testing.T) { + bma := NewBitmap() + bmr := NewBitmap() + for i := uint64(0); i < 30; i++ { + bma.Add(i) + bmr.Add(i) + } + bmr.Optimize() + bmi := bma.Intersect(bmr) + err := bitmapsEqual(bmi, bma) + if err != nil { + t.Fatalf("expected intersection to equal array") + } + err = bitmapsEqual(bmi, bmr) + if err != nil { + t.Fatalf("expected intersection to equal run") + } +} +*/ From 08d7f656672a2731c1c2df619d0a9bc1e3565ec1 Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Fri, 16 Nov 2018 13:01:21 -0600 Subject: [PATCH 26/33] increase the translate file size for tests/benchmarks --- translate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/translate_test.go b/translate_test.go index 1aea26cfe..fe78108ff 100644 --- a/translate_test.go +++ b/translate_test.go @@ -809,7 +809,7 @@ func NewTranslateFile() *TranslateFile { } f.Close() - s := &TranslateFile{TranslateFile: pilosa.NewTranslateFile(pilosa.OptTranslateFileMapSize(2 << 25))} + s := &TranslateFile{TranslateFile: pilosa.NewTranslateFile(pilosa.OptTranslateFileMapSize(2 << 26))} s.Path = f.Name() return s } From c8e6fd2e43581e83dc357750c714e6d7fd36f529 Mon Sep 17 00:00:00 2001 From: Seebs Date: Fri, 9 Nov 2018 22:44:37 -0600 Subject: [PATCH 27/33] improve type matrix for IntersectionCount benchmarks The circumstances under which bitmaps are converted between types are not 100% nailed down, and the IntersectionCount benchmark was actually using a bitmap for the "run" data set as well as for the "bitmap" data set. Fix that by using Optimize() explicitly. Also, add a second RLE set so we can compare the difference between "one run for the entire set" and "several runs". Also add array/array comparisons. We use two different lengths of arrays, because performance turns out to vary between "first array longer" and "second array longer". Also added a benchmark for getBenchData itself, since it's at least one possible use case for "creating a lot of containers". --- roaring/roaring_test.go | 117 ++++++++++++++++++++++++++++++++++------ 1 file changed, 101 insertions(+), 16 deletions(-) diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go index 64cb45e81..6c31b7701 100644 --- a/roaring/roaring_test.go +++ b/roaring/roaring_test.go @@ -1056,19 +1056,39 @@ func TestBitmapBufIterator(t *testing.T) { } -var benchmarkBitmapIntersectionCountData struct { - a, b, r *roaring.Bitmap +// this data is used to test various operations across +// different types. +type benchmarkSampleData struct { + a1, a2, b, r1, r2 *roaring.Bitmap } -func getBenchData() *struct{ a, b, r *roaring.Bitmap } { - data := &benchmarkBitmapIntersectionCountData - if data.a == nil { +var sampleData benchmarkSampleData + +func isAllType(b *roaring.Bitmap, typ string) bool { + bi := b.Info() + for _, c := range bi.Containers { + if c.Type != typ { + return false + } + } + return true +} + +func getBenchData(b *testing.B) *benchmarkSampleData { + data := &sampleData + if data.a1 == nil { const max = (1 << 24) / 64 // Build bitmap with array container. - data.a = roaring.NewFileBitmap() - for i, n := 0, 2*roaring.ArrayMaxSize/3; i < n; i++ { - data.a.Add(uint64(rand.Intn(max))) + data.a1 = roaring.NewFileBitmap() + data.a2 = roaring.NewFileBitmap() + // two lists of different lengths + for i, n := 0, roaring.ArrayMaxSize/3; i < n; i++ { + data.a1.Add(uint64(rand.Intn(max))) + data.a2.Add(uint64(rand.Intn(max))) + } + for i, n := 0, roaring.ArrayMaxSize/3; i < n; i++ { + data.a1.Add(uint64(rand.Intn(max))) } // Build bitmap with bitmap container. @@ -1078,12 +1098,42 @@ func getBenchData() *struct{ a, b, r *roaring.Bitmap } { } // build bitmap with run container - data.r = roaring.NewFileBitmap() + data.r1 = roaring.NewFileBitmap() for i, n := 0, MaxContainerVal; i < n; i++ { - data.r.Add(uint64(i)) + data.r1.Add(uint64(i)) } + // build bitmap with multiple runs + data.r2 = roaring.NewFileBitmap() + for i, n := 0, MaxContainerVal; i < n; i++ { + data.r2.Add(uint64(i)) + // break the runs up, this should produce 16 runs, which + // is small enough to make RLE tempting + if i&0xfff == 0xfff { + i += 5 + } + } + data.a1.Optimize() + data.a2.Optimize() + data.b.Optimize() + data.r1.Optimize() + data.r2.Optimize() } + if !isAllType(data.a1, "array") { + b.Fatalf("expected data.a1 to be an array, it wasn't.") + } + if !isAllType(data.a2, "array") { + b.Fatalf("expected data.a2 to be an array, it wasn't.") + } + if !isAllType(data.b, "bitmap") { + b.Fatalf("expected data.b to be a bitmap, it wasn't.") + } + if !isAllType(data.r1, "run") { + b.Fatalf("expected data.r1 to be RLE, it wasn't.") + } + if !isAllType(data.r2, "run") { + b.Fatalf("expected data.r2 to be RLE, it wasn't.") + } return data } @@ -1138,30 +1188,65 @@ func TestBitmap_Intersect(t *testing.T) { } } +func BenchmarkGetBenchData(b *testing.B) { + for i := 0; i < b.N; i++ { + sampleData = benchmarkSampleData{} + getBenchData(b) + } +} + func BenchmarkBitmap_IntersectionCount_ArrayRun(b *testing.B) { - data := getBenchData() + data := getBenchData(b) // Reset timer & benchmark. b.ResetTimer() for i := 0; i < b.N; i++ { - data.a.IntersectionCount(data.r) + data.a1.IntersectionCount(data.r1) + } +} + +func BenchmarkBitmap_IntersectionCount_ArrayRuns(b *testing.B) { + data := getBenchData(b) + // Reset timer & benchmark. + b.ResetTimer() + for i := 0; i < b.N; i++ { + data.a1.IntersectionCount(data.r2) } } func BenchmarkBitmap_IntersectionCount_BitmapRun(b *testing.B) { - data := getBenchData() + data := getBenchData(b) // Reset timer & benchmark. b.ResetTimer() for i := 0; i < b.N; i++ { - data.b.IntersectionCount(data.r) + data.b.IntersectionCount(data.r1) + } +} + +func BenchmarkBitmap_IntersectionCount_BitmapRuns(b *testing.B) { + data := getBenchData(b) + // Reset timer & benchmark. + b.ResetTimer() + for i := 0; i < b.N; i++ { + data.b.IntersectionCount(data.r2) + } +} + +func BenchmarkBitmap_IntersectionCount_ArrayArray(b *testing.B) { + data := getBenchData(b) + // Reset timer & benchmark. + b.ResetTimer() + for i := 0; i < b.N; i++ { + data.a1.IntersectionCount(data.a2) + data.a2.IntersectionCount(data.a1) } } func BenchmarkBitmap_IntersectionCount_ArrayBitmap(b *testing.B) { - data := getBenchData() + data := getBenchData(b) // Reset timer & benchmark. b.ResetTimer() for i := 0; i < b.N; i++ { - data.a.IntersectionCount(data.b) + data.a1.IntersectionCount(data.b) } } From 32c4b3540f33d9384e291ce16e481e4384184482 Mon Sep 17 00:00:00 2001 From: Seebs Date: Tue, 13 Nov 2018 12:12:13 -0600 Subject: [PATCH 28/33] simplify intersectBitmapRun output to remove a conversion If the total number of things returned was small enough to make an array, intersectBitmapRun converted to an array. This seems possibly-premature; future processing might well prefer a bitmap. We know everything gets optimized before being written out, let's not convert without a specific reason. But also, let's use an array no matter which container is small enough to prove that we can do so safely. Fixes #854. --- roaring/roaring.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 3a4050cea..ced003d37 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -2102,12 +2102,12 @@ func intersectRunRun(a, b *Container) *Container { return output } -// intersectBitmapRun returns an array container if the run container's -// cardinality is < ArrayMaxSize. Otherwise it returns a bitmap container. +// intersectBitmapRun returns an array container if either container's +// cardinality is <= ArrayMaxSize. Otherwise it returns a bitmap container. func intersectBitmapRun(a, b *Container) *Container { statsHit("intersect/BitmapRun") var output *Container - if b.n < ArrayMaxSize { + if b.n <= ArrayMaxSize || a.n <= ArrayMaxSize { // output is array container output = &Container{containerType: containerArray} for _, iv := range b.runs { @@ -2161,9 +2161,6 @@ func intersectBitmapRun(a, b *Container) *Container { valast = vastart + 63 } } - if output.n < ArrayMaxSize { - output.bitmapToArray() - } } return output } From d4364bea527f3a7d023e41974dc97b2c2ef8cc13 Mon Sep 17 00:00:00 2001 From: Seebs Date: Mon, 12 Nov 2018 18:04:54 -0600 Subject: [PATCH 29/33] slightly streamline array/array comparison The net effect of this is to not recompute "the current value of the first array" on every loop, pretty much. However, the swap to make sure the inner loop is on the longer array seems to be significant for performance. On my system, this moves runtime from ~29us per op to ~17us per op. --- roaring/roaring.go | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index ced003d37..6cf7eda1c 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1903,16 +1903,26 @@ func intersectionCount(a, b *Container) int32 { func intersectionCountArrayArray(a, b *Container) (n int32) { statsHit("intersectionCount/ArrayArray") - na, nb := len(a.array), len(b.array) - for i, j := 0, 0; i < na && j < nb; { - va, vb := a.array[i], b.array[j] - if va < vb { - i++ - } else if va > vb { - j++ - } else { + s1, s2 := a.array, b.array + if len(s1) == 0 || len(s2) == 0 { + return 0 + } + if len(s1) > len(s2) { + s1, s2 = s2, s1 + } + l2 := len(s2) + i2 := 0 + v2 := s2[0] + for _, v1 := range s1 { + for v2 < v1 { + i2++ + if i2 >= l2 { + return n + } + v2 = s2[i2] + } + if v2 == v1 { n++ - i, j = i+1, j+1 } } return n From 9b552ab5086ef9a9937baf06b773a427bf245ae9 Mon Sep 17 00:00:00 2001 From: Seebs Date: Tue, 13 Nov 2018 12:21:47 -0600 Subject: [PATCH 30/33] enhance TestRunCountRange confirm that the number of runs comes out as expected, and add a couple of numbers out of order to verify that the 17-18-19 set gets coalesced into one run even if we add 17 and 19 before 18. --- roaring/roaring_internal_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index 266bf39e7..78b686371 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -165,8 +165,8 @@ func TestRunCountRange(t *testing.T) { } c.add(17) - c.add(18) c.add(19) + c.add(18) cnt = c.runCountRange(1, 22) if cnt != 10 { @@ -180,6 +180,11 @@ func TestRunCountRange(t *testing.T) { if cnt != 9 { t.Fatalf("should get 9 from multiple ranges overlapping both sides, but got: %v", cnt) } + // verify that the disparate ops resulted in three separate runs + cnt = c.countRuns() + if cnt != 3 { + t.Fatalf("should get 3 total runs, but got: %v [%v]", cnt, c.runs) + } } func TestRunContains(t *testing.T) { From 1a8633f3a5eaafb03b2b5e384c7e70e04fec57d8 Mon Sep 17 00:00:00 2001 From: Seebs Date: Tue, 13 Nov 2018 13:33:52 -0600 Subject: [PATCH 31/33] use roaring conventions for variable names Roaring likes to call things "a" and "b", not "1" and "2", and use "n" for length, not "l", etcetera. Adopt these conventions to make code more readable. Also drop the 'vb' value since it isn't expensive to compute and the compiler can figure out that it can reuse the value. --- roaring/roaring.go | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 6cf7eda1c..bb3c5da6e 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1903,25 +1903,24 @@ func intersectionCount(a, b *Container) int32 { func intersectionCountArrayArray(a, b *Container) (n int32) { statsHit("intersectionCount/ArrayArray") - s1, s2 := a.array, b.array - if len(s1) == 0 || len(s2) == 0 { + ca, cb := a.array, b.array + na, nb := len(ca), len(cb) + if na == 0 || nb == 0 { return 0 } - if len(s1) > len(s2) { - s1, s2 = s2, s1 + if na > nb { + ca, cb = cb, ca + na, nb = nb, na } - l2 := len(s2) - i2 := 0 - v2 := s2[0] - for _, v1 := range s1 { - for v2 < v1 { - i2++ - if i2 >= l2 { + j := 0 + for _, va := range ca { + for cb[j] < va { + j++ + if j >= nb { return n } - v2 = s2[i2] } - if v2 == v1 { + if cb[j] == va { n++ } } From 7c82f4804604a12c1efa1c688110dc6aeead2ab0 Mon Sep 17 00:00:00 2001 From: Seebs Date: Tue, 13 Nov 2018 14:06:15 -0600 Subject: [PATCH 32/33] improve testing for intersections of array/array pairs A transient bug introduced in intersectionCountArrayArray was not caught by the tests, because it would only manifest when two containers of different lengths were being compared. Also improve the testing for intersectArrayArray, even though that code hasn't been changed. --- roaring/roaring_test.go | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go index 6c31b7701..f5c016ea3 100644 --- a/roaring/roaring_test.go +++ b/roaring/roaring_test.go @@ -411,13 +411,29 @@ func TestBitmap_Intersection_Empty(t *testing.T) { } func TestBitmap_IntersectArrayArray(t *testing.T) { - bm0 := roaring.NewFileBitmap(0, 1, 2683, 5005) + bm0 := roaring.NewFileBitmap(0, 1, 7, 9, 11, 2683, 5005) bm1 := roaring.NewFileBitmap(0, 2683, 2684, 5000) + expected := []uint64{0, 2683} result := bm0.Intersect(bm1) if n := result.Count(); n != 2 { t.Fatalf("unexpected n: %d", n) } + for _, e := range expected { + if !result.Contains(e) { + t.Fatalf("missing value %d", e) + } + } + // confirm that it also works going the other way + result = bm1.Intersect(bm0) + if n := result.Count(); n != 2 { + t.Fatalf("unexpected n: %d", n) + } + for _, e := range expected { + if !result.Contains(e) { + t.Fatalf("missing value %d", e) + } + } } func TestBitmap_IntersectBitmapBitmap(t *testing.T) { @@ -689,10 +705,10 @@ func TestBitmap_Flip_After(t *testing.T) { } -// Ensure bitmap can return the number of intersecting bits in two bitmaps. +// Ensure bitmap can return the number of intersecting bits in two arrays. func TestBitmap_IntersectionCount_ArrayArray(t *testing.T) { - bm0 := roaring.NewFileBitmap(0, 1, 1000001, 1000002, 1000003) - bm1 := roaring.NewFileBitmap(0, 50000, 1000001, 1000002) + bm0 := roaring.NewFileBitmap(0, 1000001, 1000002, 1000003) + bm1 := roaring.NewFileBitmap(0, 50000, 999998, 999999, 1000000, 1000001, 1000002) if n := bm0.IntersectionCount(bm1); n != 3 { t.Fatalf("unexpected n: %d", n) From e20671b2b4c8a542fd30ffb1829b836aa38dc5de Mon Sep 17 00:00:00 2001 From: Seebs Date: Tue, 13 Nov 2018 14:20:25 -0600 Subject: [PATCH 33/33] silence gometalinter I am aware that I don't actually ever use the length of a after this line of code, but if I don't correctly update it, any future change that needs that length will break mysteriously. We humbly ask gometalinter to consider the reply of counsel in _Arkell v. Pressdram_ (1971). --- roaring/roaring.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index bb3c5da6e..9c4274df9 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -1910,7 +1910,7 @@ func intersectionCountArrayArray(a, b *Container) (n int32) { } if na > nb { ca, cb = cb, ca - na, nb = nb, na + na, nb = nb, na // nolint: ineffassign } j := 0 for _, va := range ca {