flagutil: Make .Msecs private (#4906)

* Introduce flagutil.Duration

To avoid conversion bugs

* Fix tests

* Clarify documentation re. month=31 days

* Add fasttime.UnixTime() to obtain time.Time

The goal is to refactor out the last usage of `.Msecs`.

* Use fasttime for time.Now()

* wip

- Remove fasttime.UnixTime(), since it doesn't improve code readability and maintainability
- Run `make docs-sync` for syncing changes from README.md to docs/ folder
- Make lib/flagutil.Duration.Msec private
- Rename msecsPerMonth const to msecsPer31Days in order to be consistent with retention31Days

---------

Co-authored-by: Aliaksandr Valialkin <valyala@victoriametrics.com>

README.md

@@ -152,7 +152,7 @@ VictoriaMetrics can also be installed via these installation methods:
 The following command-line flags are used the most:
 
 * `-storageDataPath` - VictoriaMetrics stores all the data in this directory. Default path is `victoria-metrics-data` in the current working directory.
-* `-retentionPeriod` - retention for stored data. Older data is automatically deleted. Default retention is 1 month. The minimum retention period is 24h or 1d. See [the Retention section](#retention) for more details.
+* `-retentionPeriod` - retention for stored data. Older data is automatically deleted. Default retention is 1 month (31 days). The minimum retention period is 24h or 1d. See [these docs](#retention) for more details.
 
 Other flags have good enough default values, so set them only if you really need this. Pass `-help` to see [all the available flags with description and default values](#list-of-command-line-flags).
 
@@ -1610,8 +1610,8 @@ See also [how to work with snapshots](#how-to-work-with-snapshots).
 ## Retention
 
 Retention is configured with the `-retentionPeriod` command-line flag, which takes a number followed by a time unit 
-character - `h(ours)`, `d(ays)`, `w(eeks)`, `y(ears)`. If the time unit is not specified, a month is assumed. 
+character - `h(ours)`, `d(ays)`, `w(eeks)`, `y(ears)`. If the time unit is not specified, a month (31 days) is assumed.
-For instance, `-retentionPeriod=3` means that the data will be stored for 3 months and then deleted. 
+For instance, `-retentionPeriod=3` means that the data will be stored for 3 months (93 days) and then deleted.
 The default retention period is one month. The **minimum retention** period is 24h or 1d.
 
 Data is split in per-month partitions inside `<-storageDataPath>/data/{small,big}` folders.

app/vmstorage/main.go

@@ -75,7 +75,7 @@ func CheckTimeRange(tr storage.TimeRange) error {
 	if !*denyQueriesOutsideRetention {
 		return nil
 	}
-	minAllowedTimestamp := int64(fasttime.UnixTimestamp()*1000) - retentionPeriod.Msecs
+	minAllowedTimestamp := int64(fasttime.UnixTimestamp()*1000) - retentionPeriod.Duration().Milliseconds()
 	if tr.MinTimestamp > minAllowedTimestamp {
 		return nil
 	}

docs/README.md

@@ -155,7 +155,7 @@ VictoriaMetrics can also be installed via these installation methods:
 The following command-line flags are used the most:
 
 * `-storageDataPath` - VictoriaMetrics stores all the data in this directory. Default path is `victoria-metrics-data` in the current working directory.
-* `-retentionPeriod` - retention for stored data. Older data is automatically deleted. Default retention is 1 month. The minimum retention period is 24h or 1d. See [the Retention section](#retention) for more details.
+* `-retentionPeriod` - retention for stored data. Older data is automatically deleted. Default retention is 1 month (31 days). The minimum retention period is 24h or 1d. See [these docs](#retention) for more details.
 
 Other flags have good enough default values, so set them only if you really need this. Pass `-help` to see [all the available flags with description and default values](#list-of-command-line-flags).
 
@@ -1613,8 +1613,8 @@ See also [how to work with snapshots](#how-to-work-with-snapshots).
 ## Retention
 
 Retention is configured with the `-retentionPeriod` command-line flag, which takes a number followed by a time unit 
-character - `h(ours)`, `d(ays)`, `w(eeks)`, `y(ears)`. If the time unit is not specified, a month is assumed. 
+character - `h(ours)`, `d(ays)`, `w(eeks)`, `y(ears)`. If the time unit is not specified, a month (31 days) is assumed.
-For instance, `-retentionPeriod=3` means that the data will be stored for 3 months and then deleted. 
+For instance, `-retentionPeriod=3` means that the data will be stored for 3 months (93 days) and then deleted.
 The default retention period is one month. The **minimum retention** period is 24h or 1d.
 
 Data is split in per-month partitions inside `<-storageDataPath>/data/{small,big}` folders.

docs/Single-server-VictoriaMetrics.md

@@ -163,7 +163,7 @@ VictoriaMetrics can also be installed via these installation methods:
 The following command-line flags are used the most:
 
 * `-storageDataPath` - VictoriaMetrics stores all the data in this directory. Default path is `victoria-metrics-data` in the current working directory.
-* `-retentionPeriod` - retention for stored data. Older data is automatically deleted. Default retention is 1 month. The minimum retention period is 24h or 1d. See [the Retention section](#retention) for more details.
+* `-retentionPeriod` - retention for stored data. Older data is automatically deleted. Default retention is 1 month (31 days). The minimum retention period is 24h or 1d. See [these docs](#retention) for more details.
 
 Other flags have good enough default values, so set them only if you really need this. Pass `-help` to see [all the available flags with description and default values](#list-of-command-line-flags).
 
@@ -1621,8 +1621,8 @@ See also [how to work with snapshots](#how-to-work-with-snapshots).
 ## Retention
 
 Retention is configured with the `-retentionPeriod` command-line flag, which takes a number followed by a time unit 
-character - `h(ours)`, `d(ays)`, `w(eeks)`, `y(ears)`. If the time unit is not specified, a month is assumed. 
+character - `h(ours)`, `d(ays)`, `w(eeks)`, `y(ears)`. If the time unit is not specified, a month (31 days) is assumed.
-For instance, `-retentionPeriod=3` means that the data will be stored for 3 months and then deleted. 
+For instance, `-retentionPeriod=3` means that the data will be stored for 3 months (93 days) and then deleted.
 The default retention period is one month. The **minimum retention** period is 24h or 1d.
 
 Data is split in per-month partitions inside `<-storageDataPath>/data/{small,big}` folders.

lib/flagutil/duration.go

@@ -25,15 +25,15 @@ func NewDuration(name string, defaultValue string, description string) *Duration
 
 // Duration is a flag for holding duration.
 type Duration struct {
-	// Msecs contains parsed duration in milliseconds.
+	// msecs contains parsed duration in milliseconds.
-	Msecs int64
+	msecs int64
 
 	valueString string
 }
 
 // Duration convert to time.Duration.
 func (d *Duration) Duration() time.Duration {
-	return time.Millisecond * time.Duration(d.Msecs)
+	return time.Millisecond * time.Duration(d.msecs)
 }
 
 // String implements flag.Value interface
@@ -52,7 +52,7 @@ func (d *Duration) Set(value string) error {
 		if months < 0 {
 			return fmt.Errorf("duration months cannot be negative; got %g", months)
 		}
-		d.Msecs = int64(months * msecsPerMonth)
+		d.msecs = int64(months * msecsPer31Days)
 		d.valueString = value
 		return nil
 	}
@@ -65,11 +65,11 @@ func (d *Duration) Set(value string) error {
 	if err != nil {
 		return err
 	}
-	d.Msecs = msecs
+	d.msecs = msecs
 	d.valueString = value
 	return nil
 }
 
 const maxMonths = 12 * 100
 
-const msecsPerMonth = 31 * 24 * 3600 * 1000
+const msecsPer31Days = 31 * 24 * 3600 * 1000

lib/flagutil/duration_test.go

@@ -42,8 +42,8 @@ func TestDurationSetSuccess(t *testing.T) {
 		if err := d.Set(value); err != nil {
 			t.Fatalf("unexpected error in d.Set(%q): %s", value, err)
 		}
-		if d.Msecs != expectedMsecs {
+		if d.msecs != expectedMsecs {
-			t.Fatalf("unexpected result; got %d; want %d", d.Msecs, expectedMsecs)
+			t.Fatalf("unexpected result; got %d; want %d", d.msecs, expectedMsecs)
 		}
 		valueString := d.String()
 		valueExpected := strings.ToLower(value)
@@ -52,8 +52,8 @@ func TestDurationSetSuccess(t *testing.T) {
 		}
 	}
 	f("0", 0)
-	f("1", msecsPerMonth)
+	f("1", msecsPer31Days)
-	f("123.456", 123.456*msecsPerMonth)
+	f("123.456", 123.456*msecsPer31Days)
 	f("1h", 3600*1000)
 	f("1.5d", 1.5*24*3600*1000)
 	f("2.3W", 2.3*7*24*3600*1000)

lib/storage/index_db_test.go

@@ -1429,7 +1429,7 @@ func TestMatchTagFilters(t *testing.T) {
 func TestIndexDBRepopulateAfterRotation(t *testing.T) {
 	r := rand.New(rand.NewSource(1))
 	path := "TestIndexRepopulateAfterRotation"
-	s := MustOpenStorage(path, retentionMonth, 1e5, 1e5)
+	s := MustOpenStorage(path, retention31Days, 1e5, 1e5)
 
 	db := s.idb()
 	if db.generation == 0 {

lib/storage/storage.go

@@ -33,8 +33,8 @@ import (
 )
 
 const (
-	retentionMonth = 31 * 24 * time.Hour
+	retention31Days = 31 * 24 * time.Hour
-	retentionMax   = 100 * 12 * retentionMonth
+	retentionMax    = 100 * 12 * retention31Days
 )
 
 // Storage represents TSDB storage.
@@ -242,7 +242,7 @@ func MustOpenStorage(path string, retention time.Duration, maxHourlySeries, maxD
 	s.idbNext.Store(idbNext)
 
 	// Initialize nextRotationTimestamp
-	nowSecs := time.Now().UnixNano() / 1e9
+	nowSecs := int64(fasttime.UnixTimestamp())
 	retentionSecs := retention.Milliseconds() / 1000 // not .Seconds() because unnecessary float64 conversion
 	nextRotationTimestamp := nextRetentionDeadlineSeconds(nowSecs, retentionSecs, retentionTimezoneOffsetSecs)
 	atomic.StoreInt64(&s.nextRotationTimestamp, nextRotationTimestamp)

lib/storage/storage_test.go

@@ -478,7 +478,7 @@ func TestStorageOpenClose(t *testing.T) {
 
 func TestStorageRandTimestamps(t *testing.T) {
 	path := "TestStorageRandTimestamps"
-	retention := 10 * retentionMonth
+	retention := 10 * retention31Days
 	s := MustOpenStorage(path, retention, 0, 0)
 	t.Run("serial", func(t *testing.T) {
 		for i := 0; i < 3; i++ {
@@ -936,7 +936,7 @@ func testStorageRegisterMetricNames(s *Storage) error {
 func TestStorageAddRowsSerial(t *testing.T) {
 	rng := rand.New(rand.NewSource(1))
 	path := "TestStorageAddRowsSerial"
-	retention := 10 * retentionMonth
+	retention := 10 * retention31Days
 	s := MustOpenStorage(path, retention, 1e5, 1e5)
 	if err := testStorageAddRows(rng, s); err != nil {
 		t.Fatalf("unexpected error: %s", err)
@@ -949,7 +949,7 @@ func TestStorageAddRowsSerial(t *testing.T) {
 
 func TestStorageAddRowsConcurrent(t *testing.T) {
 	path := "TestStorageAddRowsConcurrent"
-	retention := 10 * retentionMonth
+	retention := 10 * retention31Days
 	s := MustOpenStorage(path, retention, 1e5, 1e5)
 	ch := make(chan error, 3)
 	for i := 0; i < cap(ch); i++ {
@@ -1164,7 +1164,7 @@ func testStorageAddMetrics(s *Storage, workerNum int) error {
 func TestStorageDeleteStaleSnapshots(t *testing.T) {
 	rng := rand.New(rand.NewSource(1))
 	path := "TestStorageDeleteStaleSnapshots"
-	retention := 10 * retentionMonth
+	retention := 10 * retention31Days
 	s := MustOpenStorage(path, retention, 1e5, 1e5)
 	const rowsPerAdd = 1e3
 	const addsCount = 10

lib/storage/table_test.go

@@ -7,7 +7,7 @@ import (
 
 func TestTableOpenClose(t *testing.T) {
 	const path = "TestTableOpenClose"
-	const retention = 123 * retentionMonth
+	const retention = 123 * retention31Days
 
 	if err := os.RemoveAll(path); err != nil {
 		t.Fatalf("cannot remove %q: %s", path, err)