Merge pull request #9339 from gyuho/fix-embed-auto-compact

embed: fix revision-based compaction with default value

embed/config.go

@@ -108,16 +108,22 @@ func init() {
 type Config struct {
 	// member
 
-	CorsInfo                *cors.CORSInfo
+	CorsInfo       *cors.CORSInfo
-	LPUrls, LCUrls          []url.URL
+	LPUrls, LCUrls []url.URL
-	Dir                     string `json:"data-dir"`
+	Dir            string `json:"data-dir"`
-	WalDir                  string `json:"wal-dir"`
+	WalDir         string `json:"wal-dir"`
-	MaxSnapFiles            uint   `json:"max-snapshots"`
+	MaxSnapFiles   uint   `json:"max-snapshots"`
-	MaxWalFiles             uint   `json:"max-wals"`
+	MaxWalFiles    uint   `json:"max-wals"`
-	Name                    string `json:"name"`
+	Name           string `json:"name"`
-	SnapCount               uint64 `json:"snapshot-count"`
+	SnapCount      uint64 `json:"snapshot-count"`
+
+	// AutoCompactionMode is either 'periodic' or 'revision'.
+	AutoCompactionMode string `json:"auto-compaction-mode"`
+	// AutoCompactionRetention is either duration string with time unit
+	// (e.g. '5m' for 5-minute), or revision unit (e.g. '5000').
+	// If no time unit is provided and compaction mode is 'periodic',
+	// the unit defaults to hour. For example, '5' translates into 5-hour.
 	AutoCompactionRetention string `json:"auto-compaction-retention"`
-	AutoCompactionMode      string `json:"auto-compaction-mode"`
 
 	// TickMs is the number of milliseconds between heartbeat ticks.
 	// TODO: decouple tickMs and heartbeat tick (current heartbeat tick = 1).
@@ -407,6 +413,7 @@ func (cfg *configYAML) configFromFile(path string) error {
 	return cfg.Validate()
 }
 
+// Validate ensures that '*embed.Config' fields are properly configured.
 func (cfg *Config) Validate() error {
 	if err := checkBindURLs(cfg.LPUrls); err != nil {
 		return err
@@ -465,6 +472,13 @@ func (cfg *Config) Validate() error {
 		return ErrUnsetAdvertiseClientURLsFlag
 	}
 
+	switch cfg.AutoCompactionMode {
+	case "":
+	case CompactorModeRevision, CompactorModePeriodic:
+	default:
+		return fmt.Errorf("unknown auto-compaction-mode %q", cfg.AutoCompactionMode)
+	}
+
 	return nil
 }
 

embed/config_test.go

@@ -148,3 +148,22 @@ func mustCreateCfgFile(t *testing.T, b []byte) *os.File {
 	}
 	return tmpfile
 }
+
+func TestAutoCompactionModeInvalid(t *testing.T) {
+	cfg := NewConfig()
+	cfg.AutoCompactionMode = "period"
+	err := cfg.Validate()
+	if err == nil {
+		t.Errorf("expected non-nil error, got %v", err)
+	}
+}
+
+func TestAutoCompactionModeParse(t *testing.T) {
+	dur, err := parseCompactionRetention("revision", "1")
+	if err != nil {
+		t.Error(err)
+	}
+	if dur != 1 {
+		t.Fatalf("AutoCompactionRetention expected 1, got %d", dur)
+	}
+}

embed/etcd.go

@@ -134,22 +134,13 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) {
 		}
 	}
 
-	var (
-		autoCompactionRetention time.Duration
-		h                       int
-	)
 	// AutoCompactionRetention defaults to "0" if not set.
 	if len(cfg.AutoCompactionRetention) == 0 {
 		cfg.AutoCompactionRetention = "0"
 	}
-	h, err = strconv.Atoi(cfg.AutoCompactionRetention)
+	autoCompactionRetention, err := parseCompactionRetention(cfg.AutoCompactionMode, cfg.AutoCompactionRetention)
-	if err == nil {
+	if err != nil {
-		autoCompactionRetention = time.Duration(int64(h)) * time.Hour
+		return e, err
-	} else {
-		autoCompactionRetention, err = time.ParseDuration(cfg.AutoCompactionRetention)
-		if err != nil {
-			return nil, fmt.Errorf("error parsing AutoCompactionRetention: %v", err)
-		}
 	}
 
 	srvcfg := etcdserver.ServerConfig{
@@ -562,3 +553,22 @@ func (e *Etcd) errHandler(err error) {
 	case e.errc <- err:
 	}
 }
+
+func parseCompactionRetention(mode, retention string) (ret time.Duration, err error) {
+	h, err := strconv.Atoi(retention)
+	if err == nil {
+		switch mode {
+		case CompactorModeRevision:
+			ret = time.Duration(int64(h))
+		case CompactorModePeriodic:
+			ret = time.Duration(int64(h)) * time.Hour
+		}
+	} else {
+		// periodic compaction
+		ret, err = time.ParseDuration(retention)
+		if err != nil {
+			return 0, fmt.Errorf("error parsing CompactionRetention: %v", err)
+		}
+	}
+	return ret, nil
+}