diff --git a/pkg/api/cache.go b/pkg/api/cache.go index a2c379e8de..791460af7f 100644 --- a/pkg/api/cache.go +++ b/pkg/api/cache.go @@ -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 } diff --git a/pkg/api/cache_test.go b/pkg/api/cache_test.go index ebdc268ea8..335e2455e9 100644 --- a/pkg/api/cache_test.go +++ b/pkg/api/cache_test.go @@ -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) -} diff --git a/pkg/api/install.go b/pkg/api/install.go index 0862e63adf..6375ee140d 100644 --- a/pkg/api/install.go +++ b/pkg/api/install.go @@ -3,7 +3,6 @@ package api import ( "encoding/json" "net/http" - "strings" log "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/util/sets" @@ -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 diff --git a/pkg/api/tests.go b/pkg/api/tests.go index f489a93914..2095f987f6 100644 --- a/pkg/api/tests.go +++ b/pkg/api/tests.go @@ -16,13 +16,14 @@ import ( pkgerrors "github.com/pkg/errors" log "github.com/sirupsen/logrus" "google.golang.org/api/iterator" + "gorm.io/gorm" apitype "github.com/openshift/sippy/pkg/apis/api" "github.com/openshift/sippy/pkg/apis/cache" + v1 "github.com/openshift/sippy/pkg/apis/sippyprocessing/v1" bq "github.com/openshift/sippy/pkg/bigquery" "github.com/openshift/sippy/pkg/bigquery/bqlabel" "github.com/openshift/sippy/pkg/db" - "github.com/openshift/sippy/pkg/db/models" "github.com/openshift/sippy/pkg/db/query" "github.com/openshift/sippy/pkg/filter" "github.com/openshift/sippy/pkg/html/installhtml" @@ -30,10 +31,6 @@ import ( ) const ( - testReport7dMatView = "prow_test_report_7d_matview" - testReport2dMatView = "prow_test_report_2d_matview" - testReport7dCollapsedMatView = "prow_test_report_7d_collapsed_matview" - testReport2dCollapsedMatView = "prow_test_report_2d_collapsed_matview" payloadFailedTests14dMatView = "payload_test_failures_14d_matview" ) @@ -428,17 +425,13 @@ type testResults struct { const testResultsCacheDuration = time.Hour func (spec *TestResultsSpec) buildTestsResultsFromPostgres(ctx context.Context, dbc *db.DB, cacheClient cache.Cache) (testResults, error) { - matview := testReport7dMatView - if spec.Period == "twoDay" { - matview = testReport2dMatView - } + sample, base := query.PeriodsForReportType(v1.ReportType(spec.Period)) generator := func(ctx context.Context) (testResults, []error) { - return spec.buildTestsResultsPGGenerator(ctx, dbc, matview) + return spec.buildTestsResultsPGGenerator(ctx, dbc, sample, base) } - result, errs := GetDataFromCacheOrMatview(ctx, cacheClient, + result, errs := GetDataFromCacheOrGenerate(ctx, cacheClient, cache.RequestOptions{Expiry: testResultsCacheDuration}, NewCacheSpec(spec, "PostgresTestsResults~", nil), - matview, testResultsCacheDuration, generator, testResults{}, ) @@ -448,46 +441,9 @@ func (spec *TestResultsSpec) buildTestsResultsFromPostgres(ctx context.Context, return result, nil } -// variantFiltersCoveredByCollapsedMatview returns true if all variant filter items -// are "NOT has entry" exclusions for values that are already pre-excluded in the -// collapsed matview. When true, these filters can be dropped and the collapsed -// matview used directly. -// variantFiltersCoveredByCollapsedMatview returns true if the variant filters -// exactly match the exclusions baked into the collapsed matview. Every filter -// item must be a "NOT has entry" for one of the pre-excluded values, and every -// pre-excluded value must have a corresponding filter item. -func variantFiltersCoveredByCollapsedMatview(variantFilter *filter.Filter) bool { - if variantFilter == nil || len(variantFilter.Items) == 0 { - return false - } - - matched := make(map[string]bool) - for _, item := range variantFilter.Items { - if !item.Not || item.Operator != filter.OperatorHasEntry { - return false - } - found := false - for _, excluded := range db.CollapsedVariantExclusions { - if item.Value == excluded { - matched[excluded] = true - found = true - break - } - } - if !found { - return false - } - } - - return len(matched) == len(db.CollapsedVariantExclusions) -} - -func (spec *TestResultsSpec) buildTestsResultsPGGenerator(ctx context.Context, dbc *db.DB, matview string) (result testResults, errs []error) { +func (spec *TestResultsSpec) buildTestsResultsPGGenerator(ctx context.Context, dbc *db.DB, sample, base query.DateRange) (result testResults, errs []error) { now := time.Now() - // Test results are generated by using two subqueries, which need to be filtered separately. Once during - // pre-processing where we're evaluating summed variant results, and in post-processing after we've - // assembled our final temporary table. var nameFilter, variantFilter, processedFilter *filter.Filter if spec.Filter != nil { var rawFilter *filter.Filter @@ -495,91 +451,71 @@ func (spec *TestResultsSpec) buildTestsResultsPGGenerator(ctx context.Context, d nameFilter, variantFilter = rawFilter.Split([]string{"name"}) } - // When collapsing and the variant filters are only the default exclusions (never-stable, - // aggregated), use the pre-filtered collapsed matview which has these exclusions baked in - // and the GROUP BY pre-computed. This reduces query time from ~5s to ~430ms. - useCollapsedMatview := spec.Collapse && variantFiltersCoveredByCollapsedMatview(variantFilter) - if useCollapsedMatview { - collapsedMatview := testReport7dCollapsedMatView - if spec.Period == "twoDay" { - collapsedMatview = testReport2dCollapsedMatView - } - matview = collapsedMatview - } - - rawQuery := dbc.DB.WithContext(ctx). - Table(matview). - Where("release = ?", spec.Release) - - // Collapse groups the test results together -- otherwise we return the test results per-variant combo (NURP+) testMetadataColumns := []string{"suite_name", "name", "jira_component", "jira_component_id"} - var variantColumns []string + + var finalResults *gorm.DB + workMem := "4MB" if spec.Collapse { - if useCollapsedMatview { - rawQuery = rawQuery.Select(strings.Join(append(testMetadataColumns, query.QueryTestFields), ",")) - } else { - rawQuery = rawQuery.Select(strings.Join(append(testMetadataColumns, query.QueryTestSummer), ",")).Group(strings.Join(testMetadataColumns, ",")) + workMem = "16MB" + + collapsedQuery, err := query.TestReportQueryCollapsed(dbc, spec.Release, sample, base, variantFilter, nameFilter) + if err != nil { + errs = append(errs, err) + return } + collapsedColumns := append([]string{"id"}, append(testMetadataColumns, query.QueryTestFields)...) + rawQuery := dbc.DB.WithContext(ctx). + Table("(?) AS r", collapsedQuery). + Select(strings.Join(collapsedColumns, ",")) + + // Collapsed path: inner query only has counts, so the outer layer computes + // percentages and net improvements via QueryTestSummarizer. + selectColumns := []string{"id"} + selectColumns = append(selectColumns, testMetadataColumns...) + selectColumns = append(selectColumns, query.QueryTestSummarizer) + processedResults := dbc.DB.Table("(?) as results", rawQuery). + Select(strings.Join(selectColumns, ",")). + Where("current_runs > 0 or previous_runs > 0") + finalResults = dbc.DB.Table("(?) as final_results", processedResults) } else { - rawQuery = query.TestsByNURPAndStandardDeviation(dbc, spec.Release, matview) - variantColumns = []string{ - "suite_name", "variants", - "delta_from_working_average", "working_average", "working_standard_deviation", - "delta_from_passing_average", "passing_average", "passing_standard_deviation", - "delta_from_flake_average", "flake_average", "flake_standard_deviation", + rawQuery, remainingFilter, err := query.UncollapsedTestReportWithStats(dbc, spec.Release, sample, base, nameFilter, variantFilter, processedFilter) + if err != nil { + errs = append(errs, err) + return } - } - - // Apply name and variant filters via dimension table lookups rather than directly on - // the matview. Running ILIKE against the small tests table (~36K rows) and joining by - // ID is much faster than running ILIKE against the large matview (~700K+ rows per release). - if nameFilter != nil && len(nameFilter.Items) > 0 { - testSubquery := nameFilter.ToSQL(dbc.DB.Model(&models.Test{}).Select("id"), apitype.Test{}) - rawQuery = rawQuery.Where("id IN (?)", testSubquery) - } - // Variant filters are already applied in the collapsed matview; only apply them - // when using the base matview. - if !useCollapsedMatview && variantFilter != nil && len(variantFilter.Items) > 0 { - rawQuery = variantFilter.ToSQL(rawQuery, apitype.Test{}) + processedFilter = remainingFilter + finalResults = dbc.DB.Table("(?) as final_results", rawQuery) } testReports := make([]apitype.Test, 0) - // FIXME: Add test id to matview, for now generate with ROW_NUMBER OVER - selectColumns := []string{"ROW_NUMBER() OVER() as id"} - selectColumns = append(selectColumns, testMetadataColumns...) - selectColumns = append(selectColumns, variantColumns...) - selectColumns = append(selectColumns, query.QueryTestSummarizer) - processedResults := dbc.DB.Table("(?) as results", rawQuery). - Select(strings.Join(selectColumns, ",")). - Where("current_runs > 0 or previous_runs > 0") - - finalResults := dbc.DB.Table("(?) as final_results", processedResults) + if processedFilter != nil { finalResults = processedFilter.ToSQL(finalResults, apitype.Test{}) } - frr := finalResults.Scan(&testReports) - if frr.Error != nil { - log.WithError(finalResults.Error).Error("error querying test reports") + // The global connection-level work_mem (128MB) causes the planner to + // choose sort-based plans for the large GROUP BY in the prefix sum join. + // A lower work_mem lets it choose parallel HashAggregates that fit in + // memory. SET LOCAL is scoped to the transaction. All scan paths use + // tx.Table() to ensure the query runs on the same connection. + scanErr := dbc.DB.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + if err := tx.Exec("SET LOCAL work_mem = '" + workMem + "'").Error; err != nil { + return err + } + return tx.Table("(?) AS q", finalResults).Scan(&testReports).Error + }) + if scanErr != nil { + log.WithError(scanErr).Error("error querying test reports") result.TestsAPIResult = []apitype.Test{} - errs = append(errs, frr.Error) + errs = append(errs, scanErr) return } - // Produce a special "overall" test that has a summary of all the selected tests. + // Produce a special "overall" test that has a summary of all the selected tests + // by aggregating the already-scanned results in Go, avoiding a second full query. var overallTest *apitype.Test if spec.IncludeOverall { - finalResults := dbc.DB.Table("(?) as final_results", finalResults) - finalResults = finalResults.Select(query.QueryTestSummer) - summaryResult := dbc.DB.Table("(?) as overall", finalResults).Select(query.QueryTestSummarizer) - overallTest = &apitype.Test{ - ID: math.MaxInt32, - Name: "Overall", - } - // TODO: column open_bugs does not exist here? - if err := summaryResult.Scan(overallTest).Error; err != nil { - errs = append(errs, err) - } + overallTest = computeOverallTest(testReports) } elapsed := time.Since(now) @@ -593,6 +529,43 @@ func (spec *TestResultsSpec) buildTestsResultsPGGenerator(ctx context.Context, d return } +func safePercent(numerator, denominator int) float64 { + if denominator == 0 { + return 0 + } + return float64(numerator) * 100.0 / float64(denominator) +} + +func computeOverallTest(testReports []apitype.Test) *apitype.Test { + overall := &apitype.Test{ + ID: math.MaxInt32, + Name: "Overall", + } + for _, t := range testReports { + overall.CurrentRuns += t.CurrentRuns + overall.CurrentSuccesses += t.CurrentSuccesses + overall.CurrentFailures += t.CurrentFailures + overall.CurrentFlakes += t.CurrentFlakes + overall.PreviousRuns += t.PreviousRuns + overall.PreviousSuccesses += t.PreviousSuccesses + overall.PreviousFailures += t.PreviousFailures + overall.PreviousFlakes += t.PreviousFlakes + } + overall.CurrentPassPercentage = safePercent(overall.CurrentSuccesses, overall.CurrentRuns) + overall.CurrentFailurePercentage = safePercent(overall.CurrentFailures, overall.CurrentRuns) + overall.CurrentFlakePercentage = safePercent(overall.CurrentFlakes, overall.CurrentRuns) + overall.CurrentWorkingPercentage = safePercent(overall.CurrentSuccesses+overall.CurrentFlakes, overall.CurrentRuns) + overall.PreviousPassPercentage = safePercent(overall.PreviousSuccesses, overall.PreviousRuns) + overall.PreviousFailurePercentage = safePercent(overall.PreviousFailures, overall.PreviousRuns) + overall.PreviousFlakePercentage = safePercent(overall.PreviousFlakes, overall.PreviousRuns) + overall.PreviousWorkingPercentage = safePercent(overall.PreviousSuccesses+overall.PreviousFlakes, overall.PreviousRuns) + overall.NetFailureImprovement = overall.PreviousFailurePercentage - overall.CurrentFailurePercentage + overall.NetFlakeImprovement = overall.PreviousFlakePercentage - overall.CurrentFlakePercentage + overall.NetWorkingImprovement = overall.CurrentWorkingPercentage - overall.PreviousWorkingPercentage + overall.NetImprovement = overall.CurrentPassPercentage - overall.PreviousPassPercentage + return overall +} + type testResultsBQ struct { TestsAPIResultBQ Test *apitype.TestBQ diff --git a/pkg/api/tests_test.go b/pkg/api/tests_test.go new file mode 100644 index 0000000000..d8c1ee0730 --- /dev/null +++ b/pkg/api/tests_test.go @@ -0,0 +1,152 @@ +package api + +import ( + "testing" + + apitype "github.com/openshift/sippy/pkg/apis/api" +) + +func TestComputeOverallTest(t *testing.T) { + tests := []struct { + name string + input []apitype.Test + expected apitype.Test + }{ + { + name: "empty input", + input: []apitype.Test{}, + expected: apitype.Test{ + Name: "Overall", + }, + }, + { + name: "zero runs", + input: []apitype.Test{ + {CurrentRuns: 0, PreviousRuns: 0}, + }, + expected: apitype.Test{ + Name: "Overall", + }, + }, + { + name: "single test", + input: []apitype.Test{ + { + CurrentRuns: 100, CurrentSuccesses: 90, CurrentFailures: 8, CurrentFlakes: 2, + PreviousRuns: 80, PreviousSuccesses: 70, PreviousFailures: 5, PreviousFlakes: 5, + }, + }, + expected: apitype.Test{ + Name: "Overall", + CurrentRuns: 100, + CurrentSuccesses: 90, + CurrentFailures: 8, + CurrentFlakes: 2, + CurrentPassPercentage: 90.0, + CurrentFailurePercentage: 8.0, + CurrentFlakePercentage: 2.0, + CurrentWorkingPercentage: 92.0, + PreviousRuns: 80, + PreviousSuccesses: 70, + PreviousFailures: 5, + PreviousFlakes: 5, + PreviousPassPercentage: 87.5, + PreviousFailurePercentage: 6.25, + PreviousFlakePercentage: 6.25, + PreviousWorkingPercentage: 93.75, + NetFailureImprovement: -1.75, + NetFlakeImprovement: 4.25, + NetWorkingImprovement: -1.75, + NetImprovement: 2.5, + }, + }, + { + name: "multiple tests aggregated", + input: []apitype.Test{ + { + CurrentRuns: 100, CurrentSuccesses: 90, CurrentFailures: 8, CurrentFlakes: 2, + PreviousRuns: 100, PreviousSuccesses: 80, PreviousFailures: 15, PreviousFlakes: 5, + }, + { + CurrentRuns: 100, CurrentSuccesses: 95, CurrentFailures: 3, CurrentFlakes: 2, + PreviousRuns: 100, PreviousSuccesses: 95, PreviousFailures: 3, PreviousFlakes: 2, + }, + }, + expected: apitype.Test{ + Name: "Overall", + CurrentRuns: 200, + CurrentSuccesses: 185, + CurrentFailures: 11, + CurrentFlakes: 4, + CurrentPassPercentage: 92.5, + CurrentFailurePercentage: 5.5, + CurrentFlakePercentage: 2.0, + CurrentWorkingPercentage: 94.5, + PreviousRuns: 200, + PreviousSuccesses: 175, + PreviousFailures: 18, + PreviousFlakes: 7, + PreviousPassPercentage: 87.5, + PreviousFailurePercentage: 9.0, + PreviousFlakePercentage: 3.5, + PreviousWorkingPercentage: 91.0, + NetFailureImprovement: 3.5, + NetFlakeImprovement: 1.5, + NetWorkingImprovement: 3.5, + NetImprovement: 5.0, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := computeOverallTest(tc.input) + if result.Name != tc.expected.Name { + t.Errorf("Name = %q, want %q", result.Name, tc.expected.Name) + } + if result.CurrentRuns != tc.expected.CurrentRuns { + t.Errorf("CurrentRuns = %d, want %d", result.CurrentRuns, tc.expected.CurrentRuns) + } + if result.CurrentSuccesses != tc.expected.CurrentSuccesses { + t.Errorf("CurrentSuccesses = %d, want %d", result.CurrentSuccesses, tc.expected.CurrentSuccesses) + } + if result.CurrentFailures != tc.expected.CurrentFailures { + t.Errorf("CurrentFailures = %d, want %d", result.CurrentFailures, tc.expected.CurrentFailures) + } + if result.CurrentFlakes != tc.expected.CurrentFlakes { + t.Errorf("CurrentFlakes = %d, want %d", result.CurrentFlakes, tc.expected.CurrentFlakes) + } + if result.PreviousRuns != tc.expected.PreviousRuns { + t.Errorf("PreviousRuns = %d, want %d", result.PreviousRuns, tc.expected.PreviousRuns) + } + if result.PreviousSuccesses != tc.expected.PreviousSuccesses { + t.Errorf("PreviousSuccesses = %d, want %d", result.PreviousSuccesses, tc.expected.PreviousSuccesses) + } + if result.PreviousFailures != tc.expected.PreviousFailures { + t.Errorf("PreviousFailures = %d, want %d", result.PreviousFailures, tc.expected.PreviousFailures) + } + if result.PreviousFlakes != tc.expected.PreviousFlakes { + t.Errorf("PreviousFlakes = %d, want %d", result.PreviousFlakes, tc.expected.PreviousFlakes) + } + assertFloat(t, "CurrentPassPercentage", result.CurrentPassPercentage, tc.expected.CurrentPassPercentage) + assertFloat(t, "CurrentFailurePercentage", result.CurrentFailurePercentage, tc.expected.CurrentFailurePercentage) + assertFloat(t, "CurrentFlakePercentage", result.CurrentFlakePercentage, tc.expected.CurrentFlakePercentage) + assertFloat(t, "CurrentWorkingPercentage", result.CurrentWorkingPercentage, tc.expected.CurrentWorkingPercentage) + assertFloat(t, "PreviousPassPercentage", result.PreviousPassPercentage, tc.expected.PreviousPassPercentage) + assertFloat(t, "PreviousFailurePercentage", result.PreviousFailurePercentage, tc.expected.PreviousFailurePercentage) + assertFloat(t, "PreviousFlakePercentage", result.PreviousFlakePercentage, tc.expected.PreviousFlakePercentage) + assertFloat(t, "PreviousWorkingPercentage", result.PreviousWorkingPercentage, tc.expected.PreviousWorkingPercentage) + assertFloat(t, "NetFailureImprovement", result.NetFailureImprovement, tc.expected.NetFailureImprovement) + assertFloat(t, "NetFlakeImprovement", result.NetFlakeImprovement, tc.expected.NetFlakeImprovement) + assertFloat(t, "NetWorkingImprovement", result.NetWorkingImprovement, tc.expected.NetWorkingImprovement) + assertFloat(t, "NetImprovement", result.NetImprovement, tc.expected.NetImprovement) + }) + } +} + +func assertFloat(t *testing.T, name string, got, want float64) { + t.Helper() + if got != want { + t.Errorf("%s = %f, want %f", name, got, want) + } +} diff --git a/pkg/dataloader/prowloader/prow.go b/pkg/dataloader/prowloader/prow.go index d89b46a301..6f06e3084f 100644 --- a/pkg/dataloader/prowloader/prow.go +++ b/pkg/dataloader/prowloader/prow.go @@ -1471,7 +1471,7 @@ func (pl *ProwLoader) fetchPullRequestData(refs *prow.Refs, pjPath string) []pul } if pr.Link == "" { - log.Debugf("skipping pull request with empty link for sha: %s", pr.SHA) + log.WithField("sha", pr.SHA).Debug("skipping pull request with empty link") continue } diff --git a/pkg/db/cumulativesummary/cumulative_summary.go b/pkg/db/cumulativesummary/cumulative_summary.go index a5da5a8e5a..2b4a2bfcf6 100644 --- a/pkg/db/cumulativesummary/cumulative_summary.go +++ b/pkg/db/cumulativesummary/cumulative_summary.go @@ -61,6 +61,9 @@ func doRefreshWithToday(store summaryStore, earliestChanged, today civil.Date) ( } func refreshDateRange(store summaryStore, startDate, endDate civil.Date) (civil.Date, error) { + if startDate.After(endDate) { + return startDate, nil + } loadStart := time.Now() days := endDate.DaysSince(startDate) + 1 diff --git a/pkg/db/query/cumulative_query.go b/pkg/db/query/cumulative_query.go new file mode 100644 index 0000000000..a9054950b8 --- /dev/null +++ b/pkg/db/query/cumulative_query.go @@ -0,0 +1,429 @@ +package query + +import ( + "fmt" + "strings" + "time" + + "cloud.google.com/go/civil" + "github.com/lib/pq" + log "github.com/sirupsen/logrus" + "gorm.io/gorm" + "k8s.io/apimachinery/pkg/util/sets" + + v1 "github.com/openshift/sippy/pkg/apis/sippyprocessing/v1" + "github.com/openshift/sippy/pkg/db" + "github.com/openshift/sippy/pkg/filter" +) + +// TestNameMatches specifies which tests to include in a report query. +// When all fields are empty, all tests are included. +type TestNameMatches struct { + ExactNames []string // matched with = (exact equality) + Prefixes []string // matched with LIKE 'prefix%' + Substrings []string // matched with LIKE '%substring%' +} + +// HasConditions returns true when at least one match criterion is set. +func (m TestNameMatches) HasConditions() bool { + return len(m.ExactNames) > 0 || len(m.Prefixes) > 0 || len(m.Substrings) > 0 +} + +// nameFilterConditions converts a name filter into SQL conditions and args. +// Positive items produce =, LIKE, ILIKE conditions; negative items (Not=true) +// produce NOT(...) wrapped versions of the same. +func nameFilterConditions(f *filter.Filter) (conditions []string, args []any) { + if f == nil || len(f.Items) == 0 { + return nil, nil + } + var positive, negative TestNameMatches + for _, item := range f.Items { + target := &positive + if item.Not { + target = &negative + } + switch item.Operator { + case filter.OperatorEquals: + target.ExactNames = append(target.ExactNames, item.Value) + case filter.OperatorStartsWith: + target.Prefixes = append(target.Prefixes, item.Value) + case filter.OperatorContains: + target.Substrings = append(target.Substrings, item.Value) + } + } + conditions, args = nameMatchConditions(positive) + negConds, negArgs := nameMatchConditions(negative) + conditions = append(conditions, negateConditions(negConds)...) + args = append(args, negArgs...) + return conditions, args +} + +// nameMatchConditions returns SQL conditions and args for a TestNameMatches. +func nameMatchConditions(matches TestNameMatches) (conditions []string, args []any) { + for _, name := range matches.ExactNames { + conditions = append(conditions, "tests.name = ?") + args = append(args, name) + } + for _, prefix := range matches.Prefixes { + conditions = append(conditions, "tests.name LIKE ?") + args = append(args, escapeLikeMetachars(prefix)+"%") + } + for _, sub := range matches.Substrings { + conditions = append(conditions, "tests.name ILIKE ?") + args = append(args, "%"+escapeLikeMetachars(sub)+"%") + } + return conditions, args +} + +func negateConditions(conditions []string) []string { + negated := make([]string, len(conditions)) + for i, c := range conditions { + negated[i] = "NOT(" + c + ")" + } + return negated +} + +// buildTestsJoinCondition constructs a tests JOIN clause from the match criteria. +// An empty TestNameMatches produces a plain JOIN with no filter. Conditions are +// combined with OR. +func buildTestsJoinCondition(matches TestNameMatches) (string, []any) { + const testsJoinClause = "JOIN tests ON tests.id = e.test_id" + conditions, args := nameMatchConditions(matches) + if len(conditions) == 0 { + return testsJoinClause, nil + } + return testsJoinClause + " AND (" + strings.Join(conditions, " OR ") + ")", args +} + +// escapeLikeMetachars escapes LIKE/ILIKE metacharacters (%, _, \) so they +// match literally. +func escapeLikeMetachars(s string) string { + r := strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`) + return r.Replace(s) +} + +// DateRange defines a half-open date interval [Start, End) used to compute +// period counts from prefix sums in test_cumulative_summaries. +// +// Given prefix sums P(d) = cumulative total through date d: +// +// count for [Start, End) = P(End-1) - P(Start-1) +// +// Query builders convert internally with AddDays(-1) to get the prefix sum +// lookup dates. +type DateRange struct { + Start civil.Date // first date of the period (inclusive) + End civil.Date // first date after the period (exclusive) +} + +// PeriodsForReportType returns the sample and base date ranges for the given +// report type. The sample period is the "current" window; the base period is +// the "previous" comparison window. +func PeriodsForReportType(reportType v1.ReportType) (sample, base DateRange) { + tomorrow := civil.DateOf(time.Now().UTC()).AddDays(1) + if reportType == v1.TwoDayReport { + boundary := tomorrow.AddDays(-3) + return DateRange{Start: boundary, End: tomorrow}, + DateRange{Start: tomorrow.AddDays(-10), End: boundary} + } + boundary := tomorrow.AddDays(-8) + return DateRange{Start: boundary, End: tomorrow}, + DateRange{Start: tomorrow.AddDays(-15), End: boundary} +} + +// TestReportQuery returns per-variant test report rows for a single release with +// metadata (test name, suite, jira component, open bugs, variants). It does NOT +// include percentages or cross-variant statistics; callers that need those should +// use UncollapsedTestReportWithStats instead. +func TestReportQuery(dbc *db.DB, release string, sample, base DateRange, nameMatches TestNameMatches) (*gorm.DB, error) { + return testReportPreAgg(dbc, release, sample, base, nameMatches) +} + +// resolveDateRanges clamps the Start and End of each DateRange to the latest +// available date (+1, since DateRange uses half-open intervals) for the release +// in test_cumulative_summaries. This ensures the planner sees literal dates it +// can use for partition pruning, and handles cases where data hasn't been +// backfilled up to the requested dates. +func resolveDateRanges(dbc *db.DB, release string, ranges ...*DateRange) error { + var maxDate *civil.Date + row := dbc.DB.Table("test_cumulative_summaries"). + Select("MAX(date)"). + Where("release = ?", release). + Row() + if err := row.Scan(&maxDate); err != nil { + return fmt.Errorf("resolving max date for release %s: %w", release, err) + } + if maxDate == nil { + return nil + } + clampTo := maxDate.AddDays(1) + for _, dr := range ranges { + if dr.End.After(clampTo) { + dr.End = clampTo + } + if dr.Start.After(clampTo) { + dr.Start = clampTo + } + } + return nil +} + +func openBugsSubquery(dbc *db.DB) *gorm.DB { + return dbc.DB.Table("bug_tests"). + Select("bug_tests.test_id, COUNT(DISTINCT bugs.id) AS open_bugs"). + Joins("INNER JOIN bugs ON bug_tests.bug_id = bugs.id"). + Where("LOWER(bugs.status) <> 'closed'"). + Group("bug_tests.test_id") +} + +// variantFilterConditions returns raw SQL fragments and args for variant filter items. +// Each fragment is a standalone condition (e.g., "EXISTS (...)") that assumes +// variant_combination_id is in scope. +func variantFilterConditions(variantFilter *filter.Filter) (conditions []string, args []any) { + if variantFilter == nil || len(variantFilter.Items) == 0 { + return nil, nil + } + for _, item := range variantFilter.Items { + switch item.Operator { + case filter.OperatorHasEntry: + if item.Not { + conditions = append(conditions, "NOT EXISTS (SELECT 1 FROM variant_combinations WHERE id = variant_combination_id AND ? = ANY(variants))") + } else { + conditions = append(conditions, "EXISTS (SELECT 1 FROM variant_combinations WHERE id = variant_combination_id AND ? = ANY(variants))") + } + args = append(args, item.Value) + case filter.OperatorHasEntryContaining, filter.OperatorContains: + pattern := "%" + escapeLikeMetachars(strings.ToLower(item.Value)) + "%" + if item.Not { + conditions = append(conditions, "NOT EXISTS (SELECT 1 FROM variant_combinations vc, LATERAL unnest(vc.variants) AS v(item) WHERE vc.id = variant_combination_id AND LOWER(v.item) LIKE ?)") + } else { + conditions = append(conditions, "EXISTS (SELECT 1 FROM variant_combinations vc, LATERAL unnest(vc.variants) AS v(item) WHERE vc.id = variant_combination_id AND LOWER(v.item) LIKE ?)") + } + args = append(args, pattern) + } + } + return conditions, args +} + +// pushdownSafeFields lists columns available in the filtered CTE (before the +// stats join). Only these fields can be pushed into post_filtered; stats-derived +// fields like working_average and delta_from_* exist only after the final SELECT +// and must stay in remaining for the caller to apply on the outer query. +var pushdownSafeFields = sets.New[string]( + "current_runs", "current_successes", "current_failures", "current_flakes", + "previous_runs", "previous_successes", "previous_failures", "previous_flakes", + "open_bugs", + "current_pass_percentage", "current_failure_percentage", "current_flake_percentage", "current_working_percentage", + "previous_pass_percentage", "previous_failure_percentage", "previous_flake_percentage", "previous_working_percentage", + "net_failure_improvement", "net_flake_improvement", "net_working_improvement", "net_improvement", +) + +// arithmeticOps maps filter operators to SQL comparison operators. +var arithmeticOps = map[filter.Operator]string{ + filter.OperatorEquals: "=", + filter.OperatorArithmeticEquals: "=", + filter.OperatorArithmeticNotEquals: "<>", + filter.OperatorArithmeticGreaterThan: ">", + filter.OperatorArithmeticGreaterThanOrEquals: ">=", + filter.OperatorArithmeticLessThan: "<", + filter.OperatorArithmeticLessThanOrEquals: "<=", +} + +// processedFilterConditions converts arithmetic filter items to raw SQL WHERE +// conditions with parameterized args. Items with unsupported operators (ILIKE, +// array membership, etc.) are returned in the remaining filter for the caller to +// apply via GORM. Splitting is only safe for AND-linked filters; OR-linked +// filters are returned entirely as remaining. +func processedFilterConditions(f *filter.Filter) (conditions []string, args []any, remaining *filter.Filter) { + if f == nil || len(f.Items) == 0 { + return nil, nil, nil + } + if f.LinkOperator == filter.LinkOperatorOr { + return nil, nil, f + } + var unsupported []filter.FilterItem + for _, item := range f.Items { + op, isArithmetic := arithmeticOps[item.Operator] + if isArithmetic && pushdownSafeFields.Has(item.Field) { + field := pq.QuoteIdentifier(item.Field) + cond := fmt.Sprintf("%s %s ?", field, op) + if item.Not { + cond = negateConditions([]string{cond})[0] + } + conditions = append(conditions, cond) + args = append(args, item.Value) + } else { + unsupported = append(unsupported, item) + } + } + if len(unsupported) > 0 { + remaining = &filter.Filter{Items: unsupported, LinkOperator: f.LinkOperator} + } + return conditions, args, remaining +} + +// resolvePrefixSumDates clamps the sample and base DateRanges to available data, +// warns if their boundaries don't align, and converts the half-open interval +// dates to the three prefix sum lookup dates used by the 3-way self-join +// (each shifted by -1 day): end (e), boundary (m), and start (s). +func resolvePrefixSumDates(dbc *db.DB, release string, sample, base *DateRange) (end, boundary, start civil.Date, err error) { + if err = resolveDateRanges(dbc, release, sample, base); err != nil { + return + } + if sample.Start != base.End { + log.WithFields(log.Fields{ + "sample_start": sample.Start, + "base_end": base.End, + }).Warn("sample.Start != base.End: query uses sample.Start as the shared boundary; base.End is ignored") + } + return sample.End.AddDays(-1), sample.Start.AddDays(-1), base.Start.AddDays(-1), nil +} + +// testReportCoreJoin builds the 3-way self-join on test_cumulative_summaries +// and aggregates per (test_id, suite_id, variant_combination_id, release). +// Multiple prow_jobs can share the same variant_combination_id, so summing +// here produces one row per variant combination (matching the old matview +// granularity). Keeping rows narrow lets callers add percentages before +// metadata joins. +func testReportCoreJoin(dbc *db.DB, release string, sample, base DateRange, nameMatches TestNameMatches) (*gorm.DB, error) { + end, boundary, start, err := resolvePrefixSumDates(dbc, release, &sample, &base) + if err != nil { + return nil, err + } + + query := dbc.DB. + Table("test_cumulative_summaries e"). + Select(`e.test_id, e.suite_id, pj.variant_combination_id, e.release, + SUM(COALESCE(m.prefix_sum_successes - COALESCE(s.prefix_sum_successes, 0), 0))::bigint AS previous_successes, + SUM(COALESCE(m.prefix_sum_flakes - COALESCE(s.prefix_sum_flakes, 0), 0))::bigint AS previous_flakes, + SUM(COALESCE(m.prefix_sum_failures - COALESCE(s.prefix_sum_failures, 0), 0))::bigint AS previous_failures, + SUM(COALESCE(m.prefix_sum_runs - COALESCE(s.prefix_sum_runs, 0), 0))::bigint AS previous_runs, + SUM(COALESCE(e.prefix_sum_successes - COALESCE(m.prefix_sum_successes, 0), 0))::bigint AS current_successes, + SUM(COALESCE(e.prefix_sum_flakes - COALESCE(m.prefix_sum_flakes, 0), 0))::bigint AS current_flakes, + SUM(COALESCE(e.prefix_sum_failures - COALESCE(m.prefix_sum_failures, 0), 0))::bigint AS current_failures, + SUM(COALESCE(e.prefix_sum_runs - COALESCE(m.prefix_sum_runs, 0), 0))::bigint AS current_runs`). + Joins("JOIN prow_jobs pj ON e.prow_job_id = pj.id AND pj.variant_combination_id IS NOT NULL"). + Joins("LEFT JOIN test_cumulative_summaries m ON m.test_id = e.test_id AND m.prow_job_id = e.prow_job_id AND m.suite_id = e.suite_id AND m.release = e.release AND m.date = ?", boundary). + Joins("LEFT JOIN test_cumulative_summaries s ON s.test_id = e.test_id AND s.prow_job_id = e.prow_job_id AND s.suite_id = e.suite_id AND s.release = e.release AND s.date = ?", start). + Where("e.date = ? AND e.release = ?", end, release). + Group("e.test_id, e.suite_id, pj.variant_combination_id, e.release") + + if nameMatches.HasConditions() { + testsJoin, testsJoinArgs := buildTestsJoinCondition(nameMatches) + query = query.Joins(testsJoin, testsJoinArgs...) + } + + return query, nil +} + +// testReportPreAgg wraps testReportCoreJoin with metadata joins (tests, suites, +// test_ownerships, jira_components, variant_combinations, open_bugs). The planner +// flattens the core subquery so the resulting plan is identical to a single-level +// query with all joins together. +func testReportPreAgg(dbc *db.DB, release string, sample, base DateRange, nameMatches TestNameMatches) (*gorm.DB, error) { + core, err := testReportCoreJoin(dbc, release, sample, base, nameMatches) + if err != nil { + return nil, err + } + openBugs := openBugsSubquery(dbc) + + return dbc.DB. + Table("(?) AS pre", core). + Select(`tests.id, tests.name, pre.suite_id, suites.name AS suite_name, + jira_components.name AS jira_component, jira_components.id AS jira_component_id, + pre.current_successes, pre.current_failures, pre.current_flakes, pre.current_runs, + pre.previous_successes, pre.previous_failures, pre.previous_flakes, pre.previous_runs, + ob.open_bugs, vc.variants, pre.variant_combination_id, pre.release`). + Joins("JOIN tests ON tests.id = pre.test_id"). + Joins("LEFT JOIN variant_combinations vc ON pre.variant_combination_id = vc.id"). + Joins("LEFT JOIN suites ON suites.id = pre.suite_id"). + Joins("LEFT JOIN test_ownerships ON (tests.id = test_ownerships.test_id AND pre.suite_id = test_ownerships.suite_id)"). + Joins("LEFT JOIN jira_components ON test_ownerships.jira_component = jira_components.name"). + Joins("LEFT JOIN (?) AS ob ON tests.id = ob.test_id", openBugs), nil +} + +// TestReportQueryCollapsed builds collapsed test report rows keyed by (test_id, suite_id). +// It aggregates prefix sums per date partition separately (~16K groups each), then joins +// the three small results to compute period counts. This avoids the expensive 3-way +// self-join on all ~1.8M per-prow_job rows that the uncollapsed path requires. +func TestReportQueryCollapsed(dbc *db.DB, release string, sample, base DateRange, variantFilter, nameFilter *filter.Filter) (*gorm.DB, error) { + end, boundary, start, err := resolvePrefixSumDates(dbc, release, &sample, &base) + if err != nil { + return nil, err + } + nameConds, nameArgs := nameFilterConditions(nameFilter) + variantConds, variantArgs := variantFilterConditions(variantFilter) + + var nameJoinClause string + var nameJoinArgs []any + if len(nameConds) > 0 { + joiner := " OR " + if nameFilter.LinkOperator == filter.LinkOperatorAnd { + joiner = " AND " + } + nameJoinClause = "JOIN tests ON tests.id = tcs.test_id AND (" + strings.Join(nameConds, joiner) + ")" + nameJoinArgs = nameArgs + } + + var args []any + var buf strings.Builder + + writeDateAgg := func(alias string, date civil.Date) { + buf.WriteString(`(SELECT tcs.test_id, tcs.suite_id, tcs.release, + SUM(tcs.prefix_sum_successes) AS ps_successes, + SUM(tcs.prefix_sum_failures) AS ps_failures, + SUM(tcs.prefix_sum_flakes) AS ps_flakes, + SUM(tcs.prefix_sum_runs) AS ps_runs + FROM test_cumulative_summaries tcs + JOIN prow_jobs pj ON tcs.prow_job_id = pj.id AND pj.variant_combination_id IS NOT NULL +`) + if nameJoinClause != "" { + buf.WriteString(" ") + buf.WriteString(nameJoinClause) + buf.WriteString("\n") + args = append(args, nameJoinArgs...) + } + buf.WriteString(" WHERE tcs.date = ? AND tcs.release = ?") + args = append(args, date, release) + + for _, cond := range variantConds { + buf.WriteString("\n AND ") + buf.WriteString(cond) + } + args = append(args, variantArgs...) + + fmt.Fprintf(&buf, "\n GROUP BY tcs.test_id, tcs.suite_id, tcs.release\n) AS %s", alias) + } + + buf.WriteString(`SELECT t.id, t.name, su.name AS suite_name, + jc.name AS jira_component, jc.id AS jira_component_id, e.release, + COALESCE(e.ps_successes - COALESCE(m.ps_successes, 0), 0)::bigint AS current_successes, + COALESCE(e.ps_failures - COALESCE(m.ps_failures, 0), 0)::bigint AS current_failures, + COALESCE(e.ps_flakes - COALESCE(m.ps_flakes, 0), 0)::bigint AS current_flakes, + COALESCE(e.ps_runs - COALESCE(m.ps_runs, 0), 0)::bigint AS current_runs, + COALESCE(m.ps_successes - COALESCE(s.ps_successes, 0), 0)::bigint AS previous_successes, + COALESCE(m.ps_failures - COALESCE(s.ps_failures, 0), 0)::bigint AS previous_failures, + COALESCE(m.ps_flakes - COALESCE(s.ps_flakes, 0), 0)::bigint AS previous_flakes, + COALESCE(m.ps_runs - COALESCE(s.ps_runs, 0), 0)::bigint AS previous_runs, + ob.open_bugs +FROM `) + + writeDateAgg("e", end) + + buf.WriteString("\nLEFT JOIN ") + writeDateAgg("m", boundary) + buf.WriteString(" ON m.test_id = e.test_id AND m.suite_id = e.suite_id AND m.release = e.release") + + buf.WriteString("\nLEFT JOIN ") + writeDateAgg("s", start) + buf.WriteString(` ON s.test_id = e.test_id AND s.suite_id = e.suite_id AND s.release = e.release +JOIN tests t ON t.id = e.test_id +LEFT JOIN suites su ON su.id = e.suite_id +LEFT JOIN test_ownerships too ON too.test_id = e.test_id AND too.suite_id = e.suite_id +LEFT JOIN jira_components jc ON jc.name = too.jira_component +LEFT JOIN ( + ` + openBugsSQL + ` +) AS ob ON t.id = ob.test_id`) + + return dbc.DB.Raw(buf.String(), args...), nil +} diff --git a/pkg/db/query/cumulative_query_test.go b/pkg/db/query/cumulative_query_test.go new file mode 100644 index 0000000000..06cd5691a4 --- /dev/null +++ b/pkg/db/query/cumulative_query_test.go @@ -0,0 +1,238 @@ +package query + +import ( + "testing" + "time" + + "cloud.google.com/go/civil" + + v1 "github.com/openshift/sippy/pkg/apis/sippyprocessing/v1" + "github.com/openshift/sippy/pkg/filter" +) + +func TestEscapeLikeMetachars(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "no metacharacters", + input: "install should succeed", + expected: "install should succeed", + }, + { + name: "underscore escaped", + input: "test_name_here", + expected: `test\_name\_here`, + }, + { + name: "percent escaped", + input: "100% complete", + expected: `100\% complete`, + }, + { + name: "backslash escaped", + input: `path\to\test`, + expected: `path\\to\\test`, + }, + { + name: "all metacharacters", + input: `a_b%c\d`, + expected: `a\_b\%c\\d`, + }, + { + name: "empty string", + input: "", + expected: "", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := escapeLikeMetachars(tc.input) + if got != tc.expected { + t.Errorf("escapeLikeMetachars(%q) = %q, want %q", tc.input, got, tc.expected) + } + }) + } +} + +func TestBuildTestsJoinCondition(t *testing.T) { + tests := []struct { + name string + matches TestNameMatches + wantClause string + wantArgCount int + wantFirstArg string + }{ + { + name: "empty matches joins all tests", + wantClause: "JOIN tests ON tests.id = e.test_id", + wantArgCount: 0, + }, + { + name: "exact name", + matches: TestNameMatches{ExactNames: []string{"my test"}}, + wantClause: "JOIN tests ON tests.id = e.test_id AND (tests.name = ?)", + wantArgCount: 1, + wantFirstArg: "my test", + }, + { + name: "prefix with underscore is escaped", + matches: TestNameMatches{Prefixes: []string{"install_should"}}, + wantClause: "JOIN tests ON tests.id = e.test_id AND (tests.name LIKE ?)", + wantArgCount: 1, + wantFirstArg: `install\_should%`, + }, + { + name: "substring uses ILIKE and is escaped", + matches: TestNameMatches{Substrings: []string{"test_name"}}, + wantClause: "JOIN tests ON tests.id = e.test_id AND (tests.name ILIKE ?)", + wantArgCount: 1, + wantFirstArg: `%test\_name%`, + }, + { + name: "multiple match types combined with OR", + matches: TestNameMatches{ + ExactNames: []string{"exact"}, + Prefixes: []string{"pre"}, + Substrings: []string{"sub"}, + }, + wantClause: "JOIN tests ON tests.id = e.test_id AND (tests.name = ? OR tests.name LIKE ? OR tests.name ILIKE ?)", + wantArgCount: 3, + wantFirstArg: "exact", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + clause, args := buildTestsJoinCondition(tc.matches) + if clause != tc.wantClause { + t.Errorf("clause = %q, want %q", clause, tc.wantClause) + } + if len(args) != tc.wantArgCount { + t.Errorf("len(args) = %d, want %d", len(args), tc.wantArgCount) + } + if tc.wantArgCount > 0 && len(args) > 0 { + if got, ok := args[0].(string); !ok || got != tc.wantFirstArg { + t.Errorf("args[0] = %v, want %q", args[0], tc.wantFirstArg) + } + } + }) + } +} + +func TestNameFilterConditions(t *testing.T) { + tests := []struct { + name string + filter *filter.Filter + wantCount int + wantFirstCon string + wantFirstArg string + }{ + { + name: "nil filter", + filter: nil, + wantCount: 0, + }, + { + name: "negative exact name", + filter: &filter.Filter{Items: []filter.FilterItem{ + {Field: "name", Operator: filter.OperatorEquals, Value: "excluded", Not: true}, + }}, + wantCount: 1, + wantFirstCon: "NOT(tests.name = ?)", + wantFirstArg: "excluded", + }, + { + name: "negative substring", + filter: &filter.Filter{Items: []filter.FilterItem{ + {Field: "name", Operator: filter.OperatorContains, Value: "skip_this", Not: true}, + }}, + wantCount: 1, + wantFirstCon: "NOT(tests.name ILIKE ?)", + wantFirstArg: `%skip\_this%`, + }, + { + name: "positive and negative combined", + filter: &filter.Filter{ + Items: []filter.FilterItem{ + {Field: "name", Operator: filter.OperatorContains, Value: "network"}, + {Field: "name", Operator: filter.OperatorContains, Value: "ipv6", Not: true}, + }, + LinkOperator: filter.LinkOperatorAnd, + }, + wantCount: 2, + wantFirstCon: "tests.name ILIKE ?", + wantFirstArg: "%network%", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + conditions, args := nameFilterConditions(tc.filter) + if len(conditions) != tc.wantCount { + t.Errorf("len(conditions) = %d, want %d", len(conditions), tc.wantCount) + } + if tc.wantCount > 0 && len(conditions) > 0 { + if conditions[0] != tc.wantFirstCon { + t.Errorf("conditions[0] = %q, want %q", conditions[0], tc.wantFirstCon) + } + if got, ok := args[0].(string); !ok || got != tc.wantFirstArg { + t.Errorf("args[0] = %v, want %q", args[0], tc.wantFirstArg) + } + } + }) + } +} + +func TestPeriodsForReportType(t *testing.T) { + tomorrow := civil.DateOf(time.Now().UTC()).AddDays(1) + + tests := []struct { + name string + reportType v1.ReportType + wantSample DateRange + wantBase DateRange + }{ + { + name: "current report has 7-day sample and 7-day base", + reportType: v1.CurrentReport, + wantSample: DateRange{Start: tomorrow.AddDays(-8), End: tomorrow}, + wantBase: DateRange{Start: tomorrow.AddDays(-15), End: tomorrow.AddDays(-8)}, + }, + { + name: "two-day report has 3-day sample and 7-day base", + reportType: v1.TwoDayReport, + wantSample: DateRange{Start: tomorrow.AddDays(-3), End: tomorrow}, + wantBase: DateRange{Start: tomorrow.AddDays(-10), End: tomorrow.AddDays(-3)}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + sample, base := PeriodsForReportType(tc.reportType) + if sample != tc.wantSample { + t.Errorf("sample = %+v, want %+v", sample, tc.wantSample) + } + if base != tc.wantBase { + t.Errorf("base = %+v, want %+v", base, tc.wantBase) + } + }) + } +} + +func TestPeriodsForReportType_Contiguous(t *testing.T) { + tests := []struct { + name string + reportType v1.ReportType + }{ + {name: "current", reportType: v1.CurrentReport}, + {name: "twoDay", reportType: v1.TwoDayReport}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + sample, base := PeriodsForReportType(tc.reportType) + if sample.Start != base.End { + t.Errorf("sample.Start (%v) != base.End (%v): periods are not contiguous", sample.Start, base.End) + } + }) + } +} diff --git a/pkg/db/query/feature_gates.go b/pkg/db/query/feature_gates.go index 4d3e5bf22c..6d0004b15e 100644 --- a/pkg/db/query/feature_gates.go +++ b/pkg/db/query/feature_gates.go @@ -1,29 +1,53 @@ package query import ( - "gorm.io/gorm" + "fmt" + "time" + + "cloud.google.com/go/civil" "github.com/openshift/sippy/pkg/apis/api" + "github.com/openshift/sippy/pkg/db" "github.com/openshift/sippy/pkg/filter" ) -func GetFeatureGatesFromDB(dbc *gorm.DB, release string, filterOpts *filter.FilterOptions) ([]api.FeatureGate, error) { - // Get tests by feature gate. - // Install related FG is special and is covered by install should succeed case. - // Pre-filter with LIKE to avoid running expensive regex on every row. - // regexp_matches(..., 'g') counts every tag in the name, not only the first. - byTag := dbc.Table("prow_test_report_7d_matview AS ptr"). - Select("ptr.name, ptr.release, (m.match)[2] AS gate_name"). - Joins(`CROSS JOIN LATERAL regexp_matches(ptr.name, '\[(FeatureGate|OCPFeatureGate):([^\]]+)\]', 'g') AS m(match)`). - Where("ptr.release = ? AND ptr.name LIKE ?", release, "%FeatureGate:%") - byInstall := dbc.Table("prow_test_report_7d_matview"). - Select("name, release, NULL::text AS gate_name"). - Where("release = ? AND name LIKE ?", release, "%install should succeed%") - subQuery := dbc.Raw("? UNION ALL ?", byTag, byInstall) +func GetFeatureGatesFromDB(dbc *db.DB, release string, filterOpts *filter.FilterOptions) ([]api.FeatureGate, error) { + // Get test names that had actual runs in the last 7 days. We compare + // prefix_sum_runs at the end vs start of the range to confirm the test ran + // at least once, filtering out tests that merely have carried-forward rows. + tomorrow := civil.DateOf(time.Now().UTC()).AddDays(1) + dr := DateRange{Start: tomorrow.AddDays(-8), End: tomorrow} + if err := resolveDateRanges(dbc, release, &dr); err != nil { + return nil, err + } + lookupEnd := dr.End.AddDays(-1) + lookupStart := dr.Start.AddDays(-1) + + activeTestExists := `EXISTS ( + SELECT 1 FROM test_cumulative_summaries e + LEFT JOIN test_cumulative_summaries s + ON s.test_id = e.test_id AND s.prow_job_id = e.prow_job_id + AND s.suite_id = e.suite_id AND s.release = e.release + AND s.date = ? + WHERE e.test_id = t.id AND e.date = ? AND e.release = ? + AND e.prefix_sum_runs > COALESCE(s.prefix_sum_runs, 0) + )` + + byTag := dbc.DB.Table("tests t"). + Select("t.name, ? AS release, (m.match)[2] AS gate_name", release). + Joins(`CROSS JOIN LATERAL regexp_matches(t.name, '\[(FeatureGate|OCPFeatureGate):([^\]]+)\]', 'g') AS m(match)`). + Where("t.name LIKE ?", "%FeatureGate:%"). + Where(activeTestExists, lookupStart, lookupEnd, release) + + byInstall := dbc.DB.Table("tests t"). + Select("t.name, ? AS release, NULL::text AS gate_name", release). + Where("t.name LIKE ?", "%install should succeed%"). + Where(activeTestExists, lookupStart, lookupEnd, release) + + subQuery := dbc.DB.Raw("? UNION ALL ?", byTag, byInstall) // Figure out the first release we ever saw a FG. - // Use DISTINCT ON to return exactly one row per feature gate (the earliest release). - firstSeenQuery := dbc.Raw(` + firstSeenQuery := dbc.DB.Raw(` SELECT DISTINCT ON (feature_gate) feature_gate, release AS first_seen_in, @@ -34,7 +58,7 @@ func GetFeatureGatesFromDB(dbc *gorm.DB, release string, filterOpts *filter.Filt ORDER BY feature_gate, string_to_array(release, '.')::int[] ASC `) - query := dbc.Table("feature_gates AS fg"). + fgQuery := dbc.DB.Table("feature_gates AS fg"). Select(` ROW_NUMBER() OVER (ORDER BY fg.feature_gate) AS id, fg.feature_gate, @@ -52,17 +76,17 @@ func GetFeatureGatesFromDB(dbc *gorm.DB, release string, filterOpts *filter.Filt Group("fg.feature_gate, fg.release, fs.first_seen_in, fs.first_seen_in_major, fs.first_seen_in_minor"). Order("fg.feature_gate") - table := dbc.Table("(?) AS results", query) + table := dbc.DB.Table("(?) AS results", fgQuery) q, err := filter.FilterableDBResult(table, filterOpts, api.FeatureGate{}) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to apply filter: %w", err) } results := make([]api.FeatureGate, 0) tx := q.Scan(&results) if tx.Error != nil { - return nil, tx.Error + return nil, fmt.Errorf("failed to scan feature gates: %w", tx.Error) } return results, nil diff --git a/pkg/db/query/misc_queries.go b/pkg/db/query/misc_queries.go deleted file mode 100644 index c6997c494e..0000000000 --- a/pkg/db/query/misc_queries.go +++ /dev/null @@ -1,61 +0,0 @@ -package query - -import ( - "database/sql" - "fmt" - "time" - - log "github.com/sirupsen/logrus" - "k8s.io/apimachinery/pkg/util/sets" - - "github.com/openshift/sippy/pkg/db" - "github.com/openshift/sippy/pkg/testidentification" -) - -// PlatformInfraSuccess takes a list of platforms and a period (default -// or twoDay), and returns a map containing keys for platform, and infra -// success percentage for that period. -func PlatformInfraSuccess(dbc *db.DB, platforms sets.Set[string], period string) (map[string]float64, error) { - now := time.Now() - results := make(map[string]float64) - - table := "" - switch period { - case "current": - table = "prow_test_report_7d_matview" - case "twoDay": - table = "prow_test_report_2d_matview" - default: - return nil, fmt.Errorf("unknown period %s", period) - } - - var sqlResults []struct { - Variant string - PassPercentage float64 - } - q := dbc.DB.Raw(fmt.Sprintf(` - WITH target_variants AS ( - SELECT vc.id, v.variant - FROM variant_combinations vc, unnest(vc.variants) AS v(variant) - WHERE v.variant IN @platforms - ) - SELECT tv.variant, - SUM(m.current_successes) * 100.0 / NULLIF(SUM(m.current_runs), 0) AS pass_percentage - FROM %s m - JOIN target_variants tv ON m.variant_combination_id = tv.id - WHERE m.name = @testname - GROUP BY tv.variant`, table), - sql.Named("platforms", platforms.UnsortedList()), - sql.Named("testname", testidentification.NewInfrastructureTestName), - ).Scan(&sqlResults) - - for _, r := range sqlResults { - results[r.Variant] = r.PassPercentage - } - - elapsed := time.Since(now) - log.WithFields(log.Fields{ - "elapsed": elapsed, - }).Info("PlatformInfraSuccess completed") - return results, q.Error -} diff --git a/pkg/db/query/test_queries.go b/pkg/db/query/test_queries.go index de775ae37e..dec93a701e 100644 --- a/pkg/db/query/test_queries.go +++ b/pkg/db/query/test_queries.go @@ -1,7 +1,6 @@ package query import ( - "database/sql" "errors" "fmt" "strings" @@ -17,6 +16,7 @@ import ( jira "github.com/openshift/sippy/pkg/apis/jira/v1" "github.com/openshift/sippy/pkg/db" "github.com/openshift/sippy/pkg/db/models" + "github.com/openshift/sippy/pkg/filter" "github.com/openshift/sippy/pkg/util" ) @@ -59,6 +59,12 @@ const ( QueryTestSummarizer = QueryTestFields + "," + QueryTestPercentages + openBugsSQL = `SELECT bug_tests.test_id, COUNT(DISTINCT bugs.id) AS open_bugs + FROM bug_tests + INNER JOIN bugs ON bug_tests.bug_id = bugs.id + WHERE LOWER(bugs.status) <> 'closed' + GROUP BY bug_tests.test_id` + QueryTestAnalysis = ` select current_successes, current_runs, current_successes * 100.0 / NULLIF(current_runs, 0) AS current_pass_percentage @@ -96,65 +102,73 @@ func LoadTestCache(dbc *db.DB, preloads []string) (map[string]*models.Test, erro return testCache, nil } -// TestReportsByVariant returns a test report for every test in the db matching the given substrings, separated by variant. +// TestReportsByVariant returns per-variant test report rows for every test matching +// the given name criteria. When includeAll is true, an additional "All" aggregate row +// per test is included via UNION ALL (Variant = "All", counting runs across all +// variant_combination_ids without unnesting). func TestReportsByVariant( dbc *db.DB, release string, - reportType v1.ReportType, // defaults to "current" or last 7 days vs prev 7 days - testSubStrings []string, + reportType v1.ReportType, + nameMatches TestNameMatches, excludeVariants []string, + includeAll bool, ) ([]api.Test, error) { now := time.Now() - testSubstringFilter := strings.Join(testSubStrings, "|") - testSubstringFilter = strings.ReplaceAll(testSubstringFilter, "[", "\\[") - testSubstringFilter = strings.ReplaceAll(testSubstringFilter, "]", "\\]") - - // Query and group by variant: - var testReports []api.Test - q := ` -WITH results AS ( - SELECT name, - release, - sum(current_runs) AS current_runs, - sum(current_successes) AS current_successes, - sum(current_failures) AS current_failures, - sum(current_flakes) AS current_flakes, - sum(previous_runs) AS previous_runs, - sum(previous_successes) AS previous_successes, - sum(previous_failures) AS previous_failures, - sum(previous_flakes) AS previous_flakes, - unnest(variants) AS variant - FROM prow_test_report_7d_matview - WHERE release = @release AND name ~* @testsubstrings - AND NOT EXISTS (SELECT 1 FROM variant_combinations WHERE @excluded && variants AND id = variant_combination_id) - GROUP BY name, release, variant -) -SELECT *, - current_successes * 100.0 / NULLIF(current_runs, 0) AS current_pass_percentage, - current_failures * 100.0 / NULLIF(current_runs, 0) AS current_failure_percentage, - previous_successes * 100.0 / NULLIF(previous_runs, 0) AS previous_pass_percentage, - previous_failures * 100.0 / NULLIF(previous_runs, 0) AS previous_failure_percentage, - (current_successes * 100.0 / NULLIF(current_runs, 0)) - (previous_successes * 100.0 / NULLIF(previous_runs, 0)) AS net_improvement -FROM results; -` - if reportType == v1.TwoDayReport { - q = strings.ReplaceAll(q, "prow_test_report_7d_matview", "prow_test_report_2d_matview") + sample, base := PeriodsForReportType(reportType) + inner, err := TestReportQuery(dbc, release, sample, base, nameMatches) + if err != nil { + return nil, err } - - qParams := []interface{}{ - sql.Named("excluded", pq.Array(excludeVariants)), - sql.Named("release", release), - sql.Named("testsubstrings", testSubstringFilter), + excludeArr := pq.Array(excludeVariants) + + perVariant := dbc.DB.Table("(?) AS r", inner). + Select(`name, release, + sum(current_runs) AS current_runs, + sum(current_successes) AS current_successes, + sum(current_failures) AS current_failures, + sum(current_flakes) AS current_flakes, + sum(previous_runs) AS previous_runs, + sum(previous_successes) AS previous_successes, + sum(previous_failures) AS previous_failures, + sum(previous_flakes) AS previous_flakes, + unnest(variants) AS variant`). + Where("NOT EXISTS (SELECT 1 FROM variant_combinations WHERE ? && variants AND id = variant_combination_id)", excludeArr). + Group("name, release, variant") + + var aggregated *gorm.DB + if includeAll { + allArm := dbc.DB.Table("(?) AS r", inner). + Select(`name, release, + sum(current_runs) AS current_runs, + sum(current_successes) AS current_successes, + sum(current_failures) AS current_failures, + sum(current_flakes) AS current_flakes, + sum(previous_runs) AS previous_runs, + sum(previous_successes) AS previous_successes, + sum(previous_failures) AS previous_failures, + sum(previous_flakes) AS previous_flakes, + 'All'::text AS variant`). + Where("NOT EXISTS (SELECT 1 FROM variant_combinations WHERE ? && variants AND id = variant_combination_id)", excludeArr). + Group("name, release") + aggregated = dbc.DB.Raw("? UNION ALL ?", perVariant, allArm) + } else { + aggregated = perVariant } - r := dbc.DB.Raw(q, qParams...).Scan(&testReports) + + var testReports []api.Test + r := dbc.DB.Table("(?) AS agg", aggregated). + Select(fmt.Sprintf("*, %s", QueryTestPercentages)). + Where("agg.current_runs > 0 OR agg.previous_runs > 0"). + Scan(&testReports) if r.Error != nil { log.Error(r.Error) return testReports, r.Error } elapsed := time.Since(now) - log.Infof("TestReportsByVariant completed in %s with %d results from db", elapsed, len(testReports)) + log.WithFields(log.Fields{"elapsed": elapsed, "count": len(testReports)}).Info("TestReportsByVariant completed") return testReports, nil } @@ -167,34 +181,41 @@ func TestReportExcludeVariants(dbc *db.DB, release, testName string, excludeVari WithField("release", release). WithField("test", testName) - // Query and group by variant: + sample, base := PeriodsForReportType(v1.CurrentReport) + inner, err := TestReportQuery(dbc, release, sample, base, TestNameMatches{ExactNames: []string{testName}}) + if err != nil { + logger.WithError(err).Error("failed to build test report query") + return api.Test{}, false + } + + aggregated := dbc.DB.Table("(?) AS r", inner). + Select(`name, release, + sum(current_runs) AS current_runs, + sum(current_successes) AS current_successes, + sum(current_failures) AS current_failures, + sum(current_flakes) AS current_flakes, + sum(previous_runs) AS previous_runs, + sum(previous_successes) AS previous_successes, + sum(previous_failures) AS previous_failures, + sum(previous_flakes) AS previous_flakes`). + Where("NOT EXISTS (SELECT 1 FROM variant_combinations WHERE ? && variants AND id = variant_combination_id)", pq.Array(excludeVariants)). + Group("name, release") + + var testReports []api.Test + r := dbc.DB.Table("(?) AS agg", aggregated). + Select(fmt.Sprintf("*, %s", QueryTestPercentages)). + Limit(1). + Find(&testReports) + if r.Error == nil && len(testReports) == 0 { + r.Error = gorm.ErrRecordNotFound + } var testReport api.Test - q := `WITH results AS ( - SELECT name, - release, - sum(current_runs) AS current_runs, - sum(current_successes) AS current_successes, - sum(current_failures) AS current_failures, - sum(current_flakes) AS current_flakes, - sum(previous_runs) AS previous_runs, - sum(previous_successes) AS previous_successes, - sum(previous_failures) AS previous_failures, - sum(previous_flakes) AS previous_flakes - FROM prow_test_report_7d_matview - WHERE release = @release AND name = @testname - AND NOT EXISTS (SELECT 1 FROM variant_combinations WHERE @excluded && variants AND id = variant_combination_id) - GROUP BY name, release -) SELECT *, %s FROM results;` - - q = fmt.Sprintf(q, QueryTestPercentages) - qParams := []interface{}{ - sql.Named("excluded", pq.Array(excludeVariants)), - sql.Named("release", release), - sql.Named("testname", testName), + if len(testReports) > 0 { + testReport = testReports[0] } - if r := dbc.DB.Raw(q, qParams...).First(&testReport); r.Error != nil { + if r.Error != nil { if errors.Is(r.Error, gorm.ErrRecordNotFound) { - logger.Debug("test not found") + logger.WithField("test", testName).Warn("test not found in cumulative summaries") } else { logger.WithError(r.Error).Error("query failed") } @@ -232,18 +253,159 @@ func LoadBugsForTest(dbc *db.DB, testName string, filterClosed bool) ([]models.B return results, nil } -// TestsByNURPAndStandardDeviation returns a test report for every test in the db, separated by variant. -// Each row includes current/previous test rates and cross-variant statistics (AVG, STDDEV, delta) -// that are pre-computed in the matview. Only the deltas are computed at query time. -func TestsByNURPAndStandardDeviation(dbc *db.DB, release, table string) *gorm.DB { - return dbc.DB. - Table(table). - Select(`*, - (current_working_percentage - working_average) AS delta_from_working_average, - (current_pass_percentage - passing_average) AS delta_from_passing_average, - (current_flake_percentage - flake_average) AS delta_from_flake_average`). - Where(`release = ?`, release). - Where("NOT EXISTS (SELECT 1 FROM variant_combinations WHERE 'never-stable' = any(variants) AND id = variant_combination_id)") +// UncollapsedTestReportWithStats builds a per-variant test report with cross-variant +// statistics using a materialized common table expression (CTE) pipeline: filtered -> post_filtered -> stats. +// The filtered CTE applies name matches, variant filters, and the never-stable/zero-run +// exclusions. The post_filtered CTE applies arithmetic processedFilter conditions +// (current_runs >= N, failure_percentage >= M) so the stats CTE only scans test_ids +// that survive all filters. Stats are computed across ALL variant combinations for +// each (test_id, suite_id), regardless of which individual variants passed filtering. +// +// Any processedFilter items with unsupported operators (ILIKE, array membership) are +// returned as remainingFilter for the caller to apply via GORM on the outer query. +func UncollapsedTestReportWithStats(dbc *db.DB, release string, sample, base DateRange, nameFilter, variantFilter, processedFilter *filter.Filter) (*gorm.DB, *filter.Filter, error) { + end, boundary, start, err := resolvePrefixSumDates(dbc, release, &sample, &base) + if err != nil { + return nil, nil, err + } + + var args []any + var buf strings.Builder + + nameJoinClause := "" + var nameJoinArgs []any + if nameConds, nameArgs := nameFilterConditions(nameFilter); len(nameConds) > 0 { + joiner := " OR " + if nameFilter.LinkOperator == filter.LinkOperatorAnd { + joiner = " AND " + } + nameJoinClause = "JOIN tests ON tests.id = e.test_id AND (" + strings.Join(nameConds, joiner) + ")" + nameJoinArgs = nameArgs + } + + variantConds, variantArgs := variantFilterConditions(variantFilter) + pfConds, pfArgs, remainingFilter := processedFilterConditions(processedFilter) + + // === filtered CTE: per-variant test report with percentages === + buf.WriteString(`WITH filtered AS MATERIALIZED ( + SELECT tests.id, tests.name, pre.suite_id, suites.name AS suite_name, + jira_components.name AS jira_component, jira_components.id AS jira_component_id, + pre.current_successes, pre.current_failures, pre.current_flakes, pre.current_runs, + pre.previous_successes, pre.previous_failures, pre.previous_flakes, pre.previous_runs, + ob.open_bugs, vc.variants, pre.variant_combination_id, pre.release, + `) + buf.WriteString(QueryTestPercentages) + buf.WriteString(` + FROM ( + SELECT e.test_id, e.suite_id, pj.variant_combination_id, e.release, + SUM(COALESCE(m.prefix_sum_successes - COALESCE(s.prefix_sum_successes, 0), 0))::bigint AS previous_successes, + SUM(COALESCE(m.prefix_sum_flakes - COALESCE(s.prefix_sum_flakes, 0), 0))::bigint AS previous_flakes, + SUM(COALESCE(m.prefix_sum_failures - COALESCE(s.prefix_sum_failures, 0), 0))::bigint AS previous_failures, + SUM(COALESCE(m.prefix_sum_runs - COALESCE(s.prefix_sum_runs, 0), 0))::bigint AS previous_runs, + SUM(COALESCE(e.prefix_sum_successes - COALESCE(m.prefix_sum_successes, 0), 0))::bigint AS current_successes, + SUM(COALESCE(e.prefix_sum_flakes - COALESCE(m.prefix_sum_flakes, 0), 0))::bigint AS current_flakes, + SUM(COALESCE(e.prefix_sum_failures - COALESCE(m.prefix_sum_failures, 0), 0))::bigint AS current_failures, + SUM(COALESCE(e.prefix_sum_runs - COALESCE(m.prefix_sum_runs, 0), 0))::bigint AS current_runs + FROM test_cumulative_summaries e + JOIN prow_jobs pj ON e.prow_job_id = pj.id AND pj.variant_combination_id IS NOT NULL + LEFT JOIN test_cumulative_summaries m ON m.test_id = e.test_id AND m.prow_job_id = e.prow_job_id AND m.suite_id = e.suite_id AND m.release = e.release AND m.date = ? + LEFT JOIN test_cumulative_summaries s ON s.test_id = e.test_id AND s.prow_job_id = e.prow_job_id AND s.suite_id = e.suite_id AND s.release = e.release AND s.date = ? +`) + args = append(args, boundary, start) + + if nameJoinClause != "" { + buf.WriteString(" ") + buf.WriteString(nameJoinClause) + buf.WriteString("\n") + args = append(args, nameJoinArgs...) + } + + buf.WriteString(` WHERE e.date = ? AND e.release = ? + GROUP BY e.test_id, e.suite_id, pj.variant_combination_id, e.release + ) AS pre + JOIN tests ON tests.id = pre.test_id + LEFT JOIN variant_combinations vc ON pre.variant_combination_id = vc.id + LEFT JOIN suites ON suites.id = pre.suite_id + LEFT JOIN test_ownerships ON (tests.id = test_ownerships.test_id AND pre.suite_id = test_ownerships.suite_id) + LEFT JOIN jira_components ON test_ownerships.jira_component = jira_components.name + LEFT JOIN ( + ` + openBugsSQL + ` + ) AS ob ON tests.id = ob.test_id + WHERE NOT EXISTS (SELECT 1 FROM variant_combinations WHERE 'never-stable' = any(variants) AND id = variant_combination_id) + AND (current_runs > 0 OR previous_runs > 0)`) + args = append(args, end, release) + + for _, cond := range variantConds { + buf.WriteString("\n AND ") + buf.WriteString(cond) + } + args = append(args, variantArgs...) + + buf.WriteString("\n)") + + // === post_filtered CTE: narrows filtered rows by arithmetic processedFilter + // conditions (e.g., current_runs >= 7). This determines which test_ids the stats + // CTE processes, avoiding a full-release scan when filters are selective. + statsSource := "filtered" + resultSource := "filtered f" + if len(pfConds) > 0 { + statsSource = "post_filtered" + resultSource = "post_filtered f" + buf.WriteString(`, +post_filtered AS MATERIALIZED ( + SELECT * FROM filtered + WHERE `) + buf.WriteString(strings.Join(pfConds, " AND ")) + buf.WriteString("\n)") + args = append(args, pfArgs...) + } + + // === stats CTE: AVG/STDDEV across all variant combinations per (test_id, suite_id), + // scoped to test_ids from the filtered/post_filtered CTE. Uses a 2-way prefix sum + // join (current period only) since base-period counts are not needed for + // cross-variant statistics. + fmt.Fprintf(&buf, `, +stats AS ( + SELECT c.test_id, c.suite_id, + AVG((c.current_successes + c.current_flakes) * 100.0 / NULLIF(c.current_runs, 0)) AS working_average, + STDDEV((c.current_successes + c.current_flakes) * 100.0 / NULLIF(c.current_runs, 0)) AS working_standard_deviation, + AVG(c.current_successes * 100.0 / NULLIF(c.current_runs, 0)) AS passing_average, + STDDEV(c.current_successes * 100.0 / NULLIF(c.current_runs, 0)) AS passing_standard_deviation, + AVG(c.current_flakes * 100.0 / NULLIF(c.current_runs, 0)) AS flake_average, + STDDEV(c.current_flakes * 100.0 / NULLIF(c.current_runs, 0)) AS flake_standard_deviation + FROM ( + SELECT e.test_id, e.suite_id, pj.variant_combination_id, + SUM(e.prefix_sum_successes - COALESCE(m.prefix_sum_successes, 0))::bigint AS current_successes, + SUM(e.prefix_sum_flakes - COALESCE(m.prefix_sum_flakes, 0))::bigint AS current_flakes, + SUM(e.prefix_sum_runs - COALESCE(m.prefix_sum_runs, 0))::bigint AS current_runs + FROM test_cumulative_summaries e + JOIN prow_jobs pj ON e.prow_job_id = pj.id AND pj.variant_combination_id IS NOT NULL + LEFT JOIN test_cumulative_summaries m ON m.test_id = e.test_id AND m.prow_job_id = e.prow_job_id AND m.suite_id = e.suite_id AND m.release = e.release AND m.date = ? + WHERE e.date = ? AND e.release = ? + AND e.test_id IN (SELECT DISTINCT id FROM %s) + AND NOT EXISTS (SELECT 1 FROM variant_combinations WHERE 'never-stable' = any(variants) AND id = pj.variant_combination_id) + GROUP BY e.test_id, e.suite_id, pj.variant_combination_id + ) c + GROUP BY c.test_id, c.suite_id +)`, statsSource) + args = append(args, boundary, end, release) + + // === Final SELECT: join filtered/post_filtered rows with their cross-variant statistics === + fmt.Fprintf(&buf, ` +SELECT f.*, + COALESCE(s.working_average, 0) AS working_average, + COALESCE(s.working_standard_deviation, 0) AS working_standard_deviation, + f.current_working_percentage - COALESCE(s.working_average, 0) AS delta_from_working_average, + COALESCE(s.passing_average, 0) AS passing_average, + COALESCE(s.passing_standard_deviation, 0) AS passing_standard_deviation, + f.current_pass_percentage - COALESCE(s.passing_average, 0) AS delta_from_passing_average, + COALESCE(s.flake_average, 0) AS flake_average, + COALESCE(s.flake_standard_deviation, 0) AS flake_standard_deviation, + f.current_flake_percentage - COALESCE(s.flake_average, 0) AS delta_from_flake_average +FROM %s +LEFT JOIN stats s ON f.id = s.test_id AND f.suite_id = s.suite_id`, resultSource) + + return dbc.DB.Raw(buf.String(), args...), remainingFilter, nil } func TestOutputs(dbc *db.DB, release, test string, includedVariants, excludedVariants []string, quantity int) ([]api.TestOutput, error) { diff --git a/pkg/db/views.go b/pkg/db/views.go index 6618719e4c..04fd56a36a 100644 --- a/pkg/db/views.go +++ b/pkg/db/views.go @@ -14,27 +14,6 @@ const timestampFormat = "2006-01-02 15:04:05" // TODO: for historical sippy we need to specify the pinnedDate and not use NOW var PostgresMatViews = []PostgresView{ - { - Name: "prow_test_report_7d_matview", - Definition: testReportMatView, - IndexColumns: []string{"release", "name", "id", "variant_combination_id", "suite_name"}, - ReplaceStrings: map[string]string{ - "|||START|||": "|||TIMENOW||| - INTERVAL '14 DAY'", - "|||BOUNDARY|||": "|||TIMENOW||| - INTERVAL '7 DAY'", - "|||END|||": "|||TIMENOW|||", - }, - }, - { - Name: "prow_test_report_2d_matview", - Definition: testReportMatView, - IndexColumns: []string{"release", "name", "id", "variant_combination_id", "suite_name"}, - RefreshPhase: 1, // avoid CPU overload from refreshing concurrently with the 7d matview - ReplaceStrings: map[string]string{ - "|||START|||": "|||TIMENOW||| - INTERVAL '9 DAY'", - "|||BOUNDARY|||": "|||TIMENOW||| - INTERVAL '2 DAY'", - "|||END|||": "|||TIMENOW|||", - }, - }, { Name: "prow_job_runs_report_matview", Definition: jobRunsReportMatView, @@ -57,24 +36,6 @@ var PostgresMatViews = []PostgresView{ "|||BY|||": "hour", }, }, - { - Name: "prow_test_report_7d_collapsed_matview", - Definition: testReportCollapsedMatView, - IndexColumns: []string{"release", "id", "suite_name", "jira_component", "jira_component_id"}, - RefreshPhase: 2, // reads from prow_test_report_7d_matview, which refreshes in phase 0 - ReplaceStrings: map[string]string{ - "|||SOURCE|||": "prow_test_report_7d_matview", - }, - }, - { - Name: "prow_test_report_2d_collapsed_matview", - Definition: testReportCollapsedMatView, - IndexColumns: []string{"release", "id", "suite_name", "jira_component", "jira_component_id"}, - RefreshPhase: 2, // reads from prow_test_report_2d_matview, which refreshes in phase 1 - ReplaceStrings: map[string]string{ - "|||SOURCE|||": "prow_test_report_2d_matview", - }, - }, { // TODO: this probably doesn't need to be a matview anymore since we only keep 3 months of data, // metrics show this refreshing in .6s a lot of the time, occasionally up to 5s. @@ -283,120 +244,6 @@ FROM prow_job_runs JOIN prow_jobs ON prow_job_runs.prow_job_id = prow_jobs.id WHERE prow_job_runs."timestamp" >= |||TIMENOW||| - interval '90 days' ` -const testReportMatView = ` -SELECT base.*, - COALESCE(base.current_successes * 100.0 / NULLIF(base.current_runs, 0), 0) AS current_pass_percentage, - COALESCE(base.current_failures * 100.0 / NULLIF(base.current_runs, 0), 0) AS current_failure_percentage, - COALESCE(base.current_flakes * 100.0 / NULLIF(base.current_runs, 0), 0) AS current_flake_percentage, - COALESCE((base.current_successes + base.current_flakes) * 100.0 / NULLIF(base.current_runs, 0), 0) AS current_working_percentage, - COALESCE(base.previous_successes * 100.0 / NULLIF(base.previous_runs, 0), 0) AS previous_pass_percentage, - COALESCE(base.previous_failures * 100.0 / NULLIF(base.previous_runs, 0), 0) AS previous_failure_percentage, - COALESCE(base.previous_flakes * 100.0 / NULLIF(base.previous_runs, 0), 0) AS previous_flake_percentage, - COALESCE((base.previous_successes + base.previous_flakes) * 100.0 / NULLIF(base.previous_runs, 0), 0) AS previous_working_percentage, - AVG((base.current_successes + base.current_flakes) * 100.0 / NULLIF(base.current_runs, 0)) OVER w AS working_average, - STDDEV((base.current_successes + base.current_flakes) * 100.0 / NULLIF(base.current_runs, 0)) OVER w AS working_standard_deviation, - AVG(base.current_successes * 100.0 / NULLIF(base.current_runs, 0)) OVER w AS passing_average, - STDDEV(base.current_successes * 100.0 / NULLIF(base.current_runs, 0)) OVER w AS passing_standard_deviation, - AVG(base.current_flakes * 100.0 / NULLIF(base.current_runs, 0)) OVER w AS flake_average, - STDDEV(base.current_flakes * 100.0 / NULLIF(base.current_runs, 0)) OVER w AS flake_standard_deviation -FROM ( - WITH open_bugs AS ( - SELECT - test_id, - COUNT(DISTINCT bugs.id) AS open_bugs - FROM - bug_tests - INNER JOIN tests ON tests.id = bug_tests.test_id - INNER JOIN bugs ON bug_tests.bug_id = bugs.id - WHERE - LOWER(bugs.status) <> 'closed' - GROUP BY - test_id - ), - pre_agg AS ( - SELECT - pj.variant_combination_id, - tds.test_id, - tds.suite_id, - tds.release AS prow_job_run_release, - COALESCE(SUM(tds.successes) FILTER (WHERE tds.summary_date >= |||START||| AND tds.summary_date < |||BOUNDARY|||), 0) AS previous_successes, - COALESCE(SUM(tds.flakes) FILTER (WHERE tds.summary_date >= |||START||| AND tds.summary_date < |||BOUNDARY|||), 0) AS previous_flakes, - COALESCE(SUM(tds.failures) FILTER (WHERE tds.summary_date >= |||START||| AND tds.summary_date < |||BOUNDARY|||), 0) AS previous_failures, - COALESCE(SUM(tds.runs) FILTER (WHERE tds.summary_date >= |||START||| AND tds.summary_date < |||BOUNDARY|||), 0) AS previous_runs, - COALESCE(SUM(tds.successes) FILTER (WHERE tds.summary_date >= |||BOUNDARY||| AND tds.summary_date <= |||END|||), 0) AS current_successes, - COALESCE(SUM(tds.flakes) FILTER (WHERE tds.summary_date >= |||BOUNDARY||| AND tds.summary_date <= |||END|||), 0) AS current_flakes, - COALESCE(SUM(tds.failures) FILTER (WHERE tds.summary_date >= |||BOUNDARY||| AND tds.summary_date <= |||END|||), 0) AS current_failures, - COALESCE(SUM(tds.runs) FILTER (WHERE tds.summary_date >= |||BOUNDARY||| AND tds.summary_date <= |||END|||), 0) AS current_runs - FROM - test_daily_summaries tds - JOIN prow_jobs pj ON tds.prow_job_id = pj.id - WHERE - tds.summary_date >= |||START||| AND tds.summary_date <= |||END||| - GROUP BY - pj.variant_combination_id, tds.test_id, tds.suite_id, tds.release - ) - SELECT - tests.id, - tests.name, - suites.name AS suite_name, - jira_components.name AS jira_component, - jira_components.id AS jira_component_id, - pre_agg.previous_successes::bigint AS previous_successes, - pre_agg.previous_flakes::bigint AS previous_flakes, - pre_agg.previous_failures::bigint AS previous_failures, - pre_agg.previous_runs::bigint AS previous_runs, - pre_agg.current_successes::bigint AS current_successes, - pre_agg.current_flakes::bigint AS current_flakes, - pre_agg.current_failures::bigint AS current_failures, - pre_agg.current_runs::bigint AS current_runs, - open_bugs.open_bugs AS open_bugs, - vc.variants, - pre_agg.variant_combination_id, - pre_agg.prow_job_run_release AS release - FROM - pre_agg - JOIN tests ON tests.id = pre_agg.test_id - LEFT JOIN open_bugs ON pre_agg.test_id = open_bugs.test_id - LEFT JOIN suites ON suites.id = pre_agg.suite_id - LEFT JOIN test_ownerships ON (tests.id = test_ownerships.test_id AND pre_agg.suite_id = test_ownerships.suite_id) - LEFT JOIN jira_components ON test_ownerships.jira_component = jira_components.name - LEFT JOIN variant_combinations vc ON pre_agg.variant_combination_id = vc.id -) AS base -WINDOW w AS (PARTITION BY base.id, base.suite_name, base.release) -` - -// CollapsedVariantExclusions lists the variant values that are pre-excluded in -// the collapsed matview. The API checks incoming variant filters against this -// list to decide whether the collapsed matview can be used. -var CollapsedVariantExclusions = []string{"never-stable", "aggregated"} - -var testReportCollapsedMatView = buildCollapsedMatViewSQL() - -func buildCollapsedMatViewSQL() string { - quotedExclusions := make([]string, len(CollapsedVariantExclusions)) - for i, v := range CollapsedVariantExclusions { - quotedExclusions[i] = fmt.Sprintf("'%s'", v) - } - excludedArray := "ARRAY[" + strings.Join(quotedExclusions, ",") + "]" - return ` -SELECT suite_name, name, id, jira_component, jira_component_id, release, - SUM(current_runs)::bigint AS current_runs, - SUM(current_successes)::bigint AS current_successes, - SUM(current_failures)::bigint AS current_failures, - SUM(current_flakes)::bigint AS current_flakes, - SUM(previous_runs)::bigint AS previous_runs, - SUM(previous_successes)::bigint AS previous_successes, - SUM(previous_failures)::bigint AS previous_failures, - SUM(previous_flakes)::bigint AS previous_flakes, - (array_agg(open_bugs))[1] AS open_bugs -FROM |||SOURCE||| -WHERE NOT EXISTS ( - SELECT 1 FROM variant_combinations WHERE ` + excludedArray + ` && variants AND id = variant_combination_id -) -GROUP BY suite_name, name, id, jira_component, jira_component_id, release -` -} - const testAnalysisByVariantView = ` SELECT byjob.test_id AS test_id, diff --git a/pkg/flags/postgres_benchmarking_test.go b/pkg/flags/postgres_benchmarking_test.go index dee1ed5662..49cfb1734d 100644 --- a/pkg/flags/postgres_benchmarking_test.go +++ b/pkg/flags/postgres_benchmarking_test.go @@ -690,7 +690,7 @@ func getMatviewBenchmarkCases(asOf time.Time) []benchmarkCase { name: "MatviewTestReport7d", fn: func(dbc *db.DB) error { results, err := query.TestReportsByVariant(dbc, benchmarkRelease, - v1.CurrentReport, []string{benchmarkTestName}, nil) + v1.CurrentReport, query.TestNameMatches{Substrings: []string{benchmarkTestName}}, nil, false) if err == nil { log.Printf("MatviewTestReport7d: %d results", len(results)) } @@ -701,7 +701,7 @@ func getMatviewBenchmarkCases(asOf time.Time) []benchmarkCase { name: "MatviewTestReport2d", fn: func(dbc *db.DB) error { results, err := query.TestReportsByVariant(dbc, benchmarkRelease, - v1.TwoDayReport, []string{benchmarkTestName}, nil) + v1.TwoDayReport, query.TestNameMatches{Substrings: []string{benchmarkTestName}}, nil, false) if err == nil { log.Printf("MatviewTestReport2d: %d results", len(results)) } @@ -913,9 +913,13 @@ func getAPIBenchmarkCases(asOf time.Time) []benchmarkCase { {Field: "current_flake_percentage", Operator: filter.OperatorArithmeticEquals, Value: "100", Not: true}, }, } + sample, base := query.PeriodsForReportType(v1.CurrentReport) + inner, err := query.TestReportQuery(dbc, benchmarkRelease, sample, base, query.TestNameMatches{}) + if err != nil { + return err + } rawQuery := dbc.DB. - Table("prow_test_report_7d_matview"). - Where("release = ?", benchmarkRelease). + Table("(?) AS r", inner). Select("suite_name, name, jira_component, jira_component_id, " + query.QueryTestSummer). Group("suite_name, name, jira_component, jira_component_id") rawQuery = rawFilter.ToSQL(rawQuery, apitype.Test{}) @@ -984,77 +988,11 @@ func getAPIBenchmarkCases(asOf time.Time) []benchmarkCase { } } -func Test_BenchmarkSingleReleaseMatview(t *testing.T) { +func Test_BenchmarkCumulativeQueryTestsReport(t *testing.T) { dbc, connName := getBenchmarkDBClient(t) - var source db.PostgresView - for _, mv := range db.PostgresMatViews { - if mv.Name == "prow_test_report_7d_matview" { - source = mv - break - } - } - if source.Name == "" { - t.Fatal("prow_test_report_7d_matview not found in PostgresMatViews") - } - - matviewName := fmt.Sprintf("bench_test_report_7d_%s", strings.ReplaceAll(benchmarkRelease, ".", "_")) - - viewDef := source.Definition - for k, v := range source.ReplaceStrings { - viewDef = strings.ReplaceAll(viewDef, k, v) - } - viewDef = strings.ReplaceAll(viewDef, "|||TIMENOW|||", "NOW()") - - const tsPredicate = "prow_job_run_tests.prow_job_run_timestamp >=" - if !strings.Contains(viewDef, tsPredicate) { - t.Fatalf("expected %q in %s definition", tsPredicate, source.Name) - } - viewDef = strings.Replace(viewDef, - tsPredicate, - fmt.Sprintf("prow_job_run_tests.prow_job_run_release = '%s'\n AND %s", benchmarkRelease, tsPredicate), - 1) - - t.Cleanup(func() { - if err := dbc.DB.Exec(fmt.Sprintf("DROP MATERIALIZED VIEW IF EXISTS %s", matviewName)).Error; err != nil { - t.Logf("failed to drop materialized view %s during cleanup: %v", matviewName, err) - } - }) - if err := dbc.DB.Exec(fmt.Sprintf("DROP MATERIALIZED VIEW IF EXISTS %s", matviewName)).Error; err != nil { - t.Fatalf("failed to drop pre-existing materialized view %s: %v", matviewName, err) - } - var results []benchmarkResult - results = append(results, runBenchmarkCase(t, dbc, benchmarkCase{ - name: "CreateMatview", - fn: func(dbc *db.DB) error { - if err := dbc.DB.Exec(fmt.Sprintf("DROP MATERIALIZED VIEW IF EXISTS %s", matviewName)).Error; err != nil { - return err - } - res := dbc.DB.Exec(fmt.Sprintf("CREATE MATERIALIZED VIEW %s AS %s WITH DATA", matviewName, viewDef)) - if res.Error != nil { - return res.Error - } - var count int64 - if err := dbc.DB.Raw(fmt.Sprintf("SELECT COUNT(*) FROM %s", matviewName)).Scan(&count).Error; err != nil { - return err - } - log.Printf("CreateMatview: %s populated with %d rows", matviewName, count) - return nil - }, - }, 1)) - - indexName := fmt.Sprintf("idx_%s", matviewName) - results = append(results, runBenchmarkCase(t, dbc, benchmarkCase{ - name: "CreateIndex", - fn: func(dbc *db.DB) error { - indexCols := strings.Join(source.IndexColumns, ", ") - res := dbc.DB.Exec(fmt.Sprintf("CREATE UNIQUE INDEX %s ON %s(%s)", indexName, matviewName, indexCols)) - return res.Error - }, - }, 1)) - results = append(results, runBenchmarkCase(t, dbc, benchmarkCase{ name: "QueryAPITestsReport", fn: func(dbc *db.DB) error { @@ -1071,9 +1009,13 @@ func Test_BenchmarkSingleReleaseMatview(t *testing.T) { {Field: "current_flake_percentage", Operator: filter.OperatorArithmeticEquals, Value: "100", Not: true}, }, } + sample, base := query.PeriodsForReportType(v1.CurrentReport) + inner, err := query.TestReportQuery(dbc, benchmarkRelease, sample, base, query.TestNameMatches{}) + if err != nil { + return err + } rawQuery := dbc.DB. - Table(matviewName). - Where("release = ?", benchmarkRelease). + Table("(?) AS r", inner). Select("suite_name, name, jira_component, jira_component_id, " + query.QueryTestSummer). Group("suite_name, name, jira_component, jira_component_id") rawQuery = rawFilter.ToSQL(rawQuery, apitype.Test{}) @@ -1090,7 +1032,7 @@ func Test_BenchmarkSingleReleaseMatview(t *testing.T) { if res.Error != nil { return res.Error } - log.Printf("QueryAPITestsReport: %d tests from %s", len(testReports), matviewName) + log.Printf("QueryAPITestsReport: %d tests from cumulative summaries", len(testReports)) return nil }, }, 3)) diff --git a/pkg/html/installhtml/util.go b/pkg/html/installhtml/util.go index 37a3ee26ec..9b7ec9f362 100644 --- a/pkg/html/installhtml/util.go +++ b/pkg/html/installhtml/util.go @@ -69,7 +69,7 @@ func getDataForTestsByVariantFromDB(dbc *db.DB, release string, testSubStrings [ aggregationToOverallTestResult: map[string]*currPrevTestResult{}, // may not be used in output in our first use case } - testReports, err := query.TestReportsByVariant(dbc, release, sippyprocessingv1.CurrentReport, testSubStrings, nil) + testReports, err := query.TestReportsByVariant(dbc, release, sippyprocessingv1.CurrentReport, query.TestNameMatches{Substrings: testSubStrings}, nil, false) if err != nil { return ret, err } diff --git a/pkg/sippyserver/server.go b/pkg/sippyserver/server.go index ff10654367..cf6152f9da 100644 --- a/pkg/sippyserver/server.go +++ b/pkg/sippyserver/server.go @@ -725,7 +725,7 @@ func (s *Server) jsonFeatureGates(w http.ResponseWriter, req *http.Request) { failureResponse(w, http.StatusBadRequest, "couldn't parse filter opts: "+err.Error()) return } - gates, err := query.GetFeatureGatesFromDB(s.db.DB, release, filterOpts) + gates, err := query.GetFeatureGatesFromDB(s.db, release, filterOpts) if err != nil { failureResponseWithError(w, "couldn't query feature gates", err) return