Skip to content

TRT-2741: Phase 2 - Replace prow_test_report matviews with cumulative summary queries #3769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mstaeble wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

TRT-2741: Phase 2 - Replace prow_test_report matviews with cumulative summary queries #3769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 0 additions & 73 deletions pkg/api/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,79 +164,6 @@ func isStructWithNoPublicFields(v interface{}) bool {
return true
}

// GetDataFromCacheOrMatview caches data that is based on a matview and invalidates it when the matview is refreshed
func GetDataFromCacheOrMatview[T any](ctx context.Context,
cacheClient cache.Cache, cacheSpec CacheSpec,
matview string,
cacheDuration time.Duration,
generateFn func(context.Context) (T, []error),
defaultVal T,
) (T, []error) {
if cacheClient == nil {
return generateFn(ctx)
}

cacheKey, err := cacheSpec.GetCacheKey()
if err != nil {
return defaultVal, []error{err}
}

// If someone gives us an uncacheable cacheKey, panic so it gets detected in testing
if len(cacheKey) == 0 {
panic(fmt.Sprintf("cache key is empty for %s", reflect.TypeOf(defaultVal)))
}
// If someone gives us an uncacheable value, panic so it gets detected in testing
if isStructWithNoPublicFields(defaultVal) {
panic(fmt.Sprintf("cannot cache type %s that exports no fields", reflect.TypeOf(defaultVal)))
}

var cacheVal struct {
Val T // the actual value we want to cache
Timestamp time.Time // the time when it was cached (for comparison to matview refresh time)
}
if cached, err := cacheClient.Get(ctx, string(cacheKey), 0); err == nil {
logrus.WithFields(logrus.Fields{
"key": string(cacheKey),
"type": reflect.TypeOf(defaultVal).String(),
}).Debugf("cache hit")

if err := json.Unmarshal(cached, &cacheVal); err != nil {
logrus.WithError(err).Warnf("failed to unmarshal cached item. cacheKey=%+v", cacheKey)
// fall through to generate the data instead
} else {
// look up when the matview was refreshed to see if the cached value is stale
var lastRefresh time.Time
if lastRefreshBytes, err := cacheClient.Get(ctx, RefreshMatviewKey(matview), 0); err == nil {
if parsed, err := time.Parse(time.RFC3339, string(lastRefreshBytes)); err != nil {
logrus.WithError(err).Warnf("failed to parse matview refresh timestamp %q for %q; cache will not be invalidated", lastRefreshBytes, matview)
} else {
lastRefresh = parsed
}
}

if lastRefresh.Before(cacheVal.Timestamp) {
// not invalidated by a newer refresh, so use it (if we don't know the last refresh, still use it)
return cacheVal.Val, nil
}
logrus.Debugf("matview %q refreshed at %v, will not use earlier cache entry from %v", matview, lastRefresh, cacheVal.Timestamp)
}
} else if strings.Contains(err.Error(), "connection refused") {
logrus.WithError(err).Fatalf("redis URL specified but got connection refused; exiting due to cost issues in this configuration")
} else {
logrus.WithFields(logrus.Fields{"key": string(cacheKey)}).Debugf("cache miss")
}

// Cache missed or refresh invalidated the data, so generate it.
logrus.Debugf("cache duration set to %s or approx %s for key %s", cacheDuration, time.Now().Add(cacheDuration).Format(time.RFC3339), cacheKey)
result, errs := generateFn(ctx)
if len(errs) == 0 {
cacheVal.Val = result
cacheVal.Timestamp = time.Now().UTC()
CacheSet(ctx, cacheClient, cacheVal, cacheKey, cacheDuration)
}
return result, errs
}

