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.
influxdata · May 10, 2016 · cb15307 · cb15307
1 parent 8353b0c
commit cb15307
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 51 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -19,19 +19,21 @@ 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"`
 }
 
 // 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
@@ -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]