TRT-2800: Replace failed-tests matviews with direct partitioned query

[mstaeble]

Summary

• The prow_job_failed_tests_by_day and prow_job_failed_tests_by_hour materialized views scanned all ~3,500 partitions of prow_job_run_tests with no time or release filter, taking ~3 minutes per refresh.
• A direct query with partition pruning keys (prow_job_run_release + prow_job_run_timestamp BETWEEN start AND end) eliminates the matview refresh entirely and queries the partitioned table directly.
• Removes the matview definitions and replaces the matview read in PrintJobAnalysisJSONFromDB with a direct query using the release and time range already available from the caller.
• Adds error checking on the query result.
• Tested against staging database via the local UI — job analysis page renders test failure counts correctly.

Benchmarks (staging, release 4.22 with ~1,000 job IDs, enable_partitionwise_join = on)

Approach	Cold	Warm
Matview refresh (no filters)	~3 minutes	~3 minutes
Matview read	53ms	46ms
Direct query (release + 14-day range)	257ms	249ms

The direct query is slower than reading the pre-aggregated matview (249ms vs 46ms), but eliminates the 3-minute background refresh cycle. End-to-end page load times observed: ~2.5s cold after server restart, <1.5s warm.

Test plan

• Full project compiles with go build ./...
• go vet clean
• Manually tested job analysis page against staging database
• CI passes

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved job analysis accuracy by calculating failed-test trends directly from test run records.
  • Ensured failure results are consistently filtered by release, time range, and failure status.
• Performance
  • Removed obsolete failed-test aggregation views.
  • Updated benchmarking to skip the removed view-backed cases.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 22f2890d-3ab8-4bb7-99b9-ed86f165c49f

📥 Commits

Reviewing files that changed from the base of the PR and between 964972a and 3d2dcbd.

📒 Files selected for processing (3)

• pkg/api/job_analysis.go
• pkg/db/views.go
• pkg/flags/postgres_benchmarking_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• pkg/flags/postgres_benchmarking_test.go
• pkg/db/views.go
• pkg/api/job_analysis.go

---

Walkthrough

Failed-test analysis now queries prow_job_run_tests directly with date_trunc period grouping. The day/hour failed-test materialized views, their SQL template, and related benchmark cases are removed.

Changes

Failed-test aggregation migration

Layer / File(s)	Summary
Direct failed-test aggregation pkg/api/job_analysis.go	PrintJobAnalysisJSONFromDB joins prow_job_run_tests with tests, filters failures by release and timestamp, groups with date_trunc, and returns scan errors.
Remove obsolete materialized views and benchmarks pkg/db/views.go, pkg/flags/postgres_benchmarking_test.go	Removes the day/hour failed-test materialized-view registrations, SQL template, and benchmark cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: approved, lgtm

