From 9c561f4e4b527db462bc4b271bd07ce7ebeee977 Mon Sep 17 00:00:00 2001 From: Michael Baird Date: Tue, 11 Jul 2017 11:30:57 -0500 Subject: [PATCH 1/5] string in slice, and contains substring in slice --- pilosa.go | 24 ++++++++++++++++++++++++ pilosa_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/pilosa.go b/pilosa.go index a334a2be1..8768541ef 100644 --- a/pilosa.go +++ b/pilosa.go @@ -17,6 +17,7 @@ package pilosa import ( "errors" "regexp" + "strings" "github.com/pilosa/pilosa/internal" ) @@ -56,6 +57,9 @@ var ( ErrFragmentNotFound = errors.New("fragment not found") ErrQueryRequired = errors.New("query required") ErrTooManyWrites = errors.New("too many write commands") + + ErrConfigHosts = errors.New("missing bind address in cluster hosts") + ErrConfigBroadcastPort = errors.New("missing broadcast port in internal-hosts") ) // Regular expression to validate index and frame names. @@ -132,3 +136,23 @@ func ValidateLabel(label string) error { } return nil } + +// StringInSlice checks is substring a is in the slice +func StringInSlice(a string, list []string) bool { + for _, b := range list { + if b == a { + return true + } + } + return false +} + +// ContainsSubstring checks is substring a is contained in the slice +func ContainsSubstring(a string, list []string) bool { + for _, b := range list { + if strings.Contains(b, a) { + return true + } + } + return false +} diff --git a/pilosa_test.go b/pilosa_test.go index 7f43fefbf..3b136dd56 100644 --- a/pilosa_test.go +++ b/pilosa_test.go @@ -54,3 +54,27 @@ func TestValidateLabelInvalid(t *testing.T) { } } } + +func TestStringInSlice(t *testing.T) { + list := []string{"localhost:10101", "localhost:10102", "localhost:10103"} + substr := "localhost:10101" + if !pilosa.StringInSlice(substr, list) { + t.Fatalf("Expected substring %s in %v", substr, list) + } + substr = "10101" + if pilosa.StringInSlice(substr, list) { + t.Fatalf("Expected substring %s not in %v", substr, list) + } +} + +func TestContainsSubstring(t *testing.T) { + list := []string{"localhost:10101", "localhost:10102", "localhost:10103"} + substr := "10101" + if !pilosa.ContainsSubstring(substr, list) { + t.Fatalf("Expected substring %s contained in %v", substr, list) + } + substr = "4000" + if pilosa.ContainsSubstring(substr, list) { + t.Fatalf("Expected substring %s in not contained in %v", substr, list) + } +} From baff53ae3d811bb6053d5ffec363dcb50cdba6dc Mon Sep 17 00:00:00 2001 From: Michael Baird Date: Tue, 11 Jul 2017 11:31:27 -0500 Subject: [PATCH 2/5] config test and validate --- config.go | 20 ++++++++++++++++++++ config_test.go | 18 ++++++++++++++++++ server/server.go | 6 +++++- 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 config_test.go diff --git a/config.go b/config.go index b6961850b..b926a44fa 100644 --- a/config.go +++ b/config.go @@ -89,6 +89,26 @@ func NewConfig() *Config { return c } +// Validate that all configuration permutations are compatible with each other. +func (c *Config) Validate() error { + if !foundItem(c.Cluster.Hosts, c.Bind) { + return ErrConfigHosts + } + + // Validate cluster types + // TODO cluster types + // TODO validate len hosts + // TODO vaidate replica num and host len + // TODO internal-hosts and hosts len must match + + if c.Cluster.Type == "http" || c.Cluster.Type == "gossip" { + if !ContainsSubstring(c.Cluster.InternalPort, c.Cluster.InternalHosts) { + return ErrConfigBroadcastPort + } + } + return nil +} + // Duration is a TOML wrapper type for time.Duration. type Duration time.Duration diff --git a/config_test.go b/config_test.go new file mode 100644 index 000000000..1109f1436 --- /dev/null +++ b/config_test.go @@ -0,0 +1,18 @@ +package pilosa_test + +import ( + "testing" + + "github.com/pilosa/pilosa" +) + +func Test_NewConfig(t *testing.T) { + x := pilosa.NewConfig() + + // Check for bind addres in cluster hosts + if err := x.Validate(); err != pilosa.ErrConfigHosts { + t.Fatal(err) + } + + x.Cluster.Type = "http" +} diff --git a/server/server.go b/server/server.go index fd4462be9..3133dac05 100644 --- a/server/server.go +++ b/server/server.go @@ -107,7 +107,11 @@ func (m *Command) Run(args ...string) (err error) { // SetupServer use the cluster configuration to setup this server func (m *Command) SetupServer() error { - var err error + err := m.Config.Validate() + if err != nil { + return err + } + cluster := pilosa.NewCluster() cluster.ReplicaN = m.Config.Cluster.ReplicaN From 4b53ec1f9c05653c136e957429cd05a9a780f285 Mon Sep 17 00:00:00 2001 From: Michael Baird Date: Tue, 11 Jul 2017 14:17:06 -0500 Subject: [PATCH 3/5] const cluster types, and more validation tests --- cmd/server_test.go | 10 ++++++++++ config.go | 46 ++++++++++++++++++++++++++++++++++------------ config_test.go | 42 ++++++++++++++++++++++++++++++++++++++---- pilosa.go | 9 +++++++-- server/server.go | 6 +++--- 5 files changed, 92 insertions(+), 21 deletions(-) diff --git a/cmd/server_test.go b/cmd/server_test.go index a0f6096c8..25d0d7fd1 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -52,10 +52,15 @@ func TestServerConfig(t *testing.T) { [cluster] poll-interval = "45s" + type = "http" replicas = 2 hosts = [ "localhost:19444", ] + internal-hosts = [ + "localhost:19500", + "localhost:19501", + ] `, validation: func() error { v := validator{} @@ -75,9 +80,14 @@ func TestServerConfig(t *testing.T) { bind = "localhost:0" data-dir = "` + actualDataDir + `" [cluster] + type = "http" hosts = [ "localhost:19444", ] + internal-hosts = [ + "localhost:19500", + "localhost:19501", + ] [plugins] path = "/var/sloth" `, diff --git a/config.go b/config.go index b926a44fa..8607c3c86 100644 --- a/config.go +++ b/config.go @@ -14,7 +14,17 @@ package pilosa -import "time" +import ( + "time" +) + +// Cluster types. +const ( + ClusterNone = "" + ClusterStatic = "static" + ClusterHTTP = "http" + ClusterGossip = "gossip" +) const ( // DefaultHost is the default hostname to use. @@ -24,7 +34,7 @@ const ( DefaultPort = "10101" // DefaultClusterType sets the node intercommunication method. - DefaultClusterType = "static" + DefaultClusterType = ClusterStatic // DefaultInternalPort the port the nodes intercommunicate on. DefaultInternalPort = "14000" @@ -36,6 +46,9 @@ const ( DefaultMaxWritesPerRequest = 5000 ) +// ClusterTypes set of cluster types. +var ClusterTypes = []string{ClusterNone, ClusterStatic, ClusterHTTP, ClusterGossip} + // Config represents the configuration for the command. type Config struct { DataDir string `toml:"data-dir"` @@ -91,21 +104,30 @@ func NewConfig() *Config { // Validate that all configuration permutations are compatible with each other. func (c *Config) Validate() error { - if !foundItem(c.Cluster.Hosts, c.Bind) { - return ErrConfigHosts + if !StringInSlice(c.Cluster.Type, ClusterTypes) { + return ErrConfigClusterTypeInvalid } - - // Validate cluster types - // TODO cluster types - // TODO validate len hosts - // TODO vaidate replica num and host len - // TODO internal-hosts and hosts len must match - - if c.Cluster.Type == "http" || c.Cluster.Type == "gossip" { + if len(c.Cluster.Hosts) > 1 && !(c.Cluster.Type == ClusterHTTP || c.Cluster.Type == ClusterGossip) { + return ErrConfigClusterTypeMissing + } + if c.Cluster.Type == ClusterHTTP || c.Cluster.Type == ClusterGossip { + if c.Cluster.ReplicaN > len(c.Cluster.Hosts) { + return ErrConfigReplicaNInvalid + } + if len(c.Cluster.Hosts) != len(c.Cluster.InternalHosts) { + return ErrConfigHostsMismatch + } + if !foundItem(c.Cluster.Hosts, c.Bind) { + return ErrConfigHostsMissing + } if !ContainsSubstring(c.Cluster.InternalPort, c.Cluster.InternalHosts) { return ErrConfigBroadcastPort } } + if c.Cluster.Type == ClusterGossip && !StringInSlice(c.Cluster.GossipSeed, c.Cluster.InternalHosts) { + return ErrConfigGossipSeed + } + return nil } diff --git a/config_test.go b/config_test.go index 1109f1436..9f7767a4e 100644 --- a/config_test.go +++ b/config_test.go @@ -7,12 +7,46 @@ import ( ) func Test_NewConfig(t *testing.T) { - x := pilosa.NewConfig() + c := pilosa.NewConfig() - // Check for bind addres in cluster hosts - if err := x.Validate(); err != pilosa.ErrConfigHosts { + c.Cluster.Hosts = []string{c.Bind, "localhost:10102"} + if err := c.Validate(); err != pilosa.ErrConfigClusterTypeMissing { t.Fatal(err) } - x.Cluster.Type = "http" + c.Cluster.Type = "test" + if err := c.Validate(); err != pilosa.ErrConfigClusterTypeInvalid { + t.Fatal(err) + } + + c.Cluster.Type = pilosa.ClusterHTTP + if err := c.Validate(); err != pilosa.ErrConfigHostsMismatch { + t.Fatal(err) + } + + c.Cluster.InternalPort = pilosa.DefaultInternalPort + c.Cluster.InternalHosts = []string{"localhost:14004", "localhost:14001"} + if err := c.Validate(); err != pilosa.ErrConfigBroadcastPort { + t.Fatal(err) + } + + c.Cluster.InternalHosts = []string{"localhost:14000", "localhost:14001"} + c.Bind = "localhost:1" + // Check for bind addres in cluster hosts + if err := c.Validate(); err != pilosa.ErrConfigHostsMissing { + t.Fatal(err) + } + + c.Bind = "localhost:10101" + c.Cluster.ReplicaN = 3 + if err := c.Validate(); err != pilosa.ErrConfigReplicaNInvalid { + t.Fatal(err) + } + + c.Cluster.ReplicaN = 2 + c.Cluster.Type = pilosa.ClusterGossip + c.Cluster.GossipSeed = "localhost:10101" + if err := c.Validate(); err != pilosa.ErrConfigGossipSeed { + t.Fatal(err) + } } diff --git a/pilosa.go b/pilosa.go index 8768541ef..51910ffee 100644 --- a/pilosa.go +++ b/pilosa.go @@ -58,8 +58,13 @@ var ( ErrQueryRequired = errors.New("query required") ErrTooManyWrites = errors.New("too many write commands") - ErrConfigHosts = errors.New("missing bind address in cluster hosts") - ErrConfigBroadcastPort = errors.New("missing broadcast port in internal-hosts") + ErrConfigClusterTypeInvalid = errors.New("invalid cluster type") + ErrConfigClusterTypeMissing = errors.New("missing cluster type") + ErrConfigHostsMissing = errors.New("missing bind address in cluster hosts") + ErrConfigBroadcastPort = errors.New("missing broadcast port in internal-hosts") + ErrConfigHostsMismatch = errors.New("hosts and internal-hosts length mismatch") + ErrConfigReplicaNInvalid = errors.New("replica number must be <= hosts") + ErrConfigGossipSeed = errors.New("invalid gossip seed") ) // Regular expression to validate index and frame names. diff --git a/server/server.go b/server/server.go index 3133dac05..6e51c4e74 100644 --- a/server/server.go +++ b/server/server.go @@ -158,7 +158,7 @@ func (m *Command) SetupServer() error { } switch m.Config.Cluster.Type { - case "http": + case pilosa.ClusterHTTP: m.Server.Broadcaster = httpbroadcast.NewHTTPBroadcaster(m.Server, internalPortStr) m.Server.BroadcastReceiver = httpbroadcast.NewHTTPBroadcastReceiver(internalPortStr, m.Server.LogOutput) m.Server.Cluster.NodeSet = httpbroadcast.NewHTTPNodeSet() @@ -166,7 +166,7 @@ func (m *Command) SetupServer() error { if err != nil { return err } - case "gossip": + case pilosa.ClusterGossip: gossipPort, err := strconv.Atoi(internalPortStr) if err != nil { return err @@ -184,7 +184,7 @@ func (m *Command) SetupServer() error { m.Server.Cluster.NodeSet = gossipNodeSet m.Server.Broadcaster = gossipNodeSet m.Server.BroadcastReceiver = gossipNodeSet - case "static", "": + case pilosa.ClusterStatic, pilosa.ClusterNone: m.Server.Broadcaster = pilosa.NopBroadcaster m.Server.Cluster.NodeSet = pilosa.NewStaticNodeSet() m.Server.BroadcastReceiver = pilosa.NopBroadcastReceiver From cfedda29ef9e634094dcf871bbe5fc47c4a7cdec Mon Sep 17 00:00:00 2001 From: Michael Baird Date: Wed, 12 Jul 2017 09:07:38 -0500 Subject: [PATCH 4/5] Added Duration tests --- config.go | 1 + config_test.go | 36 ++++++++++++++++++++++++++++++++++++ fragment_test.go | 2 +- 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/config.go b/config.go index 8607c3c86..30c1ebdd3 100644 --- a/config.go +++ b/config.go @@ -153,6 +153,7 @@ func (d Duration) MarshalText() (text []byte, err error) { return []byte(d.String()), nil } +// MarshalTOML write duration into valid TOML. func (d Duration) MarshalTOML() ([]byte, error) { return []byte(d.String()), nil } diff --git a/config_test.go b/config_test.go index 9f7767a4e..78c4a6127 100644 --- a/config_test.go +++ b/config_test.go @@ -1,7 +1,9 @@ package pilosa_test import ( + "reflect" "testing" + "time" "github.com/pilosa/pilosa" ) @@ -49,4 +51,38 @@ func Test_NewConfig(t *testing.T) { if err := c.Validate(); err != pilosa.ErrConfigGossipSeed { t.Fatal(err) } + + c.Cluster.GossipSeed = "localhost:14000" + if err := c.Validate(); err != nil { + t.Fatal(err) + } +} + +func TestDuration(t *testing.T) { + d := pilosa.Duration(time.Second * 182) + if d.String() != "3m2s" { + t.Fatalf("Unexpected time Duration %s", d) + } + + b := []byte{51, 109, 50, 115} + v, _ := d.MarshalText() + if !reflect.DeepEqual(b, v) { + t.Fatalf("Unexpected marshalled value %v", v) + } + + v, _ = d.MarshalTOML() + if !reflect.DeepEqual(b, v) { + t.Fatalf("Unexpected marshalled value %v", v) + } + + err := d.UnmarshalText([]byte("5")) + if err.Error() != "time: missing unit in duration 5" { + t.Fatalf("expected time: missing unit in duration: %s", err) + } + + err = d.UnmarshalText([]byte("3m2s")) + v, _ = d.MarshalText() + if !reflect.DeepEqual(b, v) { + t.Fatalf("Unexpected marshalled value %v", v) + } } diff --git a/fragment_test.go b/fragment_test.go index 74ee91eeb..7ba243e0a 100644 --- a/fragment_test.go +++ b/fragment_test.go @@ -497,7 +497,7 @@ func TestFragment_Checksum(t *testing.T) { // Ensure new checksum is different. if chksum := f.Checksum(); bytes.Equal(chksum, orig) { - t.Fatalf("expected checksum to change: %x", chksum, orig) + t.Fatalf("expected checksum to change: %x - %x", chksum, orig) } } From 69e1372d4a2d5a13e5b3741ce6282de980cb85d8 Mon Sep 17 00:00:00 2001 From: Michael Baird Date: Wed, 12 Jul 2017 13:17:36 -0500 Subject: [PATCH 5/5] clarified configuration error message on internal port mismatch. --- pilosa.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pilosa.go b/pilosa.go index 51910ffee..5ed50e8dd 100644 --- a/pilosa.go +++ b/pilosa.go @@ -61,7 +61,7 @@ var ( ErrConfigClusterTypeInvalid = errors.New("invalid cluster type") ErrConfigClusterTypeMissing = errors.New("missing cluster type") ErrConfigHostsMissing = errors.New("missing bind address in cluster hosts") - ErrConfigBroadcastPort = errors.New("missing broadcast port in internal-hosts") + ErrConfigBroadcastPort = errors.New("internal-port not found in internal-hosts") ErrConfigHostsMismatch = errors.New("hosts and internal-hosts length mismatch") ErrConfigReplicaNInvalid = errors.New("replica number must be <= hosts") ErrConfigGossipSeed = errors.New("invalid gossip seed")