[WIP] TRT-2741: Add per-request BQ/PG toggle for Component Readiness

README.md

@@ -24,6 +24,11 @@ See [the API documentation](pkg/api/README.md)
 
 See [the front end documentation](sippy-ng/README.md)
 
+## Database
+
+See [database tuning](docs/database-tuning.md) for required PostgreSQL
+parameter group settings.
+
 ## Chat
 
 See [the chat documentation](chat/README.md)

cmd/sippy/annotatejobruns.go

@@ -11,6 +11,7 @@ import (
 	bqprovider "github.com/openshift/sippy/pkg/api/componentreadiness/dataprovider/bigquery"
 	"github.com/openshift/sippy/pkg/apis/api/componentreport/crstatus"
 	"github.com/openshift/sippy/pkg/apis/api/componentreport/crtest"
+	"github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts"
 	"github.com/openshift/sippy/pkg/apis/cache"
 	bqcachedclient "github.com/openshift/sippy/pkg/bigquery"
 	"github.com/openshift/sippy/pkg/bigquery/bqlabel"
@@ -180,7 +181,7 @@ Example run: sippy annotate-job-runs  --google-service-account-credential-file=f
 				return errors.WithMessage(err, "couldn't get DB client")
 			}
 
-			allVariants, errs := componentreadiness.GetJobVariants(ctx, bqprovider.NewBigQueryProvider(bigQueryClient))
+			allVariants, errs := componentreadiness.GetJobVariants(ctx, bqprovider.NewBigQueryProvider(bigQueryClient), reqopts.RequestOptions{})
 			if len(errs) > 0 {
 				return fmt.Errorf("failed to get job variants: %v", errs)
 			}

cmd/sippy/automatejira.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/openshift/sippy/pkg/api/componentreadiness"
 	"github.com/openshift/sippy/pkg/apis/api/componentreport/crtest"
+	"github.com/openshift/sippy/pkg/apis/api/componentreport/reqopts"
 	"github.com/openshift/sippy/pkg/apis/cache"
 	jiratype "github.com/openshift/sippy/pkg/apis/jira/v1"
 	bqcachedclient "github.com/openshift/sippy/pkg/bigquery"
@@ -168,7 +169,7 @@ func NewAutomateJiraCommand() *cobra.Command {
 			if err != nil {
 				return err
 			}
-			allVariants, errs := componentreadiness.GetJobVariants(ctx, provider)
+			allVariants, errs := componentreadiness.GetJobVariants(ctx, provider, reqopts.RequestOptions{})
 			if len(errs) > 0 {
 				return fmt.Errorf("failed to get job variants: %v", errs)
 			}

docs/database-tuning.md

@@ -0,0 +1,21 @@
+# Database Tuning
+
+This document is the source of truth for PostgreSQL parameter group
+customizations required by Sippy. These settings must be configured in the
+RDS parameter group (or equivalent server-level configuration) and are not
+controlled by the application code.
+
+Session-level parameters set by the application on each connection are in
+`pkg/db/db.go`. Per-query transaction overrides are in the query code itself
+(look for `SET LOCAL`). This document covers only the server-level settings
+that live outside the codebase.
+
+## RDS Parameter Group Settings
+
+Differences from the default `default.postgres14` parameter group:
+
+| Parameter | Value | Default | Rationale |
+|-----------|-------|---------|-----------|
+| `effective_io_concurrency` | 200 | 1 | Number of concurrent I/O operations the OS can handle. Set high for SSD/NVMe-backed storage (gp3, io1). |
+| `max_parallel_workers` | 16 | `GREATEST({DBInstanceVCPU/2},8)` | Total parallel workers available across all concurrent queries. Component readiness queries request 4 workers each via `SET LOCAL`, and multiple queries run concurrently. Fixed to avoid dependence on instance vCPU count. |
+| `random_page_cost` | 1.1 | 4.0 | Favors index scans over sequential scans, appropriate for SSD/NVMe storage. Also set per-session by the application in `pkg/db/db.go`. |

pkg/api/cache.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/openshift/sippy/pkg/apis/cache"
 	"github.com/openshift/sippy/pkg/util"
-	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
@@ -96,10 +95,11 @@ func GetDataFromCacheOrGenerate[T any](
 					"type": reflect.TypeOf(defaultVal).String(),
 				}).Infof("cache hit")
 				var cr T