Suggested reviewers: stbenjam, deepsm007

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (3 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Go Error Handling	⚠️ Warning	The new scan error path returns raw errors (return results, err) instead of wrapping them with context via fmt.Errorf(...%w).	Wrap the new query failure path with context, e.g. return results, fmt.Errorf("failed to load test failures: %w", err).
Test Coverage For New Features	⚠️ Warning	The PR changes PrintJobAnalysisJSONFromDB and removes two matviews, but there are no unit/regression tests covering the new direct-query path; only benchmarks were removed.	Add a regression/unit test for job-analysis test-failure aggregation using the new direct query, covering day/hour grouping and release/time filters.
Stable And Deterministic Test Names	❓ Inconclusive	Gathering evidence from changed test files.	Need to inspect Ginkgo titles in the modified tests and verify whether any contain dynamic or overly specific strings.

✅ Passed checks (17 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: replacing failed-tests materialized views with a direct partitioned query.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Sql Injection Prevention	✅ Passed	New SQL in PrintJobAnalysisJSONFromDB uses placeholders for period, release, timestamps, and status; no user input is concatenated into SQL.
Excessive Css In React Should Use Styles	✅ Passed	PASS: The PR only changes Go files; no React components, JSX/TSX, or inline CSS/style objects are present.
Single Responsibility And Clear Naming	✅ Passed	Focused refactor: one direct query replaces two failed-test matviews, view registration is removed, and no broad/generic new types or packages were added.
Feature Documentation	✅ Passed	PASS: No docs/features page covers this job-analysis query path; the only feature doc is about symptoms, and the related TRT-1989 plan already documents the rewrite.
Test Structure And Quality	✅ Passed	PASS: The PR only changes a standard benchmark file and non-test code; no Ginkgo It/Describe blocks, cluster resources, or Eventually waits are added or modified.
Microshift Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the only test change is removing plain Go benchmark cases, so no MicroShift-incompatible APIs/features to flag.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the only touched test file is a Go benchmark test, and it contains no SNO-sensitive assumptions.
Topology-Aware Scheduling Compatibility	✅ Passed	Only query/view/benchmark code changed; no manifests, controllers, replicas, affinities, selectors, tolerations, or PDBs were introduced.
Ote Binary Stdout Contract	✅ Passed	No process-level stdout writes were added; touched files have no main/init/TestMain/BeforeSuite setup, and the only fmt.Print/logging is inside ordinary test helpers.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the changed files are API/DB code plus benchmark tests, with no IPv4-only or public-internet assumptions found.
No-Weak-Crypto	✅ Passed	Patch only changes SQL and benchmark wiring; no crypto primitives, weak ciphers, or secret comparisons were added.
Container-Privileges	✅ Passed	PR changes only Go source/tests; no container/K8s manifest files were modified and no privilege-related settings appear in the diff.
No-Sensitive-Data-In-Logs	✅ Passed	Patch only swaps SQL and removes matview registrations; no new log statements or sensitive literals were added in the diff.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mstaeble
Once this PR has been reviewed and has the lgtm label, please assign stbenjam for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

pkg/api/job_analysis.go (1)

112-114: 💤 Low value

Consider moving period validation earlier to ensure both queries use the same value.

This validation only affects the test-failure query (lines 116-127). The earlier job-runs aggregation (lines 51-67) passes period directly to GORM. If an invalid period reaches this function, the two queries could behave inconsistently. Moving the validation before line 51 would ensure uniform behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/api/job_analysis.go` around lines 112 - 114, The period validation logic
that checks if period is not equal to PeriodDay and PeriodHour (and defaults to
PeriodDay if invalid) is currently positioned after the job-runs aggregation
query. Move this validation block to the beginning of the function, before the
aggregation query that uses the period parameter, so that both the early
aggregation query and the later test-failure query operate with the same
validated period value and ensure consistent behavior throughout the function.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/api/job_analysis.go`:
- Line 124: The GROUP BY clause at line 124 includes prow_job_id, which causes
the query to return separate counts for each job. However, when aggregating
results later (line 132), the code uses assignment rather than addition, causing
per-job counts to be overwritten and discarded. To count total test failures
across all selected jobs per period, remove pjrt.prow_job_id from the Group()
method call in the query construction so the results are grouped only by test
name and date truncation without the job ID dimension.

---

Nitpick comments:
In `@pkg/api/job_analysis.go`:
- Around line 112-114: The period validation logic that checks if period is not
equal to PeriodDay and PeriodHour (and defaults to PeriodDay if invalid) is
currently positioned after the job-runs aggregation query. Move this validation
block to the beginning of the function, before the aggregation query that uses
the period parameter, so that both the early aggregation query and the later
test-failure query operate with the same validated period value and ensure
consistent behavior throughout the function.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: cf4f597c-12b0-43d8-bba6-3422519a9f0c

📥 Commits

Reviewing files that changed from the base of the PR and between 0ecf781 and d553fc3.

📒 Files selected for processing (3)

• pkg/api/job_analysis.go
• pkg/db/views.go
• pkg/flags/postgres_benchmarking_test.go

💤 Files with no reviewable changes (2)

• pkg/db/views.go
• pkg/flags/postgres_benchmarking_test.go

[openshift-merge-bot]

Scheduling required tests:
/test e2e

[openshift-ci-robot]

@mstaeble: This pull request references TRT-2800 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

• The prow_job_failed_tests_by_day and prow_job_failed_tests_by_hour materialized views scanned all ~3,500 partitions of prow_job_run_tests with no time or release filter, taking ~3 minutes per refresh.
• A direct query with partition pruning keys (prow_job_run_release + prow_job_run_timestamp BETWEEN start AND end) eliminates the matview refresh entirely and queries the partitioned table directly.
• Removes the matview definitions and replaces the matview read in PrintJobAnalysisJSONFromDB with a direct query using the release and time range already available from the caller.
• Adds error checking on the query result.
• Tested against staging database via the local UI — job analysis page renders test failure counts correctly.

Benchmarks (staging, release 4.22 with ~1,000 job IDs, enable_partitionwise_join = on)

Approach	Cold	Warm
Matview refresh (no filters)	~3 minutes	~3 minutes
Matview read	53ms	46ms
Direct query (release + 14-day range)	257ms	249ms

The direct query is slower than reading the pre-aggregated matview (249ms vs 46ms), but eliminates the 3-minute background refresh cycle. End-to-end page load times observed: ~2.5s cold after server restart, <1.5s warm.

Test plan

• Full project compiles with go build ./...
• go vet clean
• Manually tested job analysis page against staging database
• CI passes

🤖 Generated with Claude Code

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-merge-bot]

Scheduling required tests:
/test e2e

[openshift-ci]

@mstaeble: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.