From 0c7d46d637cd67c0e39d53eb7194392c9d7eff9c Mon Sep 17 00:00:00 2001 From: Dima Lazerka <58356625+dima-vm@users.noreply.github.com> Date: Sun, 3 Sep 2023 11:33:37 +0300 Subject: [PATCH] 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 --- README.md | 6 +++--- app/vmstorage/main.go | 2 +- docs/README.md | 6 +++--- docs/Single-server-VictoriaMetrics.md | 6 +++--- lib/flagutil/duration.go | 12 ++++++------ lib/flagutil/duration_test.go | 8 ++++---- lib/storage/index_db_test.go | 2 +- lib/storage/storage.go | 6 +++--- lib/storage/storage_test.go | 8 ++++---- lib/storage/table_test.go | 2 +- 10 files changed, 29 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index aebd03b8f..e40e57f30 100644 --- a/README.md +++ b/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. -For instance, `-retentionPeriod=3` means that the data will be stored for 3 months and then deleted. +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 (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. diff --git a/app/vmstorage/main.go b/app/vmstorage/main.go index eef298616..14eefb5e4 100644 --- a/app/vmstorage/main.go +++ b/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 } diff --git a/docs/README.md b/docs/README.md index 34412c343..9410c1020 100644 --- a/docs/README.md +++ b/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. -For instance, `-retentionPeriod=3` means that the data will be stored for 3 months and then deleted. +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 (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. diff --git a/docs/Single-server-VictoriaMetrics.md b/docs/Single-server-VictoriaMetrics.md index d26ed865d..2af0746b7 100644 --- a/docs/Single-server-VictoriaMetrics.md +++ b/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. -For instance, `-retentionPeriod=3` means that the data will be stored for 3 months and then deleted. +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 (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. diff --git a/lib/flagutil/duration.go b/lib/flagutil/duration.go index d42e20831..4b565bbaa 100644 --- a/lib/flagutil/duration.go +++ b/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 int64 + // msecs contains parsed duration in milliseconds. + 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 diff --git a/lib/flagutil/duration_test.go b/lib/flagutil/duration_test.go index b899c45a8..bb138605f 100644 --- a/lib/flagutil/duration_test.go +++ b/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 { - t.Fatalf("unexpected result; got %d; want %d", d.Msecs, expectedMsecs) + if 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("123.456", 123.456*msecsPerMonth) + f("1", msecsPer31Days) + 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) diff --git a/lib/storage/index_db_test.go b/lib/storage/index_db_test.go index 742592b95..28c01b72e 100644 --- a/lib/storage/index_db_test.go +++ b/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 { diff --git a/lib/storage/storage.go b/lib/storage/storage.go index 9514ea8c9..18b0ba34a 100644 --- a/lib/storage/storage.go +++ b/lib/storage/storage.go @@ -33,8 +33,8 @@ import ( ) const ( - retentionMonth = 31 * 24 * time.Hour - retentionMax = 100 * 12 * retentionMonth + retention31Days = 31 * 24 * time.Hour + 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) diff --git a/lib/storage/storage_test.go b/lib/storage/storage_test.go index d2bfe3c71..763835ddd 100644 --- a/lib/storage/storage_test.go +++ b/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 diff --git a/lib/storage/table_test.go b/lib/storage/table_test.go index 49b2881ee..46670d4e7 100644 --- a/lib/storage/table_test.go +++ b/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)