-				if err := json.Unmarshal(res, &cr); err != nil {
-					return defaultVal, []error{errors.WithMessagef(err, "failed to unmarshal cached item.  cacheKey=%+v", cacheKey)}
+				unmarshalErr := json.Unmarshal(res, &cr)
+				if unmarshalErr == nil {
+					return cr, nil
 				}
-				return cr, nil
+				logrus.WithError(unmarshalErr).WithField("key", string(cacheKey)).Warn("cached item failed to unmarshal, regenerating")
 			} 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")
 			}

pkg/api/cache_test.go

@@ -505,6 +505,31 @@ func TestNewCacheSpec_Prefix(t *testing.T) {
 	assert.NotContains(t, string(keyWithout), "pfx~")
 }
 
+// TestGetDataFromCacheOrGenerate_MalformedCacheEntry verifies that a cached
+// entry that fails to unmarshal triggers regeneration instead of returning an error.
+func TestGetDataFromCacheOrGenerate_MalformedCacheEntry(t *testing.T) {
+	mc := newMockCache()
+	spec := NewCacheSpec(testCacheKey{Query: "q1"}, "prefix~", nil)
+
+	// Pre-populate the cache with invalid JSON
+	cacheKey, err := spec.GetCacheKey()
+	require.NoError(t, err)
+	mc.store[string(cacheKey)] = []byte(`{not valid json`)
+
+	var generateCalls int
+	expected := testResult{Value: "regenerated"}
+	result, errs := GetDataFromCacheOrGenerate(
+		context.Background(), mc, cache.RequestOptions{}, spec,
+		makeGenerateFn(expected, &generateCalls), testResult{},
+	)
+
+	assert.Empty(t, errs)
+	assert.Equal(t, expected, result, "should return regenerated value")
+	assert.Equal(t, 1, generateCalls, "should call generateFn when cached data is malformed")
+	assert.Equal(t, 1, mc.getCalls, "should attempt cache read")
+	assert.Equal(t, 1, mc.setCalls, "should cache the regenerated value")
+}
+
 func timePtr(t time.Time) *time.Time {
 	return &t
 }

pkg/api/componentreadiness/component_report.go

@@ -2,7 +2,6 @@ package componentreadiness
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"maps"
 	"os"
@@ -65,9 +64,8 @@ func GetComponentTestVariants(ctx context.Context, provider dataprovider.DataPro
 		api.NewCacheSpec(generator, "TestVariants~", nil), generator.GenerateCacheVariants, CacheVariants{})
 }
 
