feat: implement tree details denormalization with tests_rollup table#1795
feat: implement tree details denormalization with tests_rollup table#1795gustavobtflores wants to merge 4 commits intokernelci:mainfrom
Conversation
gustavobtflores
force-pushed
the
feat/tree-details-denormalization
branch
3 times, most recently
from
644025c to
0330580
Compare
MarceloRobert
added
Backend
|
|
||
| checkout = build.checkout | ||
| path = test.path or "" | ||
| path_group = path.split(".", 1)[0] if path else "-" |
There was a problem hiding this comment.
nit: we could build a helper function for this kind of string manipulation.
| issue_version = issue_info["issue_version"] if issue_info else None | ||
| issue_uncategorized = issue_id is None and test.full_status == "FAIL" | ||
|
|
||
| arch = build.architecture or "Unknown" |
There was a problem hiding this comment.
Prefer the UNKNOWN constant here.
Or (not for this PR) should we invest in some input sanitization? this way we could avoid Nones and other invalid states.
| for test in tests: | ||
| checkout = test.build.checkout | ||
| path = test.path or "" | ||
| path_group = path.split(".", 1)[0] if path else "-" |
There was a problem hiding this comment.
nit: we could build a helper function for this kind of string manipulation.
| config = test.build.config_name or "Unknown" | ||
| is_boot = bool(path) and path.startswith("boot") | ||
|
|
||
| rollup_key = ( |
There was a problem hiding this comment.
Not sure why we need to create a tuple, instead of using the Rollup object here.
|
|
||
|
|
||
| class TestsRollup(models.Model): | ||
| origin = models.TextField() |
There was a problem hiding this comment.
wouldn't varchar be preferable for the smaller strings?
There was a problem hiding this comment.
varchar and text have similar performance in Postgres, and the origin field varies in size, so I think keeping text here is okay
backend/kernelCI_app/management/commands/process_pending_aggregations.py
Outdated
Show resolved
Hide resolved
backend/kernelCI_app/management/commands/process_pending_aggregations.py
Outdated
Show resolved
Hide resolved
backend/kernelCI_app/management/commands/process_pending_aggregations.py
Show resolved
Hide resolved
|
|
||
| return incidents | ||
|
|
||
| def create_tests_rollup( |
There was a problem hiding this comment.
looks like this function could mostly be shared with the one in process_pending_aggregation
gustavobtflores
force-pushed
the
feat/tree-details-denormalization
branch
4 times, most recently
from
f03087d to
21a7f67
Compare
| status=simplify_status(t.status), | ||
| is_boot=is_boot(t.path) if t.path else False, | ||
| path=t.path, | ||
| duration=t.duration, |
There was a problem hiding this comment.
PendingTest has no duration field, this is breaking the ingester because the model is not expecting this field
There was a problem hiding this comment.
yeah, forgot to remove it after we decide to not have duration in the rollup table
gustavobtflores
force-pushed
the
feat/tree-details-denormalization
branch
from
82a0ed5 to
1ecfc60
Compare
gustavobtflores
force-pushed
the
feat/tree-details-denormalization
branch
from
1ecfc60 to
169c461
Compare
3 participants
Description
Implements a denormalization strategy for tree details by introducing a rollup table to improve query performance. This PR adds new database models and refactors the tree details summary view to use pre-aggregated data instead of computing statistics on-the-fly.
Changes
How to test
poetry run python manage.py migratepoetry run python manage.py process_pending_aggregationsRelated issues
Closes #1801