From cb15307978fd0a425acb62a83e78c6d7da712735 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 42029e3e689..3dd02fa6214 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Features - [#3541](https://github.com/influxdata/influxdb/issues/3451): Update SHOW FIELD KEYS to return the field type with the field key. +- [#3733](https://github.com/influxdata/influxdb/issues/3733): Modify the default retention policy name and make it configurable. ## v0.13.0 [unreleased] 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 108e1aed13c..6ffcfa91303 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]