Skip to content
117 changes: 15 additions & 102 deletions backend/kernelCI_app/helpers/hardwareDetails.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import bisect
import json
from collections import defaultdict
from datetime import datetime, timezone
Expand Down Expand Up @@ -28,7 +27,6 @@
misc_value_or_default,
)
from kernelCI_app.typeModels.commonDetails import (
BuildArchitectures,
BuildSummary,
EnvironmentMisc,
StatusCount,
Expand Down Expand Up @@ -290,8 +288,6 @@ def handle_test_history(
full_environment_misc: bool = False,
) -> None:
create_record_test_platform(record=record)
record_misc = sanitize_dict(record.get("misc"))

environment_misc_dict = get_environment_misc_value(
full_environment_misc=full_environment_misc,
parsed_environment_misc=record.get("parsed_environment_misc"),
Expand All @@ -313,11 +309,7 @@ def handle_test_history(
environment_misc=environment_misc,
tree_name=record["build__checkout__tree_name"],
git_repository_branch=record["build__checkout__git_repository_branch"],
lab=(
record_misc.get("runtime", UNKNOWN_STRING)
if record_misc
else UNKNOWN_STRING
),
lab=record.get("lab") or UNKNOWN_STRING,
)

task.append(test_history_item)
Expand Down Expand Up @@ -380,16 +372,14 @@ def handle_test_summary(
getattr(task.origins[origin], status) + 1,
)

misc = sanitize_dict(record.get("misc")) or {}
lab = misc.get("runtime", UNKNOWN_STRING)
if lab:
if task.labs.get(lab) is None:
task.labs[lab] = StatusCount()
setattr(
task.labs[lab],
status,
getattr(task.labs[lab], status) + 1,
)
lab = record.get("lab") or UNKNOWN_STRING
if task.labs.get(lab) is None:
task.labs[lab] = StatusCount()
setattr(
task.labs[lab],
status,
getattr(task.labs[lab], status) + 1,
)


def handle_build_history(
Expand All @@ -402,79 +392,7 @@ def handle_build_history(
builds.append(build)


def handle_build_summary(
*,
record: Dict,
builds_summary: BuildSummary,
issue_dict: Dict,
tree_index: int,
) -> None:
build: HardwareBuildHistoryItem = get_build_typed(record, tree_idx=tree_index)

status_key = build.status
setattr(
builds_summary.status,
status_key,
getattr(builds_summary.status, status_key) + 1,
)

if config := build.config_name:
build_config_summary = builds_summary.configs.get(config)
if not build_config_summary:
build_config_summary = StatusCount()
builds_summary.configs[config] = build_config_summary
setattr(
builds_summary.configs[config],
status_key,
getattr(builds_summary.configs[config], status_key) + 1,
)

if arch := build.architecture:
build_arch_summary = builds_summary.architectures.get(arch)
if not build_arch_summary:
build_arch_summary = BuildArchitectures()
builds_summary.architectures[arch] = build_arch_summary
setattr(
builds_summary.architectures[arch],
status_key,
getattr(builds_summary.architectures[arch], status_key) + 1,
)

compiler = build.compiler
if (
compiler is not None
and compiler not in builds_summary.architectures.get(arch).compilers
):
bisect.insort(builds_summary.architectures[arch].compilers, compiler)

if origin := build.origin:
build_origin_summary = builds_summary.origins.get(origin)
if not build_origin_summary:
build_origin_summary = StatusCount()
builds_summary.origins[origin] = build_origin_summary
setattr(
builds_summary.origins[origin],
status_key,
getattr(builds_summary.origins[origin], status_key) + 1,
)

misc = sanitize_dict(build.misc) or {}
lab = misc.get("lab", UNKNOWN_STRING)
if lab:
build_lab_summary = builds_summary.labs.get(lab)
if not build_lab_summary:
build_lab_summary = StatusCount()
builds_summary.labs[lab] = build_lab_summary
setattr(
builds_summary.labs[lab],
status_key,
getattr(builds_summary.labs[lab], status_key) + 1,
)

process_issue(record=record, task_issues_dict=issue_dict, issue_from="build")


# deprecated, use handle_build_history and handle_build_summary separately instead, with typing
# deprecated, use handle_build_history separately instead, with typing
def handle_build(*, instance, record: Dict, build: Dict) -> None:
instance.builds["items"].append(build)
update_issues(
Expand Down Expand Up @@ -579,8 +497,7 @@ def decide_if_is_full_record_filtered_out(
if not is_current_tree_selected:
return True

misc = sanitize_dict(record.get("misc")) or {}
lab = misc.get("runtime", UNKNOWN_STRING)
lab = record.get("lab") or UNKNOWN_STRING

is_record_filtered_out_result = instance.filters.is_record_filtered_out(
hardwares=record["environment_compatible"],
Expand Down Expand Up @@ -730,10 +647,8 @@ def process_filters(*, instance, record: Dict) -> None:

instance.unfiltered_origins["build"].add(record["build__origin"])

build_misc = sanitize_dict(record.get("build__misc")) or {}
build_lab = build_misc.get("lab")
if build_lab:
instance.unfiltered_labs["build"].add(build_lab)
if record.get("build_lab"):
instance.unfiltered_labs["build"].add(record["build_lab"])

if record["id"] is not None:
if is_boot(record["path"]):
Expand Down Expand Up @@ -771,10 +686,8 @@ def process_filters(*, instance, record: Dict) -> None:
platform_set.add(test_platform)
origin_set.add(record["test_origin"])

test_misc = sanitize_dict(record.get("misc")) or {}
test_lab = test_misc.get("runtime")
if test_lab:
instance.unfiltered_labs[flag_tab].add(test_lab)
if record.get("lab"):
instance.unfiltered_labs[flag_tab].add(record["lab"])


def is_record_tree_selected(*, record, tree: Tree, is_all_selected: bool) -> bool:
Expand Down
63 changes: 31 additions & 32 deletions backend/kernelCI_app/helpers/treeDetails.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
create_issue_typed,
extract_error_message,
is_boot,
sanitize_dict,
)


Expand Down Expand Up @@ -87,42 +86,41 @@ def get_current_row_data(
"test_number_value": current_row[10],
"test_misc": current_row[11],
"test_environment_compatible": current_row[tmp_test_env_comp_key],
"build_id": current_row[13],
"build_origin": current_row[14],
"build_comment": current_row[15],
"build_start_time": current_row[16],
"build_duration": current_row[17],
"build_architecture": current_row[18],
"build_command": current_row[19],
"build_compiler": current_row[20],
"build_config_name": current_row[21],
"build_config_url": current_row[22],
"build_log_url": current_row[23],
"build_status": current_row[24],
"build_misc": current_row[25],
"checkout_id": current_row[26],
"checkout_git_repository_url": current_row[27],
"checkout_git_repository_branch": current_row[28],
"checkout_git_commit_tags": current_row[29],
"checkout_origin": current_row[30],
"incident_id": current_row[31],
"incident_test_id": current_row[32],
"incident_present": current_row[33],
"issue_id": current_row[34],
"issue_version": current_row[35],
"issue_comment": current_row[36],
"issue_report_url": current_row[37],
"test_lab": current_row[13],
"build_id": current_row[14],
"build_origin": current_row[15],
"build_comment": current_row[16],
"build_start_time": current_row[17],
"build_duration": current_row[18],
"build_architecture": current_row[19],
"build_command": current_row[20],
"build_compiler": current_row[21],
"build_config_name": current_row[22],
"build_config_url": current_row[23],
"build_log_url": current_row[24],
"build_status": current_row[25],
"build_misc": current_row[26],
"build_lab": current_row[27],
"checkout_id": current_row[28],
"checkout_git_repository_url": current_row[29],
"checkout_git_repository_branch": current_row[30],
"checkout_git_commit_tags": current_row[31],
"checkout_origin": current_row[32],
"incident_id": current_row[33],
"incident_test_id": current_row[34],
"incident_present": current_row[35],
"issue_id": current_row[36],
"issue_version": current_row[37],
"issue_comment": current_row[38],
"issue_report_url": current_row[39],
}

parsed_environment_misc = handle_misc(
misc_value_or_default(current_row_data["test_environment_misc"])
)
current_row_data["test_platform"] = parsed_environment_misc.get("platform")
current_row_data["parsed_environment_misc"] = parsed_environment_misc
test_misc = sanitize_dict(current_row_data["test_misc"])
test_runtime_lab = UNKNOWN_STRING
if test_misc is not None:
test_runtime_lab = test_misc.get("runtime", UNKNOWN_STRING)
test_runtime_lab = current_row_data["test_lab"] or UNKNOWN_STRING

if current_row_data["test_status"] is None:
current_row_data["test_status"] = NULL_STATUS
Expand Down Expand Up @@ -495,8 +493,9 @@ def process_filters(instance, row_data: dict, skip_build_filters: bool = False)
instance.global_architectures.add(row_data["build_architecture"])
instance.global_compilers.add(row_data["build_compiler"])
instance.unfiltered_origins["build"].add(row_data["build_origin"])
if (build_misc := row_data["build_misc"]) is not None:
instance.unfiltered_labs["build"].add(build_misc.get("lab", UNKNOWN_STRING))
instance.unfiltered_labs["build"].add(
row_data.get("build_lab") or UNKNOWN_STRING
)

build_issue_id, build_issue_version, is_build_issue = (
should_increment_build_issue(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@
"log_url",
"log_excerpt",
"misc",
"lab_id",
"status",
],
"query": """
Expand All @@ -172,10 +173,11 @@
log_url,
log_excerpt,
misc,
lab_id,
status
)
VALUES (
%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s
%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s
)
ON CONFLICT (id)
DO UPDATE SET
Expand All @@ -193,6 +195,7 @@
log_url = COALESCE(builds.log_url, EXCLUDED.log_url),
log_excerpt = COALESCE(builds.log_excerpt, EXCLUDED.log_excerpt),
misc = COALESCE(builds.misc, EXCLUDED.misc),
lab_id = COALESCE(builds.lab_id, EXCLUDED.lab_id),
status = COALESCE(builds.status, EXCLUDED.status);
""",
},
Expand All @@ -213,6 +216,7 @@
"duration",
"output_files",
"misc",
"lab_id",
"number_value",
"environment_compatible",
"number_prefix",
Expand All @@ -236,14 +240,15 @@
duration,
output_files,
misc,
lab_id,
number_value,
environment_compatible,
number_prefix,
number_unit,
input_files
)
VALUES (
%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s
%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s
)
ON CONFLICT (id)
DO UPDATE SET
Expand All @@ -259,6 +264,7 @@
duration = COALESCE(tests.duration, EXCLUDED.duration),
output_files = COALESCE(tests.output_files, EXCLUDED.output_files),
misc = COALESCE(tests.misc, EXCLUDED.misc),
lab_id = COALESCE(tests.lab_id, EXCLUDED.lab_id),
number_value = COALESCE(tests.number_value, EXCLUDED.number_value),
environment_compatible = COALESCE(tests.environment_compatible, EXCLUDED.environment_compatible),
number_prefix = COALESCE(tests.number_prefix, EXCLUDED.number_prefix),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ def _standardize_lab_field(item: dict[str, Any], field: str) -> None:
"""
lab = item.get("misc", {}).get(field)
is_automatic = lab and AUTOMATIC_LABS.match(lab)
# Real lab for the lab_id FK, captured before the origin fallback (#1752) below
# overwrites misc, which would otherwise pollute the lab dimension.
item["_real_lab"] = None if is_automatic else lab
if is_automatic:
item["misc"][AUTOMATIC_LAB_FIELD] = lab
item["misc"].pop(field, None)
Expand Down Expand Up @@ -188,6 +191,39 @@ def prepare_file_data(
}


def assign_lab_ids(builds_buf: list[Builds], tests_buf: list[Tests]) -> None:
"""Resolve each instance's real lab name to a labs.id and set its lab_id FK.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the lab name is unique in the database, why not use that as a primary key? In that case you'd make a first query to check which labs are not in the database yet, a second query to update the table if needed, but you wouldn't need the third query to get the id of the newly added labs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly to avoid string keys, since this is an internal key, we might benefit more from integer indices.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I am worried that we are adding one query here, one query there and soon we will have a bloat of unnecessary queries in the ingester, but it's fine for now

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a valid concern. If we plain to move some operations to ingestion time, we certainly are going to end up increasing "ingestion complexity". But I believe that, as long as we keep the ingester fast enough, simplifying the analysis step should be the goal.


Select-first so we only INSERT genuinely new labs (avoids burning the id
sequence on every flush). New labs are committed outside the fact-insert
transaction (autocommit).
"""
objs = [*builds_buf, *tests_buf]
names = {name for obj in objs if (name := obj._lab_name)}

id_map: dict[str, int] = {}
if names:
with connections["default"].cursor() as cursor:
cursor.execute(
"SELECT id, name FROM labs WHERE name = ANY(%s)", [list(names)]
)
id_map = {name: lab_id for lab_id, name in cursor.fetchall()}

missing = [name for name in names if name not in id_map]
if missing:
cursor.executemany(
"INSERT INTO labs (name) VALUES (%s) ON CONFLICT (name) DO NOTHING",
[(name,) for name in missing],
)
cursor.execute(
"SELECT id, name FROM labs WHERE name = ANY(%s)", [missing]
)
id_map.update({name: lab_id for lab_id, name in cursor.fetchall()})

for obj in objs:
obj.lab_id = id_map.get(obj._lab_name) if obj._lab_name else None


def consume_buffer(buffer: list[TableModels], table_name: TableNames) -> None:
"""
Consume a buffer of items and insert them into the database.
Expand Down Expand Up @@ -245,6 +281,8 @@ def flush_buffers(
if total == 0:
return

assign_lab_ids(builds_buf, tests_buf)

# Insert in dependency-safe order
flush_start = time.time()
try:
Expand Down
Loading
Loading