func RefreshMatviewKey(matview string) string {
return "matview_refreshed:" + matview
}
215 changes: 0 additions & 215 deletions pkg/api/cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,218 +508,3 @@ func TestNewCacheSpec_Prefix(t *testing.T) {
func timePtr(t time.Time) *time.Time {
return &t
}

const testMatview = "prow_test_report_7d_matview"

// helper to pre-populate the cache with a matview-style cached value (val + timestamp)
func seedMatviewCache(t *testing.T, mc *mockCache, spec CacheSpec, val testResult, cachedAt time.Time) {
t.Helper()
cacheKey, err := spec.GetCacheKey()
require.NoError(t, err)
entry := struct {
Val testResult
Timestamp time.Time
}{Val: val, Timestamp: cachedAt}
data, err := json.Marshal(entry)
require.NoError(t, err)
mc.store[string(cacheKey)] = data
}

func seedRefreshTimestamp(mc *mockCache, matview string, refreshedAt time.Time) {
mc.store[RefreshMatviewKey(matview)] = []byte(refreshedAt.UTC().Format(time.RFC3339))
}

// TestGetDataFromCacheOrMatview_NilCache verifies that with no cache, generateFn is always called.
func TestGetDataFromCacheOrMatview_NilCache(t *testing.T) {
var generateCalls int
expected := testResult{Value: "generated"}
spec := NewCacheSpec(testCacheKey{Query: "q1"}, "mv~", nil)

result, errs := GetDataFromCacheOrMatview(
context.Background(), nil, spec, testMatview, time.Hour,
makeGenerateFn(expected, &generateCalls), testResult{},
)

assert.Empty(t, errs)
assert.Equal(t, expected, result)
assert.Equal(t, 1, generateCalls)
}

// TestGetDataFromCacheOrMatview_CacheMiss verifies that on a miss, generateFn is called and the result is stored.
func TestGetDataFromCacheOrMatview_CacheMiss(t *testing.T) {
mc := newMockCache()
var generateCalls int
expected := testResult{Value: "generated"}
spec := NewCacheSpec(testCacheKey{Query: "q1"}, "mv~", nil)

result, errs := GetDataFromCacheOrMatview(
context.Background(), mc, spec, testMatview, time.Hour,
makeGenerateFn(expected, &generateCalls), testResult{},
)

assert.Empty(t, errs)
assert.Equal(t, expected, result)
assert.Equal(t, 1, generateCalls)
assert.Equal(t, 1, mc.setCalls, "should store result in cache")
}

// TestGetDataFromCacheOrMatview_CacheHit_NoRefresh verifies that a cached value is returned
// when no matview refresh timestamp exists in the cache.
func TestGetDataFromCacheOrMatview_CacheHit_NoRefresh(t *testing.T) {
mc := newMockCache()
spec := NewCacheSpec(testCacheKey{Query: "q1"}, "mv~", nil)
cached := testResult{Value: "cached"}
seedMatviewCache(t, mc, spec, cached, time.Now().UTC())

var generateCalls int
result, errs := GetDataFromCacheOrMatview(
context.Background(), mc, spec, testMatview, time.Hour,
makeGenerateFn(testResult{Value: "fresh"}, &generateCalls), testResult{},
)

assert.Empty(t, errs)
assert.Equal(t, cached, result, "should return cached value when no refresh timestamp exists")
assert.Equal(t, 0, generateCalls, "should not call generateFn")
}

// TestGetDataFromCacheOrMatview_CacheHit_RefreshBeforeCacheTime verifies that a cached value
// is returned when the matview was refreshed before the data was cached.
func TestGetDataFromCacheOrMatview_CacheHit_RefreshBeforeCacheTime(t *testing.T) {
mc := newMockCache()
spec := NewCacheSpec(testCacheKey{Query: "q1"}, "mv~", nil)
cachedAt := time.Now().UTC()
cached := testResult{Value: "cached"}
seedMatviewCache(t, mc, spec, cached, cachedAt)
// Matview was refreshed 10 minutes before the data was cached
seedRefreshTimestamp(mc, testMatview, cachedAt.Add(-10*time.Minute))

var generateCalls int
result, errs := GetDataFromCacheOrMatview(
context.Background(), mc, spec, testMatview, time.Hour,
makeGenerateFn(testResult{Value: "fresh"}, &generateCalls), testResult{},
)

assert.Empty(t, errs)
assert.Equal(t, cached, result, "should return cached value when refresh predates cache entry")
assert.Equal(t, 0, generateCalls, "should not call generateFn")
}

// TestGetDataFromCacheOrMatview_CacheInvalidated_RefreshAfterCacheTime verifies that when the
// matview was refreshed after the data was cached, the cached value is invalidated and
// generateFn is called to produce fresh data.
func TestGetDataFromCacheOrMatview_CacheInvalidated_RefreshAfterCacheTime(t *testing.T) {
mc := newMockCache()
spec := NewCacheSpec(testCacheKey{Query: "q1"}, "mv~", nil)
cachedAt := time.Now().UTC().Add(-5 * time.Minute)
seedMatviewCache(t, mc, spec, testResult{Value: "stale"}, cachedAt)
// Matview was refreshed 1 minute ago, after the data was cached
seedRefreshTimestamp(mc, testMatview, time.Now().UTC().Add(-1*time.Minute))

var generateCalls int
expected := testResult{Value: "fresh"}
result, errs := GetDataFromCacheOrMatview(
context.Background(), mc, spec, testMatview, time.Hour,
makeGenerateFn(expected, &generateCalls), testResult{},
)

assert.Empty(t, errs)
assert.Equal(t, expected, result, "should regenerate data after matview refresh")
assert.Equal(t, 1, generateCalls, "should call generateFn when cache is invalidated")
assert.GreaterOrEqual(t, mc.setCalls, 1, "should store the fresh result")
}

// TestGetDataFromCacheOrMatview_GenerateErrorSkipsCacheWrite verifies that errors from
// generateFn are not cached.
func TestGetDataFromCacheOrMatview_GenerateErrorSkipsCacheWrite(t *testing.T) {
mc := newMockCache()
spec := NewCacheSpec(testCacheKey{Query: "q1"}, "mv~", nil)

var generateCalls int
result, errs := GetDataFromCacheOrMatview(
context.Background(), mc, spec, testMatview, time.Hour,
makeFailingGenerateFn(&generateCalls), testResult{},
)

assert.Len(t, errs, 1)
assert.Equal(t, testResult{}, result)
assert.Equal(t, 1, generateCalls)
assert.Equal(t, 0, mc.setCalls, "should not cache error results")
}

// TestGetDataFromCacheOrMatview_InvalidRefreshTimestamp verifies that an unparseable refresh
// timestamp in the cache is treated like no refresh (cache is still used).
func TestGetDataFromCacheOrMatview_InvalidRefreshTimestamp(t *testing.T) {
mc := newMockCache()
spec := NewCacheSpec(testCacheKey{Query: "q1"}, "mv~", nil)
cached := testResult{Value: "cached"}
seedMatviewCache(t, mc, spec, cached, time.Now().UTC())
// Store garbage as the refresh timestamp
mc.store[RefreshMatviewKey(testMatview)] = []byte("not-a-timestamp")

var generateCalls int
result, errs := GetDataFromCacheOrMatview(
context.Background(), mc, spec, testMatview, time.Hour,
makeGenerateFn(testResult{Value: "fresh"}, &generateCalls), testResult{},
)

assert.Empty(t, errs)
assert.Equal(t, cached, result, "should return cached value when refresh timestamp is unparseable")
assert.Equal(t, 0, generateCalls)
}

// TestGetDataFromCacheOrMatview_CacheDurationPassedToSet verifies that the specified
// cacheDuration is used when writing to the cache.
func TestGetDataFromCacheOrMatview_CacheDurationPassedToSet(t *testing.T) {
mc := newMockCache()
spec := NewCacheSpec(testCacheKey{Query: "q1"}, "mv~", nil)
expectedDuration := 2 * time.Hour

var generateCalls int
_, errs := GetDataFromCacheOrMatview(
context.Background(), mc, spec, testMatview, expectedDuration,
makeGenerateFn(testResult{Value: "data"}, &generateCalls), testResult{},
)

assert.Empty(t, errs)
cacheKey, _ := spec.GetCacheKey()
assert.Equal(t, expectedDuration, mc.setDurations[string(cacheKey)], "should use specified cache duration")
}

// TestGetDataFromCacheOrMatview_DifferentMatviews verifies that cache entries for different
// matviews are invalidated independently.
func TestGetDataFromCacheOrMatview_DifferentMatviews(t *testing.T) {
mc := newMockCache()
matview7d := "prow_test_report_7d_matview"
matview2d := "prow_test_report_2d_matview"
spec7d := NewCacheSpec(testCacheKey{Query: "7d"}, "mv~", nil)
spec2d := NewCacheSpec(testCacheKey{Query: "2d"}, "mv~", nil)

cachedAt := time.Now().UTC().Add(-5 * time.Minute)
seedMatviewCache(t, mc, spec7d, testResult{Value: "cached-7d"}, cachedAt)
seedMatviewCache(t, mc, spec2d, testResult{Value: "cached-2d"}, cachedAt)

// Only refresh the 7d matview (after caching)
seedRefreshTimestamp(mc, matview7d, time.Now().UTC().Add(-1*time.Minute))
// 2d matview was refreshed before caching
seedRefreshTimestamp(mc, matview2d, cachedAt.Add(-10*time.Minute))

var gen7dCalls, gen2dCalls int

// 7d should be invalidated
result7d, errs := GetDataFromCacheOrMatview(
context.Background(), mc, spec7d, matview7d, time.Hour,
makeGenerateFn(testResult{Value: "fresh-7d"}, &gen7dCalls), testResult{},
)
assert.Empty(t, errs)
assert.Equal(t, "fresh-7d", result7d.Value, "7d cache should be invalidated")
assert.Equal(t, 1, gen7dCalls)

// 2d should still be cached
result2d, errs := GetDataFromCacheOrMatview(
context.Background(), mc, spec2d, matview2d, time.Hour,
makeGenerateFn(testResult{Value: "fresh-2d"}, &gen2dCalls), testResult{},
)
assert.Empty(t, errs)
assert.Equal(t, "cached-2d", result2d.Value, "2d cache should not be invalidated")
assert.Equal(t, 0, gen2dCalls)
}
49 changes: 10 additions & 39 deletions pkg/api/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package api
import (
"encoding/json"
"net/http"
"strings"

log "github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -57,54 +56,26 @@ func PrintInstallJSONReportFromDB(w http.ResponseWriter, dbc *db.DB, release str
// VariantTestsReport returns a set of all variant columns plus "All", and a map of testName to variant column to test results for that variant.
// Caller can provide exact test names to match, test name prefixes, or test substrings.
func VariantTestsReport(dbc *db.DB, release string, reportType v1.ReportType, testNames, testPrefixes, testSubStrings sets.Set[string], excludedVariants []string) (sets.Set[string], map[string]map[string]apitype.Test, error) {
nameMatches := query.TestNameMatches{
ExactNames: sets.List(testNames),
Prefixes: sets.List(testPrefixes),
Substrings: sets.List(testSubStrings),
}

// Build a list of all sub-strings to search for; we'll sort out exact matches later as these
// can pickup unintended tests.
testSearchStrings := sets.List(testNames.Clone().Union(testPrefixes).Union(testSubStrings))

testReports, err := query.TestReportsByVariant(dbc, release, reportType, testSearchStrings, excludedVariants)
testReports, err := query.TestReportsByVariant(dbc, release, reportType, nameMatches, excludedVariants, true)
if err != nil {
return sets.New[string](), map[string]map[string]apitype.Test{}, err
}

variantColumns := sets.New[string]()
variantColumns.Insert("All") // Insert the default overall "All" column we also display with the variants.
tests := make(map[string]map[string]apitype.Test)

for _, tr := range testReports {
var prefixMatches bool
var subStringMatches bool
for _, prefix := range testPrefixes.UnsortedList() {
if strings.HasPrefix(tr.Name, prefix) {
prefixMatches = true
break
}
}
for _, subString := range testSubStrings.UnsortedList() {
if strings.HasPrefix(tr.Name, subString) {
subStringMatches = true
break
}
}

switch {
case testNames.Has(tr.Name) || prefixMatches || subStringMatches:
variantColumns.Insert(tr.Variant)
if _, ok := tests[tr.Name]; !ok {
tests[tr.Name] = map[string]apitype.Test{}
}
tests[tr.Name][tr.Variant] = tr
default:
// Our substring searching can pickup unintended tests:
log.Infof("Ignoring test %s for variant %s", tr.Name, tr.Variant)
}
}

// Add in the All column for each test:
for testName := range tests {
if allReport, found := query.TestReportExcludeVariants(dbc, release, testName, excludedVariants); found {
tests[testName]["All"] = allReport
variantColumns.Insert(tr.Variant)
if _, ok := tests[tr.Name]; !ok {
tests[tr.Name] = map[string]apitype.Test{}
}
tests[tr.Name][tr.Variant] = tr
}

return variantColumns, tests, nil
Expand Down
Loading