From 64eedb7db0cf0fc7aaf272a0d328229edeb3603c Mon Sep 17 00:00:00 2001 From: Paul Dix Date: Fri, 5 Sep 2014 14:54:56 -0400 Subject: [PATCH] Update shard space to not set defaults Fixes #886. Shard spaces would not have compiled regexes when the server is restarted and the cluster config is pulled from a raft snapshot. A call to MatchSeries would then reset the regex for the shard space. BAAAAAD. --- cluster/cluster_configuration_test.go | 53 +++++++++++++++++++++++ cluster/shard_space.go | 23 +++++----- integration/helpers/server.go | 6 ++- integration/single_server_test.go | 62 +++++++++++++++++++++++++++ 4 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 cluster/cluster_configuration_test.go diff --git a/cluster/cluster_configuration_test.go b/cluster/cluster_configuration_test.go new file mode 100644 index 00000000000..fb3074e5aef --- /dev/null +++ b/cluster/cluster_configuration_test.go @@ -0,0 +1,53 @@ +package cluster + +import ( + "github.com/influxdb/influxdb/configuration" + "github.com/influxdb/influxdb/metastore" + . "launchpad.net/gocheck" +) + +type ClusterConfigurationSuite struct{} + +var _ = Suite(&ClusterConfigurationSuite{}) + +// wrote this test while tracking down https://github.com/influxdb/influxdb/issues/886 +func (self *ClusterConfigurationSuite) TestSerializesShardSpaces(c *C) { + config := &configuration.Configuration{} + store := metastore.NewStore() + clusterConfig := NewClusterConfiguration(config, nil, nil, nil, store) + clusterConfig.CreateDatabase("db1") + space1 := NewShardSpace("db1", "space1") + space1.Regex = "/space1/" + space1.ReplicationFactor = 2 + space1.Split = 2 + err := clusterConfig.AddShardSpace(space1) + c.Assert(err, IsNil) + space2 := NewShardSpace("db1", "space2") + space2.Regex = "/space2/" + err = clusterConfig.AddShardSpace(space2) + c.Assert(err, IsNil) + fmt.Println(clusterConfig.databaseShardSpaces) + + verify := func(conf *ClusterConfiguration) { + spaces := conf.databaseShardSpaces["db1"] + c.Assert(spaces, HasLen, 2) + space2 := spaces[0] + space1 := spaces[1] + c.Assert(space1.Name, Equals, "space1") + c.Assert(space1.Regex, Equals, "/space1/") + c.Assert(space1.ReplicationFactor, Equals, uint32(2)) + c.Assert(space1.Split, Equals, uint32(2)) + c.Assert(space2.Name, Equals, "space2") + c.Assert(space2.Regex, Equals, "/space2/") + } + + verify(clusterConfig) + + bytes, err := clusterConfig.Save() + c.Assert(err, IsNil) + newConfig := NewClusterConfiguration(config, nil, nil, nil, store) + err = newConfig.Recovery(bytes) + c.Assert(err, IsNil) + verify(newConfig) + verify(clusterConfig) +} diff --git a/cluster/shard_space.go b/cluster/shard_space.go index 68c5a1e41a2..eb5e1a9a2cf 100644 --- a/cluster/shard_space.go +++ b/cluster/shard_space.go @@ -59,15 +59,7 @@ func (s *ShardSpace) Validate(clusterConfig *ClusterConfiguration, checkForDb bo if s.Regex == "" { return fmt.Errorf("A regex is required for a shard space") } - reg := s.Regex - if strings.HasPrefix(reg, "/") { - if strings.HasSuffix(reg, "/i") { - reg = fmt.Sprintf("(?i)%s", reg[1:len(reg)-2]) - } else { - reg = reg[1 : len(reg)-1] - } - } - r, err := regexp.Compile(reg) + r, err := s.compileRegex(s.Regex) if err != nil { return fmt.Errorf("Error parsing regex: %s", err) } @@ -91,6 +83,17 @@ func (s *ShardSpace) Validate(clusterConfig *ClusterConfiguration, checkForDb bo return nil } +func (s *ShardSpace) compileRegex(reg string) (*regexp.Regexp, error) { + if strings.HasPrefix(reg, "/") { + if strings.HasSuffix(reg, "/i") { + reg = fmt.Sprintf("(?i)%s", reg[1:len(reg)-2]) + } else { + reg = reg[1 : len(reg)-1] + } + } + return regexp.Compile(reg) +} + func (s *ShardSpace) SetDefaults() { r, _ := regexp.Compile(".*") s.compiledRegex = r @@ -103,7 +106,7 @@ func (s *ShardSpace) SetDefaults() { func (s *ShardSpace) MatchesSeries(name string) bool { if s.compiledRegex == nil { - s.SetDefaults() + s.compiledRegex, _ = s.compileRegex(s.Regex) } return s.compiledRegex.MatchString(name) } diff --git a/integration/helpers/server.go b/integration/helpers/server.go index de0f5f91fa7..79132d60e35 100644 --- a/integration/helpers/server.go +++ b/integration/helpers/server.go @@ -129,7 +129,11 @@ func (self *Server) GetClientWithUser(db, username, password string, c *C) *infl } func (self *Server) WriteData(data interface{}, c *C, precision ...influxdb.TimePrecision) { - client := self.GetClient("db1", c) + self.WriteDataToDatabase("db1", data, c, precision...) +} + +func (self *Server) WriteDataToDatabase(db string, data interface{}, c *C, precision ...influxdb.TimePrecision) { + client := self.GetClient(db, c) var series []*influxdb.Series switch x := data.(type) { case string: diff --git a/integration/single_server_test.go b/integration/single_server_test.go index 87d2361db46..18afe4f857f 100644 --- a/integration/single_server_test.go +++ b/integration/single_server_test.go @@ -951,3 +951,65 @@ func (self *SingleServerSuite) TestSeriesShouldReturnSorted(c *C) { c.Assert(s.Name, Equals, fmt.Sprintf("sort_%.3d", i+1)) } } + +// fix for #886 - https://github.com/influxdb/influxdb/issues/886 +func (self *SingleServerSuite) TestShardSpacesCorrectAfterRestart(c *C) { + client := self.server.GetClient("", c) + db := "test_shard_restart" + c.Assert(client.CreateDatabase(db), IsNil) + client = self.server.GetClient(db, c) + + space1 := &influxdb.ShardSpace{Name: "space1", RetentionPolicy: "30d", Regex: "/.*/"} + err := client.CreateShardSpace(db, space1) + c.Assert(err, IsNil) + space2 := &influxdb.ShardSpace{Name: "space2", RetentionPolicy: "1d", Regex: "/^space2.*/"} + err = client.CreateShardSpace(db, space2) + c.Assert(err, IsNil) + + self.server.WriteDataToDatabase(db, ` +[ + { + "name": "foo", + "columns": ["val"], + "points":[[1]] + }, + { + "name": "space2.foo", + "columns": ["val"], + "points":[[2]] + } +]`, c) + + spaces, err := client.GetShardSpaces() + c.Assert(err, IsNil) + c.Assert(self.getSpace(db, "space2", "/^space2.*/", spaces), NotNil) + c.Assert(self.getSpace(db, "space1", "/.*/", spaces), NotNil) + + series, err := client.Query("select count(val) from /.*/") + c.Assert(err, IsNil) + c.Assert(series, HasLen, 2) + c.Assert(series[0].Name, Equals, "foo") + c.Assert(series[0].Points[0][1], Equals, float64(1)) + c.Assert(series[1].Name, Equals, "space2.foo") + c.Assert(series[1].Points[0][1], Equals, float64(1)) + + _, err = http.Post("http://localhost:8086/raft/force_compaction?u=root&p=root", "", nil) + c.Assert(err, IsNil) + + self.server.Stop() + c.Assert(self.server.Start(), IsNil) + self.server.WaitForServerToStart() + + series, err = client.Query("select count(val) from /.*/") + c.Assert(err, IsNil) + c.Assert(series, HasLen, 2) + c.Assert(series[0].Name, Equals, "foo") + c.Assert(series[0].Points[0][1], Equals, float64(1)) + c.Assert(series[1].Name, Equals, "space2.foo") + c.Assert(series[1].Points[0][1], Equals, float64(1)) + + spaces, err = client.GetShardSpaces() + c.Assert(err, IsNil) + c.Assert(self.getSpace(db, "space2", "/^space2.*/", spaces), NotNil) + c.Assert(self.getSpace(db, "space1", "/.*/", spaces), NotNil) +}