From 49f5487de90a711dadb72be91129f7478ebe0bec Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Tue, 10 May 2016 13:21:10 -0400 Subject: [PATCH] Modify the default retention policy name and make it configurable The default retention policy name is changed to "autogen" instead of "default" since it ends up being ambiguous when we tell a user to check the default retention policy, it is uncertain if we are referring to the default retention policy (which can be changed) or the retention policy with the name "default". Now the automatically generated retention policy name is "autogen". The default retention policy is now also configurable through the configuration file so an administrator can customize what they think should be the default. Fixes #3733. --- CHANGELOG.md | 1 + cmd/influxd/run/server_suite_test.go | 2 +- cmd/influxd/run/server_test.go | 2 +- influxql/parser.go | 4 +-- influxql/parser_test.go | 5 ---- monitor/service.go | 40 ++++++++-------------------- services/meta/client.go | 4 +-- services/meta/client_test.go | 1 + services/meta/config.go | 14 +++++----- services/meta/data.go | 31 ++++++++++++++++++--- 10 files changed, 53 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ec340655d4..e9f4f30559d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ ### Bugfixes - [#6604](https://github.com/influxdata/influxdb/pull/6604): Remove old cluster code +- [#3733](https://github.com/influxdata/influxdb/issues/3733): Modify the default retention policy name and make it configurable. ## v0.13.0 [2016-05-12] diff --git a/cmd/influxd/run/server_suite_test.go b/cmd/influxd/run/server_suite_test.go index edec932b6b3..65b27d03531 100644 --- a/cmd/influxd/run/server_suite_test.go +++ b/cmd/influxd/run/server_suite_test.go @@ -464,7 +464,7 @@ func init() { &Query{ name: "show retention policies should return auto-created policy", command: `SHOW RETENTION POLICIES ON db0`, - exp: `{"results":[{"series":[{"columns":["name","duration","shardGroupDuration","replicaN","default"],"values":[["default","0","168h0m0s",1,true]]}]}]}`, + exp: `{"results":[{"series":[{"columns":["name","duration","shardGroupDuration","replicaN","default"],"values":[["autogen","0","168h0m0s",1,true]]}]}]}`, }, }, } diff --git a/cmd/influxd/run/server_test.go b/cmd/influxd/run/server_test.go index fbbc8ace222..71f77844823 100644 --- a/cmd/influxd/run/server_test.go +++ b/cmd/influxd/run/server_test.go @@ -527,7 +527,7 @@ func TestServer_Query_DefaultDBAndRP(t *testing.T) { &Query{ name: "default rp exists", command: `show retention policies ON db0`, - exp: `{"results":[{"series":[{"columns":["name","duration","shardGroupDuration","replicaN","default"],"values":[["default","0","168h0m0s",1,false],["rp0","0","168h0m0s",1,true]]}]}]}`, + exp: `{"results":[{"series":[{"columns":["name","duration","shardGroupDuration","replicaN","default"],"values":[["autogen","0","168h0m0s",1,false],["rp0","0","168h0m0s",1,true]]}]}]}`, }, &Query{ name: "default rp", diff --git a/influxql/parser.go b/influxql/parser.go index 00b772d1fcf..26fa8b1fb7b 100644 --- a/influxql/parser.go +++ b/influxql/parser.go @@ -1568,16 +1568,14 @@ func (p *Parser) parseCreateDatabaseStatement() (*CreateDatabaseStatement, error } // Look for "NAME" - var rpName string = "default" // default is default if err := p.parseTokens([]Token{NAME}); err != nil { p.unscan() } else { - rpName, err = p.parseIdent() + stmt.RetentionPolicyName, err = p.parseIdent() if err != nil { return nil, err } } - stmt.RetentionPolicyName = rpName } else { p.unscan() } diff --git a/influxql/parser_test.go b/influxql/parser_test.go index 4cd0144f5b9..93a39a002ab 100644 --- a/influxql/parser_test.go +++ b/influxql/parser_test.go @@ -1403,7 +1403,6 @@ func TestParser_ParseStatement(t *testing.T) { RetentionPolicyCreate: true, RetentionPolicyDuration: 24 * time.Hour, RetentionPolicyReplication: 1, - RetentionPolicyName: "default", }, }, { @@ -1415,7 +1414,6 @@ func TestParser_ParseStatement(t *testing.T) { RetentionPolicyDuration: 0, RetentionPolicyReplication: 1, RetentionPolicyShardGroupDuration: 30 * time.Minute, - RetentionPolicyName: "default", }, }, { @@ -1426,7 +1424,6 @@ func TestParser_ParseStatement(t *testing.T) { RetentionPolicyCreate: true, RetentionPolicyDuration: 24 * time.Hour, RetentionPolicyReplication: 1, - RetentionPolicyName: "default", }, }, { @@ -1437,7 +1434,6 @@ func TestParser_ParseStatement(t *testing.T) { RetentionPolicyCreate: true, RetentionPolicyDuration: 0, RetentionPolicyReplication: 2, - RetentionPolicyName: "default", }, }, { @@ -1448,7 +1444,6 @@ func TestParser_ParseStatement(t *testing.T) { RetentionPolicyCreate: true, RetentionPolicyDuration: 0, RetentionPolicyReplication: 2, - RetentionPolicyName: "default", }, }, { diff --git a/monitor/service.go b/monitor/service.go index addb957545a..77a765d0e23 100644 --- a/monitor/service.go +++ b/monitor/service.go @@ -12,7 +12,6 @@ import ( "sync" "time" - "github.com/influxdata/influxdb" "github.com/influxdata/influxdb/models" "github.com/influxdata/influxdb/monitor/diagnostics" "github.com/influxdata/influxdb/services/meta" @@ -51,10 +50,8 @@ type Monitor struct { storeInterval time.Duration MetaClient interface { - CreateDatabase(name string) (*meta.DatabaseInfo, error) - CreateRetentionPolicy(database string, rpi *meta.RetentionPolicyInfo) (*meta.RetentionPolicyInfo, error) - SetDefaultRetentionPolicy(database, name string) error - DropRetentionPolicy(database, name string) error + CreateDatabaseWithRetentionPolicy(name string, rpi *meta.RetentionPolicyInfo) (*meta.DatabaseInfo, error) + Database(name string) *meta.DatabaseInfo } // Writer for pushing stats back into the database. @@ -346,31 +343,16 @@ func (m *Monitor) createInternalStorage() { return } - if _, err := m.MetaClient.CreateDatabase(m.storeDatabase); err != nil { - m.Logger.Printf("failed to create database '%s', failed to create storage: %s", - m.storeDatabase, err.Error()) - return - } - - rpi := meta.NewRetentionPolicyInfo(MonitorRetentionPolicy) - rpi.Duration = MonitorRetentionPolicyDuration - rpi.ReplicaN = 1 - if _, err := m.MetaClient.CreateRetentionPolicy(m.storeDatabase, rpi); err != nil { - m.Logger.Printf("failed to create retention policy '%s', failed to create internal storage: %s", - rpi.Name, err.Error()) - return - } + if di := m.MetaClient.Database(m.storeDatabase); di == nil { + rpi := meta.NewRetentionPolicyInfo(MonitorRetentionPolicy) + rpi.Duration = MonitorRetentionPolicyDuration + rpi.ReplicaN = 1 - if err := m.MetaClient.SetDefaultRetentionPolicy(m.storeDatabase, rpi.Name); err != nil { - m.Logger.Printf("failed to set default retention policy on '%s', failed to create internal storage: %s", - m.storeDatabase, err.Error()) - return - } - - err := m.MetaClient.DropRetentionPolicy(m.storeDatabase, "default") - if err != nil && err.Error() != influxdb.ErrRetentionPolicyNotFound("default").Error() { - m.Logger.Printf("failed to delete retention policy 'default', failed to created internal storage: %s", err.Error()) - return + if _, err := m.MetaClient.CreateDatabaseWithRetentionPolicy(m.storeDatabase, rpi); err != nil { + m.Logger.Printf("failed to create database '%s', failed to create storage: %s", + m.storeDatabase, err.Error()) + return + } } // Mark storage creation complete. diff --git a/services/meta/client.go b/services/meta/client.go index 8dad1d6732a..dd0d8a8252c 100644 --- a/services/meta/client.go +++ b/services/meta/client.go @@ -76,6 +76,7 @@ func NewClient(config *Config) *Client { cacheData: &Data{ ClusterID: uint64(uint64(rand.Int63())), Index: 1, + DefaultRetentionPolicyName: config.DefaultRetentionPolicyName, }, closing: make(chan struct{}), changed: make(chan struct{}), @@ -196,12 +197,11 @@ func (c *Client) CreateDatabase(name string) (*DatabaseInfo, error) { // create default retention policy if c.retentionAutoCreate { if err := data.CreateRetentionPolicy(name, &RetentionPolicyInfo{ - Name: "default", ReplicaN: 1, }); err != nil { return nil, err } - if err := data.SetDefaultRetentionPolicy(name, "default"); err != nil { + if err := data.SetDefaultRetentionPolicy(name, ""); err != nil { return nil, err } } diff --git a/services/meta/client_test.go b/services/meta/client_test.go index e346e7e87b6..ee57b5971fe 100644 --- a/services/meta/client_test.go +++ b/services/meta/client_test.go @@ -819,6 +819,7 @@ func newClient() (string, *meta.Client) { func newConfig() *meta.Config { cfg := meta.NewConfig() cfg.Dir = testTempDir(2) + cfg.DefaultRetentionPolicyName = "default" return cfg } diff --git a/services/meta/config.go b/services/meta/config.go index 5ad7a57cedf..aa599815964 100644 --- a/services/meta/config.go +++ b/services/meta/config.go @@ -19,9 +19,10 @@ const ( type Config struct { Dir string `toml:"dir"` - RetentionAutoCreate bool `toml:"retention-autocreate"` - LoggingEnabled bool `toml:"logging-enabled"` - PprofEnabled bool `toml:"pprof-enabled"` + RetentionAutoCreate bool `toml:"retention-autocreate"` + DefaultRetentionPolicyName string `toml:"default-retention-policy-name"` + LoggingEnabled bool `toml:"logging-enabled"` + PprofEnabled bool `toml:"pprof-enabled"` LeaseDuration toml.Duration `toml:"lease-duration"` } @@ -29,9 +30,10 @@ type Config struct { // NewConfig builds a new configuration with default values. func NewConfig() *Config { return &Config{ - RetentionAutoCreate: true, - LeaseDuration: toml.Duration(DefaultLeaseDuration), - LoggingEnabled: DefaultLoggingEnabled, + RetentionAutoCreate: true, + DefaultRetentionPolicyName: "autogen", + LeaseDuration: toml.Duration(DefaultLeaseDuration), + LoggingEnabled: DefaultLoggingEnabled, } } diff --git a/services/meta/data.go b/services/meta/data.go index 8a91547850f..dd362691911 100644 --- a/services/meta/data.go +++ b/services/meta/data.go @@ -37,6 +37,8 @@ type Data struct { MaxShardGroupID uint64 MaxShardID uint64 + + DefaultRetentionPolicyName string `json:"-"` } // NewShardOwner sets the owner of the provided shard to the data node @@ -133,9 +135,7 @@ func (data *Data) RetentionPolicy(database, name string) (*RetentionPolicyInfo, // Returns an error if name is blank or if a database does not exist. func (data *Data) CreateRetentionPolicy(database string, rpi *RetentionPolicyInfo) error { // Validate retention policy. - if rpi.Name == "" { - return ErrRetentionPolicyNameRequired - } else if rpi.ReplicaN < 1 { + if rpi.ReplicaN < 1 { return ErrReplicationFactorTooLow } @@ -155,9 +155,18 @@ func (data *Data) CreateRetentionPolicy(database string, rpi *RetentionPolicyInf return nil } + // Determine the retention policy name if it is blank. + rpName := rpi.Name + if rpName == "" { + if data.DefaultRetentionPolicyName == "" { + return ErrRetentionPolicyNameRequired + } + rpName = data.DefaultRetentionPolicyName + } + // Append copy of new policy. rp := RetentionPolicyInfo{ - Name: rpi.Name, + Name: rpName, Duration: rpi.Duration, ReplicaN: rpi.ReplicaN, ShardGroupDuration: rpi.ShardGroupDuration, @@ -252,6 +261,13 @@ func (data *Data) UpdateRetentionPolicy(database, name string, rpu *RetentionPol // SetDefaultRetentionPolicy sets the default retention policy for a database. func (data *Data) SetDefaultRetentionPolicy(database, name string) error { + if name == "" { + if data.DefaultRetentionPolicyName == "" { + return ErrRetentionPolicyNameRequired + } + name = data.DefaultRetentionPolicyName + } + // Find database and verify policy exists. di := data.Database(database) if di == nil { @@ -728,6 +744,13 @@ type DatabaseInfo struct { // RetentionPolicy returns a retention policy by name. func (di DatabaseInfo) RetentionPolicy(name string) *RetentionPolicyInfo { + if name == "" { + if di.DefaultRetentionPolicy == "" { + return nil + } + name = di.DefaultRetentionPolicy + } + for i := range di.RetentionPolicies { if di.RetentionPolicies[i].Name == name { return &di.RetentionPolicies[i]