Skip to content

Commit d1cbe99

Browse files
fix: sort aggregation values to avoid deadlocks in process_pending (#1851)
* fix: sort aggregation values to avoid deadlocks in process_pending_aggregations and update_tree_listing * refactor: introduce TreeListingRow and CheckoutRow
1 parent 9af36b8 commit d1cbe99

3 files changed

Lines changed: 100 additions & 35 deletions

File tree

backend/kernelCI_app/management/commands/helpers/aggregation_helpers.py

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
import time
2-
from typing import Sequence
2+
from typing import Optional, Sequence
33

44

55
from django.db import connections
66
from kernelCI_app.helpers.logger import out
7+
from kernelCI_app.management.commands.helpers.tree_listing import (
8+
CheckoutRow,
9+
tree_listing_sort_key,
10+
)
711
from kernelCI_app.models import (
812
Checkouts,
913
PendingTest,
@@ -14,7 +18,6 @@
1418
Builds,
1519
)
1620
from kernelCI_app.utils import is_boot
17-
from typing import Optional
1821

1922

2023
def simplify_status(status: Optional[StatusChoices]) -> SimplifiedStatusChoices:
@@ -65,20 +68,23 @@ def update_tree_listing(checkouts_instances: Sequence[Checkouts]):
6568
return
6669

6770
t0 = time.time()
68-
checkout_values = [
69-
(
70-
checkout.id,
71-
checkout.origin,
72-
checkout.tree_name,
73-
checkout.git_repository_url,
74-
checkout.git_repository_branch,
75-
checkout.git_commit_hash,
76-
checkout.git_commit_name,
77-
checkout.git_commit_tags,
78-
checkout.start_time,
79-
)
80-
for checkout in checkouts_instances
81-
]
71+
checkout_values = sorted(
72+
[
73+
CheckoutRow(
74+
checkout_id=checkout.id,
75+
origin=checkout.origin,
76+
tree_name=checkout.tree_name,
77+
git_repository_url=checkout.git_repository_url,
78+
git_repository_branch=checkout.git_repository_branch,
79+
git_commit_hash=checkout.git_commit_hash,
80+
git_commit_name=checkout.git_commit_name,
81+
git_commit_tags=checkout.git_commit_tags,
82+
start_time=checkout.start_time,
83+
)
84+
for checkout in checkouts_instances
85+
],
86+
key=tree_listing_sort_key,
87+
)
8288

8389
with connections["default"].cursor() as cursor:
8490
# Set values as 0 when inserting a new tree
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
from datetime import datetime
2+
from typing import Any, NamedTuple
3+
4+
5+
class CheckoutRow(NamedTuple):
6+
checkout_id: str
7+
origin: str
8+
tree_name: str | None
9+
git_repository_url: str | None
10+
git_repository_branch: str | None
11+
git_commit_hash: str | None
12+
git_commit_name: str | None
13+
git_commit_tags: list[str] | None
14+
start_time: datetime | None
15+
16+
17+
class TreeListingRow(NamedTuple):
18+
"""Flat row passed to cursor.executemany() for UPDATE tree_listing.
19+
20+
Field order must match the SQL parameter placeholders in _process_tree_listing.
21+
"""
22+
23+
build_pass: int
24+
build_failed: int
25+
build_inc: int
26+
boot_pass: int
27+
boot_failed: int
28+
boot_inc: int
29+
test_pass: int
30+
test_failed: int
31+
test_inc: int
32+
origin: str
33+
tree_name: str | None
34+
git_repository_branch: str | None
35+
git_repository_url: str | None
36+
git_commit_hash: str | None
37+
38+
39+
def tree_listing_sort_key(v: Any) -> tuple:
40+
"""Sort key for tree_listing rows by unique constraint columns.
41+
42+
Ensures deterministic lock acquisition order in concurrent transactions,
43+
preventing deadlocks between the ingester and aggregation processor.
44+
Compatible with both CheckoutRow and TreeListingRow.
45+
"""
46+
return (
47+
(v.origin is None, v.origin or ""),
48+
(v.tree_name is None, v.tree_name or ""),
49+
(v.git_repository_url is None, v.git_repository_url or ""),
50+
(v.git_repository_branch is None, v.git_repository_branch or ""),
51+
(v.git_commit_hash is None, v.git_commit_hash or ""),
52+
)

backend/kernelCI_app/management/commands/process_pending_aggregations.py

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@
1414
from kernelCI_app.constants.general import MAESTRO_DUMMY_BUILD_PREFIX
1515
from kernelCI_app.helpers.logger import out
1616
from kernelCI_app.management.commands.helpers.aggregation_helpers import simplify_status
17+
from kernelCI_app.management.commands.helpers.tree_listing import (
18+
TreeListingRow,
19+
tree_listing_sort_key,
20+
)
1721
from kernelCI_app.constants.ingester import PROMETHEUS_MULTIPROC_DIR
1822
from prometheus_client import start_http_server
1923
from kernelCI_app.models import (
@@ -647,25 +651,28 @@ def _process_tree_listing(
647651
if not tree_listing_data:
648652
return
649653

650-
values = [
651-
(
652-
data["build_pass"],
653-
data["build_failed"],
654-
data["build_inc"],
655-
data["boot_pass"],
656-
data["boot_failed"],
657-
data["boot_inc"],
658-
data["test_pass"],
659-
data["test_failed"],
660-
data["test_inc"],
661-
data["origin"],
662-
data["tree_name"],
663-
data["git_repository_branch"],
664-
data["git_repository_url"],
665-
data["git_commit_hash"],
666-
)
667-
for data in tree_listing_data.values()
668-
]
654+
values = sorted(
655+
[
656+
TreeListingRow(
657+
build_pass=data["build_pass"],
658+
build_failed=data["build_failed"],
659+
build_inc=data["build_inc"],
660+
boot_pass=data["boot_pass"],
661+
boot_failed=data["boot_failed"],
662+
boot_inc=data["boot_inc"],
663+
test_pass=data["test_pass"],
664+
test_failed=data["test_failed"],
665+
test_inc=data["test_inc"],
666+
origin=data["origin"],
667+
tree_name=data["tree_name"],
668+
git_repository_branch=data["git_repository_branch"],
669+
git_repository_url=data["git_repository_url"],
670+
git_commit_hash=data["git_commit_hash"],
671+
)
672+
for data in tree_listing_data.values()
673+
],
674+
key=tree_listing_sort_key,
675+
)
669676

670677
t0 = time.time()
671678
with connection.cursor() as cursor:

0 commit comments

Comments
 (0)