From baaa782c9504323d054841ae10e82b9543d6351a Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Mon, 23 May 2016 15:32:26 -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 | 17 +++++++----- services/meta/data.go | 31 ++++++++++++++++++--- 10 files changed, 56 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cabcb45628..0ea027bf3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ - [#5906](https://github.com/influxdata/influxdb/issues/5906): Dynamically update the documentation link in the admin UI. - [#6686](https://github.com/influxdata/influxdb/pull/6686): Optimize timestamp run-length decoding - [#6713](https://github.com/influxdata/influxdb/pull/6713): Reduce allocations during query parsing. +- [#3733](https://github.com/influxdata/influxdb/issues/3733): Modify the default retention policy name and make it configurable. ### Bugfixes diff --git a/cmd/influxd/run/server_suite_test.go b/cmd/influxd/run/server_suite_test.go index edec932b6b..65b27d0353 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 ec248eaddb..528e8abccf 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 7c4c28fd22..84269de560 100644 --- a/influxql/parser.go +++ b/influxql/parser.go @@ -1573,16 +1573,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 1fddbd77d0..c269262a30 100644 --- a/influxql/parser_test.go +++ b/influxql/parser_test.go @@ -1624,7 +1624,6 @@ func TestParser_ParseStatement(t *testing.T) { RetentionPolicyCreate: true, RetentionPolicyDuration: 24 * time.Hour, RetentionPolicyReplication: 1, - RetentionPolicyName: "default", }, }, { @@ -1636,7 +1635,6 @@ func TestParser_ParseStatement(t *testing.T) { RetentionPolicyDuration: 0, RetentionPolicyReplication: 1, RetentionPolicyShardGroupDuration: 30 * time.Minute, - RetentionPolicyName: "default", }, }, { @@ -1647,7 +1645,6 @@ func TestParser_ParseStatement(t *testing.T) { RetentionPolicyCreate: true, RetentionPolicyDuration: 24 * time.Hour, RetentionPolicyReplication: 1, - RetentionPolicyName: "default", }, }, { @@ -1658,7 +1655,6 @@ func TestParser_ParseStatement(t *testing.T) { RetentionPolicyCreate: true, RetentionPolicyDuration: 0, RetentionPolicyReplication: 2, - RetentionPolicyName: "default", }, }, { @@ -1669,7 +1665,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 addb957545..77a765d0e2 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 - } + if di := m.MetaClient.Database(m.storeDatabase); di == nil { + rpi := meta.NewRetentionPolicyInfo(MonitorRetentionPolicy) + rpi.Duration = MonitorRetentionPolicyDuration + rpi.ReplicaN = 1 - 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 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 8dad1d6732..dd0d8a8252 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 e346e7e87b..ee57b5971f 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 5ad7a57ced..95db58400f 100644 --- a/services/meta/config.go +++ b/services/meta/config.go @@ -13,15 +13,19 @@ const ( // DefaultLoggingEnabled determines if log messages are printed for the meta service DefaultLoggingEnabled = true + + // DefaultRetentionPolicyName is the default retention policy name. + DefaultRetentionPolicyName = "autogen" ) // Config represents the meta configuration. 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 +33,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: DefaultRetentionPolicyName, + LeaseDuration: toml.Duration(DefaultLeaseDuration), + LoggingEnabled: DefaultLoggingEnabled, } } diff --git a/services/meta/data.go b/services/meta/data.go index 8a91547850..dd36269191 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]