From 31217fbfd2a37a887b90fc68a67466f3cf8f98f7 Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Tue, 5 Dec 2023 14:23:52 +0100 Subject: [PATCH 1/6] stores: normalise timestamps on metrics --- stores/metrics.go | 24 +++++++++++++++++++++--- stores/metrics_test.go | 12 +++++++----- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/stores/metrics.go b/stores/metrics.go index 9c4f5af8b..b16116d38 100644 --- a/stores/metrics.go +++ b/stores/metrics.go @@ -292,7 +292,9 @@ func (s *SQLStore) contractMetrics(ctx context.Context, start time.Time, n uint6 if err != nil { return nil, fmt.Errorf("failed to fetch contract metrics: %w", err) } - + for i, m := range metrics { + metrics[i].Timestamp = normaliseTimestamp(start, interval, m.Timestamp) + } return metrics, nil } @@ -312,7 +314,9 @@ func (s *SQLStore) contractSetChurnMetrics(ctx context.Context, start time.Time, if err != nil { return nil, fmt.Errorf("failed to fetch contract set churn metrics: %w", err) } - + for i, m := range metrics { + metrics[i].Timestamp = normaliseTimestamp(start, interval, m.Timestamp) + } return metrics, nil } @@ -327,10 +331,21 @@ func (s *SQLStore) contractSetMetrics(ctx context.Context, start time.Time, n ui if err != nil { return nil, fmt.Errorf("failed to fetch contract set metrics: %w", err) } - + for i, m := range metrics { + metrics[i].Timestamp = normaliseTimestamp(start, interval, m.Timestamp) + } return metrics, nil } +func normaliseTimestamp(start time.Time, interval time.Duration, t unixTimeMS) unixTimeMS { + toNormalise := time.Time(t).UTC() + if start.After(toNormalise) { + return unixTimeMS(start) + } + normalized := (toNormalise.UnixMilli()-start.UnixMilli())/int64(interval)*int64(interval) + start.UnixMilli() + return unixTimeMS(time.UnixMilli(normalized).UTC()) +} + // findPeriods is the core of all methods retrieving metrics. By using integer // division rounding combined with a GROUP BY operation, all rows of a table are // split into intervals and the row with the lowest timestamp for each interval @@ -362,6 +377,9 @@ func (s *SQLStore) walletMetrics(ctx context.Context, start time.Time, n uint64, if err != nil { return nil, fmt.Errorf("failed to fetch wallet metrics: %w", err) } + for i, m := range metrics { + metrics[i].Timestamp = normaliseTimestamp(start, interval, m.Timestamp) + } return } diff --git a/stores/metrics_test.go b/stores/metrics_test.go index c367b059f..0494f0567 100644 --- a/stores/metrics_test.go +++ b/stores/metrics_test.go @@ -14,7 +14,7 @@ import ( ) func TestContractSetMetrics(t *testing.T) { - testStart := time.Now() + testStart := time.Now().Round(time.Millisecond).UTC() ss := newTestSQLStore(t, defaultTestSQLStoreConfig) defer ss.Close() @@ -25,8 +25,8 @@ func TestContractSetMetrics(t *testing.T) { t.Fatal(err) } else if m := metrics[0]; m.Contracts != 0 { t.Fatalf("expected 0 contracts, got %v", m.Contracts) - } else if !time.Time(m.Timestamp).After(testStart) { - t.Fatal("expected time to be after test start") + } else if ti := time.Time(m.Timestamp); !ti.Equal(testStart) { + t.Fatal("expected time to match start time") } else if m.Name != testContractSet { t.Fatalf("expected name to be %v, got %v", testContractSet, m.Name) } @@ -277,8 +277,10 @@ func TestContractMetrics(t *testing.T) { t.Fatal("expected metrics to be sorted by time") } for _, m := range metrics { - if !cmp.Equal(m, fcid2Metric[m.ContractID], cmp.Comparer(api.CompareTimeRFC3339)) { - t.Fatal("unexpected metric", cmp.Diff(m, fcid2Metric[m.ContractID])) + expectedMetric := fcid2Metric[m.ContractID] + expectedMetric.Timestamp = api.TimeRFC3339(normaliseTimestamp(start, interval, unixTimeMS(expectedMetric.Timestamp))) + if !cmp.Equal(m, expectedMetric, cmp.Comparer(api.CompareTimeRFC3339)) { + t.Fatal("unexpected metric", cmp.Diff(m, expectedMetric, cmp.Comparer(api.CompareTimeRFC3339))) } cmpFn(m) } From 189e28ca181c2647555566243ef2a1feca4c5849 Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Tue, 5 Dec 2023 14:44:07 +0100 Subject: [PATCH 2/6] testing: fix TestBusRecordedMetrics --- internal/testing/cluster_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/testing/cluster_test.go b/internal/testing/cluster_test.go index ea5aee8b2..fca36b99b 100644 --- a/internal/testing/cluster_test.go +++ b/internal/testing/cluster_test.go @@ -2182,7 +2182,7 @@ func TestWalletFormUnconfirmed(t *testing.T) { } func TestBusRecordedMetrics(t *testing.T) { - startTime := time.Now() + startTime := time.Now().UTC().Round(time.Second) cluster := newTestCluster(t, testClusterOptions{ hosts: 1, @@ -2208,8 +2208,8 @@ func TestBusRecordedMetrics(t *testing.T) { t.Fatalf("expected 1 contract, got %v", m.Contracts) } else if m.Name != testContractSet { t.Fatalf("expected contract set %v, got %v", testContractSet, m.Name) - } else if !m.Timestamp.Std().After(startTime) { - t.Fatal("expected time to be after start time") + } else if !m.Timestamp.Std().Equal(startTime) { + t.Fatal("expected time to match start time", m.Timestamp.Std(), startTime) } } @@ -2225,8 +2225,8 @@ func TestBusRecordedMetrics(t *testing.T) { t.Fatal("expected non-zero FCID") } else if m.Name != testContractSet { t.Fatalf("expected contract set %v, got %v", testContractSet, m.Name) - } else if !m.Timestamp.Std().After(startTime) { - t.Fatal("expected time to be after start time") + } else if !m.Timestamp.Std().Equal(startTime) { + t.Fatal("expected time to match start time", m.Timestamp.Std(), startTime) } // Get contract metrics. @@ -2243,8 +2243,8 @@ func TestBusRecordedMetrics(t *testing.T) { if len(cMetrics) != 1 { t.Fatalf("expected 1 metric, got %v", len(cMetrics)) - } else if m := cMetrics[0]; !startTime.Before(m.Timestamp.Std()) { - t.Fatalf("expected time to be after start time, got %v", m.Timestamp) + } else if m := cMetrics[0]; !startTime.Equal(m.Timestamp.Std()) { + t.Fatal("expected time to match start time", m.Timestamp.Std(), startTime) } else if m.ContractID == (types.FileContractID{}) { t.Fatal("expected non-zero FCID") } else if m.HostKey == (types.PublicKey{}) { From c7fb0500858c24187f605713d7694ea60b8a5cb5 Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Tue, 5 Dec 2023 15:33:38 +0100 Subject: [PATCH 3/6] stores: add TestNormaliseTimestamp --- stores/metrics.go | 3 ++- stores/metrics_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/stores/metrics.go b/stores/metrics.go index b16116d38..cf12dc57c 100644 --- a/stores/metrics.go +++ b/stores/metrics.go @@ -338,11 +338,12 @@ func (s *SQLStore) contractSetMetrics(ctx context.Context, start time.Time, n ui } func normaliseTimestamp(start time.Time, interval time.Duration, t unixTimeMS) unixTimeMS { + start = start.UTC() toNormalise := time.Time(t).UTC() if start.After(toNormalise) { return unixTimeMS(start) } - normalized := (toNormalise.UnixMilli()-start.UnixMilli())/int64(interval)*int64(interval) + start.UnixMilli() + normalized := (toNormalise.UnixMilli()-start.UnixMilli())/int64(interval.Milliseconds())*int64(interval.Milliseconds()) + start.UnixMilli() return unixTimeMS(time.UnixMilli(normalized).UTC()) } diff --git a/stores/metrics_test.go b/stores/metrics_test.go index 0494f0567..24df0d0ee 100644 --- a/stores/metrics_test.go +++ b/stores/metrics_test.go @@ -13,6 +13,40 @@ import ( "lukechampine.com/frand" ) +func TestNormaliseTimestamp(t *testing.T) { + tests := []struct { + start time.Time + interval time.Duration + ti time.Time + result time.Time + }{ + { + start: time.UnixMilli(100), + interval: 10 * time.Millisecond, + ti: time.UnixMilli(105), + result: time.UnixMilli(100), + }, + { + start: time.UnixMilli(100), + interval: 10 * time.Millisecond, + ti: time.UnixMilli(115), + result: time.UnixMilli(110), + }, + { + start: time.UnixMilli(100), + interval: 10 * time.Millisecond, + ti: time.UnixMilli(125), + result: time.UnixMilli(120), + }, + } + + for _, test := range tests { + if result := time.Time(normaliseTimestamp(test.start, test.interval, unixTimeMS(test.ti))); !result.Equal(test.result) { + t.Fatalf("expected %v, got %v", test.result, result) + } + } +} + func TestContractSetMetrics(t *testing.T) { testStart := time.Now().Round(time.Millisecond).UTC() ss := newTestSQLStore(t, defaultTestSQLStoreConfig) From f4d7ea012aad2905ee57c16a601af6d725fa59be Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Tue, 5 Dec 2023 16:33:13 +0100 Subject: [PATCH 4/6] stores: use FLOOR for MySQL --- stores/metrics.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/stores/metrics.go b/stores/metrics.go index cf12dc57c..558fa289f 100644 --- a/stores/metrics.go +++ b/stores/metrics.go @@ -356,8 +356,12 @@ func (s *SQLStore) findPeriods(tx *gorm.DB, dst interface{}, start time.Time, n end := start.Add(time.Duration(n) * interval) // inner groups all metrics within the requested time range into periods of // 'interval' length and gives us the min timestamp of each period. + floorExpr := "(timestamp - ?) / ? * ?" + if !isSQLite(s.dbMetrics) { + floorExpr = "FLOOR((timestamp - ?) / ?) * ?" + } inner := tx.Model(dst). - Select("MIN(timestamp) AS min_time, (timestamp - ?) / ? * ? AS period", unixTimeMS(start), interval.Milliseconds(), interval.Milliseconds()). + Select(fmt.Sprintf("MIN(timestamp) AS min_time, %s AS period", floorExpr), unixTimeMS(start), interval.Milliseconds(), interval.Milliseconds()). Where("timestamp >= ? AND timestamp < ?", unixTimeMS(start), unixTimeMS(end)). Group("period") // mid then joins the result with the original table. This might yield From 4029fd3bcba9c11076c9a083be3da6c644c0351b Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Tue, 5 Dec 2023 16:45:09 +0100 Subject: [PATCH 5/6] testing: fix TestBusRecordedMetrics --- internal/testing/cluster_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/testing/cluster_test.go b/internal/testing/cluster_test.go index fca36b99b..645db0bd0 100644 --- a/internal/testing/cluster_test.go +++ b/internal/testing/cluster_test.go @@ -2208,8 +2208,8 @@ func TestBusRecordedMetrics(t *testing.T) { t.Fatalf("expected 1 contract, got %v", m.Contracts) } else if m.Name != testContractSet { t.Fatalf("expected contract set %v, got %v", testContractSet, m.Name) - } else if !m.Timestamp.Std().Equal(startTime) { - t.Fatal("expected time to match start time", m.Timestamp.Std(), startTime) + } else if m.Timestamp.Std().Before(startTime) { + t.Fatalf("expected time to be after start time %v, got %v", startTime, m.Timestamp.Std()) } } @@ -2225,8 +2225,8 @@ func TestBusRecordedMetrics(t *testing.T) { t.Fatal("expected non-zero FCID") } else if m.Name != testContractSet { t.Fatalf("expected contract set %v, got %v", testContractSet, m.Name) - } else if !m.Timestamp.Std().Equal(startTime) { - t.Fatal("expected time to match start time", m.Timestamp.Std(), startTime) + } else if m.Timestamp.Std().Before(startTime) { + t.Fatalf("expected time to be after start time %v, got %v", startTime, m.Timestamp.Std()) } // Get contract metrics. @@ -2243,8 +2243,8 @@ func TestBusRecordedMetrics(t *testing.T) { if len(cMetrics) != 1 { t.Fatalf("expected 1 metric, got %v", len(cMetrics)) - } else if m := cMetrics[0]; !startTime.Equal(m.Timestamp.Std()) { - t.Fatal("expected time to match start time", m.Timestamp.Std(), startTime) + } else if m := cMetrics[0]; m.Timestamp.Std().Before(startTime) { + t.Fatalf("expected time to be after start time %v, got %v", startTime, m.Timestamp.Std()) } else if m.ContractID == (types.FileContractID{}) { t.Fatal("expected non-zero FCID") } else if m.HostKey == (types.PublicKey{}) { From 0853cd030a0171718277b7f64a5327698879b098 Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Wed, 6 Dec 2023 09:46:22 +0100 Subject: [PATCH 6/6] stores: refactor normaliseTimestamp --- stores/metrics.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/stores/metrics.go b/stores/metrics.go index 558fa289f..d6586cff2 100644 --- a/stores/metrics.go +++ b/stores/metrics.go @@ -338,13 +338,14 @@ func (s *SQLStore) contractSetMetrics(ctx context.Context, start time.Time, n ui } func normaliseTimestamp(start time.Time, interval time.Duration, t unixTimeMS) unixTimeMS { - start = start.UTC() - toNormalise := time.Time(t).UTC() - if start.After(toNormalise) { + startMS := start.UnixMilli() + toNormaliseMS := time.Time(t).UnixMilli() + intervalMS := interval.Milliseconds() + if startMS > toNormaliseMS { return unixTimeMS(start) } - normalized := (toNormalise.UnixMilli()-start.UnixMilli())/int64(interval.Milliseconds())*int64(interval.Milliseconds()) + start.UnixMilli() - return unixTimeMS(time.UnixMilli(normalized).UTC()) + normalizedMS := (toNormaliseMS-startMS)/intervalMS*intervalMS + start.UnixMilli() + return unixTimeMS(time.UnixMilli(normalizedMS)) } // findPeriods is the core of all methods retrieving metrics. By using integer