-func GetJobVariants(ctx context.Context, provider dataprovider.DataProvider) (crtest.JobVariants,
-	[]error) {
-	return provider.QueryJobVariants(ctx)
+func GetJobVariants(ctx context.Context, provider dataprovider.DataProvider, reqOptions reqopts.RequestOptions) (crtest.JobVariants, []error) {
+	return provider.QueryJobVariants(ctx, reqOptions)
 }
 
 func GetComponentReport(
@@ -194,7 +192,8 @@ type GeneratorCacheKey struct {
 	AdvancedOption  reqopts.Advanced
 	TestFilters     reqopts.TestFilters
 	TestIDOptions   []reqopts.TestIdentification
-	IncludeAllTests bool `json:"include_all_tests,omitempty"`
+	IncludeAllTests bool   `json:"include_all_tests,omitempty"`
+	DataSource      string `json:",omitempty"`
 }
 
 // GetCacheKey creates a cache key using the generator properties that we want included for uniqueness in what
@@ -210,6 +209,7 @@ func (c *ComponentReportGenerator) GetCacheKey() GeneratorCacheKey {
 		TestFilters:     c.ReqOptions.TestFilters,
 		TestIDOptions:   c.ReqOptions.TestIDOptions,
 		IncludeAllTests: c.ReqOptions.IncludeAllTests,
+		DataSource:      c.ReqOptions.DataSource,
 	}
 
 	// TestIDOptions initialization differences caused many cache misses. This hacky bit of code attempts to handle
@@ -425,29 +425,22 @@ func goInterruptible(ctx context.Context, wg *sync.WaitGroup, closure func()) {
 	}()
 }
 
-var componentAndCapabilityGetter func(test crtest.KeyWithVariants, stats crstatus.TestStatus) (string, []string)
+var componentAndCapabilityGetter func(stats crstatus.TestStatus) (string, []string)
 
-func testToComponentAndCapability(_ crtest.KeyWithVariants, stats crstatus.TestStatus) (string, []string) {
+func testToComponentAndCapability(stats crstatus.TestStatus) (string, []string) {
 	return stats.Component, stats.Capabilities
 }
 
 // getRowColumnIdentifications defines the rows and columns since they are variable. For rows, different pages have different row titles (component, capability etc)
 // Columns titles depends on the columnGroupBy parameter user requests. A particular test can belong to multiple rows of different capabilities.
-func (c *ComponentReportGenerator) getRowColumnIdentifications(testIDStr string, stats crstatus.TestStatus) ([]crtest.RowIdentification, []crtest.ColumnID, error) {
-	var test crtest.KeyWithVariants
+func (c *ComponentReportGenerator) getRowColumnIdentifications(stats crstatus.TestStatus) ([]crtest.RowIdentification, []crtest.ColumnID) {
 	columnGroupByVariants := c.ReqOptions.VariantOption.ColumnGroupBy
 	// We show column groups by DBGroupBy only for the last page before test details
 	if len(c.ReqOptions.TestIDOptions) > 0 && c.ReqOptions.TestIDOptions[0].TestID != "" {
 		columnGroupByVariants = c.ReqOptions.VariantOption.DBGroupBy
 	}
 
-	// TODO: is this too slow?
-	err := json.Unmarshal([]byte(testIDStr), &test)
-	if err != nil {
-		return []crtest.RowIdentification{}, []crtest.ColumnID{}, err
-	}
-
-	testComponent, testCapabilities := componentAndCapabilityGetter(test, stats)
+	testComponent, testCapabilities := componentAndCapabilityGetter(stats)
 	rows := []crtest.RowIdentification{}
 	// First Page with no component requested
 	requestedComponent, requestedCapability, requestedTestID := "", "", ""
@@ -464,9 +457,13 @@ func (c *ComponentReportGenerator) getRowColumnIdentifications(testIDStr string,
 	} else if requestedComponent == testComponent {
 		// A component filter was specified and this test matches that component:
 
+		if stats.TestName == "" {
+			return rows, nil
+		}
+
 		row := crtest.RowIdentification{
 			Component: testComponent,
-			TestID:    test.TestID,
+			TestID:    stats.TestID,
 			TestName:  stats.TestName,
 			TestSuite: stats.TestSuite,
 		}
@@ -494,18 +491,14 @@ func (c *ComponentReportGenerator) getRowColumnIdentifications(testIDStr string,
 
 	columns := []crtest.ColumnID{}
 	column := crtest.ColumnIdentification{Variants: map[string]string{}}
-	for key, value := range test.Variants {
+	for key, value := range stats.Variants {
 		if columnGroupByVariants.Has(key) {
 			column.Variants[key] = value
 		}
 	}
-	columnKeyBytes, err := json.Marshal(column)
-	if err != nil {
-		return []crtest.RowIdentification{}, []crtest.ColumnID{}, err
-	}
-	columns = append(columns, crtest.ColumnID(columnKeyBytes))
+	columns = append(columns, column.Encode())
 
-	return rows, columns, nil
+	return rows, columns
 }
 
 type cellStatus struct {
@@ -638,10 +631,7 @@ func (c *ComponentReportGenerator) generateComponentTestReport(basisStatusMap, s
 		if !sampleThere {
 			status = basisStatus
 		}
-		testKey, err := utils.DeserializeTestKey(status, testKeyStr)
-		if err != nil {
-			return crtype.ComponentReport{}, err
-		}
+		testKey := utils.IdentificationFromStatus(status)
 
 		if !sampleThere {
 			// we use this to find tests associated with the basis that we don't see now in sample,
@@ -666,20 +656,14 @@ func (c *ComponentReportGenerator) generateComponentTestReport(basisStatusMap, s
 			}
 		}
 
-		rowIdentifications, columnIdentifications, err := c.getRowColumnIdentifications(testKeyStr, status)
-		if err != nil {
-			return crtype.ComponentReport{}, err
-		}
+		rowIdentifications, columnIdentifications := c.getRowColumnIdentifications(status)
 		updateCellStatus(
 			rowIdentifications, columnIdentifications, testKey, cellReport, includeAllTests, // inputs
 			aggregatedStatus, allRows, allColumns, // these three are maps to be updated
 		)
 	}
 
-	rows, err := buildReport(sortRowIdentifications(allRows), sortColumnIdentifications(allColumns), aggregatedStatus, includeAllTests)
-	if err != nil {
-		return crtype.ComponentReport{}, err
-	}
+	rows := buildReport(sortRowIdentifications(allRows), sortColumnIdentifications(allColumns), aggregatedStatus, includeAllTests)
 	return crtype.ComponentReport{Rows: rows}, nil
 }
 
@@ -715,7 +699,7 @@ func sortColumnIdentifications(allColumns map[crtest.ColumnID]struct{}) []crtest
 	return sortedColumns
 }
 
-func buildReport(sortedRows []crtest.RowIdentification, sortedColumns []crtest.ColumnID, aggregatedStatus map[crtest.RowIdentification]map[crtest.ColumnID]cellStatus, includeAllTests bool) ([]crtype.ReportRow, error) {
+func buildReport(sortedRows []crtest.RowIdentification, sortedColumns []crtest.ColumnID, aggregatedStatus map[crtest.RowIdentification]map[crtest.ColumnID]cellStatus, includeAllTests bool) []crtype.ReportRow {
 	// Now build the report
 	var regressionRows, goodRows []crtype.ReportRow
 	for _, rowID := range sortedRows {
@@ -729,11 +713,7 @@ func buildReport(sortedRows []crtest.RowIdentification, sortedColumns []crtest.C
 			if reportRow.Columns == nil {
 				reportRow.Columns = []crtype.ReportColumn{}
 			}
-			var colIDStruct crtest.ColumnIdentification
-			err := json.Unmarshal([]byte(columnID), &colIDStruct)
-			if err != nil {
-				return nil, err
-			}
+			colIDStruct := crtest.DecodeColumnID(columnID)
 			reportColumn := crtype.ReportColumn{ColumnIdentification: colIDStruct}
 			status, ok := columns[columnID]
 			if !ok {
@@ -766,7 +746,7 @@ func buildReport(sortedRows []crtest.RowIdentification, sortedColumns []crtest.C
 	}
 
 	regressionRows = append(regressionRows, goodRows...)
-	return regressionRows, nil
+	return regressionRows
 }
 
 func getRegressionStatus(basisPassPercentage, samplePassPercentage float64) crtest.Status {
@@ -938,7 +918,7 @@ func (c *ComponentReportGenerator) fischerExactTest(confidenceRequired, sampleFa
 func (c *ComponentReportGenerator) getUniqueJUnitColumnValuesLast60Days(ctx context.Context, field string,
 	nested bool) ([]string,
 	error) {
-	return c.dataProvider.QueryUniqueVariantValues(ctx, field, nested)
+	return c.dataProvider.QueryUniqueVariantValues(ctx, c.ReqOptions, field, nested)
 }
 
 func init() {