Report: improve generation performance, fix y-axis filtering, and correct config applicability detection#347
Open
ol-nata wants to merge 8 commits into
Open
Report: improve generation performance, fix y-axis filtering, and correct config applicability detection#347ol-nata wants to merge 8 commits into
ol-nata wants to merge 8 commits into
Conversation
Determine report configuration applicability based on the tests it configures rather than `test_names_order`, which is intended only for sorting. Signed-off-by: Natalia Rybchenko <natalia.rybchenko@oktetlabs.ru>
Treat `test_names_order` as optional when processing report configurations, allowing report configurations without an explicit test order. Signed-off-by: Natalia Rybchenko <natalia.rybchenko@oktetlabs.ru>
Make `test_names_order` optional in the report configuration schema, allowing report configurations without an explicit test order. Signed-off-by: Natalia Rybchenko <natalia.rybchenko@oktetlabs.ru>
Y-axis config entries (and their nested fields) could previously be empty at the schema level, which resulted in either an empty report for the corresponding test or, due to a filtering bug, a report showing everything unfiltered. The schema now forbids this. Signed-off-by: Natalia Rybchenko <natalia.rybchenko@oktetlabs.ru>
Add a configuration reformatting step to remove empty y-axis values from existing report configs, so they remain valid under the newly tightened schema. Signed-off-by: Natalia Rybchenko <natalia.rybchenko@oktetlabs.ru>
Filtering by y-axis-measurement values and by arguments whose matching iterations should be excluded from the report, and assembling the final report queryset from each test's results, each built their result by materializing a separate queryset per condition and combining them with `union()`, producing a query plan with a `UNION` branch for every one of them. Rewrite all three to build a single condition using per-condition subqueries, then apply it in one `filter()`/`exclude()` call. Related to: ts-factory#346 Signed-off-by: Natalia Rybchenko <natalia.rybchenko@oktetlabs.ru>
Checking whether a test had any results left after filtering fetched the entire queryset just to test its truthiness. Use `exists()` instead, which only needs a single lightweight query. Related to: ts-factory#346 Signed-off-by: Natalia Rybchenko <natalia.rybchenko@oktetlabs.ru>
Improve report generation performance by eliminating redundant per-result database queries when building `ReportPoint` objects. Use `select_related` for `measurement` and `result.iteration.test`, and `prefetch_related` for `result.iteration.test_arguments` and `measurement.metas`. Closes: ts-factory#346 Signed-off-by: Natalia Rybchenko <natalia.rybchenko@oktetlabs.ru>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Info
Report generation was extremely slow for runs with many measurement results — slow enough to hit the default 10-minute proxy timeout and never complete (#346). This PR fixes the underlying query patterns causing that, and separately hardens the report config schema against a few states that silently produced broken or misleading reports.
Closes: #346
Overview of changes
Performance
union()of per-condition querysets, producing a query plan with aUNIONbranch for every condition. Rewritten to build a single condition from per-condition subqueries and apply it in onefilter()/exclude()call.if not qs:) materialized the entire queryset just to test truthiness. Replaced withexists().ReportPointissued several queries per result (iteration, test, measurement, metas, test arguments) — classic N+1. Fixed withselect_related/prefetch_relatedonmeasurement,result.iteration.test,result.iteration.test_arguments, andmeasurement.metas.Y-axis filtering fix
Y-axis config entries (and nested fields) could previously be empty at the schema level. An empty entry produced no filtering at all for the corresponding test, silently showing every result instead of the intended subset. The schema now forbids empty y-axis entries, and a reformat step cleans up existing stored configs so they remain valid under the tightened schema.
Config applicability fix
Report config applicability for a run was determined from
test_names_order, a field meant only for sorting, which could be absent or unrelated to the tests the config actually configures. This could cause a config to be wrongly reported as applicable, or a valid config to be missed. Applicability is now determined from the tests the config actually configures.Optional test ordering
test_names_orderis now optional in the schema and treated as optional wherever it's read, so report configs no longer need to specify an explicit test order.Deployment
./scripts/deploy --steps per_project_conf run_services