Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Move test name column to build2test table #2148

Merged
merged 2 commits into from
May 8, 2024

Conversation

williamjallen
Copy link
Collaborator

The database schema currently has a build2test table which contains one row for each instance of a test run, in addition to a test table which contains deduplicated test names. The joins required when searching for build2test rows by test name, or for other relations by test name through the build2test relation, incur a significant amount of indexing overhead, and lead to poor query plans in some cases. The presence of a project ID column on the test relation also encourages poor query design, by applying the project filter to the test relation instead of the build relation.

This PR moves the test name to the build2test table, and drops the test table. This change is almost entirely internal, but has the potential to boost query performance in several places, as well as simplify future additions to the GraphQL API. The database migration associated with this change is large and may take a significant amount of time to apply on large CDash systems.

@sbelsk
Copy link
Collaborator

sbelsk commented Apr 29, 2024

Clicked around the site in order to test the functionality, and couldn't find anything broken. 👍

@williamjallen williamjallen added this pull request to the merge queue May 8, 2024
Merged via the queue into Kitware:master with commit 1870f63 May 8, 2024
6 checks passed
@williamjallen williamjallen deleted the remove-test-table branch May 8, 2024 14:17
github-merge-queue bot pushed a commit that referenced this pull request May 9, 2024
#1455 mistakenly used the test ID instead of the test output ID to find
labels for the queryTests page. This issue was separately resolved in
the 3.5 release series by the refactor in #2148.

Failures like this are a good motivation for our effort to move to a
GraphQL API. With GraphQL, there is a single source of truth about where
labels come from, and there isn't a need to separately query the labels
in dozens of different locations. These types of failures are also a
good motivation for more complete testing data, with most tables having
at least thousands of rows. With limited data, this failure went
unnoticed because each test ID corresponds to a test output ID in the
testing environment. This limited issue was resolved by the removal of
the test table in #2148.
github-merge-queue bot pushed a commit that referenced this pull request Sep 17, 2024
#2148 introduced a large migration which has proven to be problematic
for CDash instances with many tests. This PR breaks the single large
update statement into a batched update process where updates are
performed in groups of 5000 rows at a time.

Co-authored-by: Zack Galbreath <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants