Skip to content

Commit 49f42c8

Browse files
committed
perf: handle huge job groups gracefully
- Optimize compute_build_results by using database-level aggregation instead of fetching and iterating all job objects. - Move job deduplication (per scenario) to the database level. - Implement a safety limit of 5,000 jobs per build to prevent timeouts. - Optimize comment/review tracking by only checking failed jobs. - Add t/61-job_group_aggregation.t to verify aggregation and limit enforcement. - Extract category mapping into _get_job_result_category helper. - Reuse helper in both count_job and _count_job_aggregated. - Ensure consistent result categorization across legacy and optimized paths. - Add job_group_overview_max_jobs to misc_limits in openqa.ini. - Pass this limit from web and API controllers to compute_build_results. - De-duplicate common job data in t/61-job_group_aggregation.t. - Add controller and API tests for limit enforcement. Related progress issue: https://progress.opensuse.org/issues/196913
1 parent b82f14f commit 49f42c8

6 files changed

Lines changed: 278 additions & 64 deletions

File tree

lib/OpenQA/BuildResults.pm

Lines changed: 121 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ use OpenQA::Log qw(log_error);
1313
use Date::Format;
1414
use Sort::Versions;
1515
use Time::Seconds;
16+
use List::Util 'any';
17+
18+
use constant DEFAULT_MAX_JOBS_PER_BUILD => 5000;
1619

1720
sub init_job_figures ($job_result) {
1821

@@ -30,41 +33,41 @@ sub init_job_figures ($job_result) {
3033
$job_result->{total} = 0;
3134
}
3235

36+
sub _get_job_result_category ($state, $result) {
37+
if ($state eq OpenQA::Jobs::Constants::DONE) {
38+
return 'passed' if $result eq OpenQA::Jobs::Constants::PASSED;
39+
return 'softfailed' if $result eq OpenQA::Jobs::Constants::SOFTFAILED;
40+
return 'skipped' if any { $result eq $_ } OpenQA::Jobs::Constants::ABORTED_RESULTS;
41+
return 'failed' if any { $result eq $_ } OpenQA::Jobs::Constants::NOT_OK_RESULTS;
42+
}
43+
return 'skipped' if $state eq OpenQA::Jobs::Constants::CANCELLED;
44+
return 'unfinished';
45+
}
46+
47+
sub _count_job_aggregated ($stat, $jr, $count) {
48+
$jr->{total} += $count;
49+
my $category = _get_job_result_category($stat->state, $stat->result);
50+
$jr->{$category} += $count;
51+
}
52+
3353
sub count_job ($job, $jr, $labels) {
3454

3555
$jr->{total}++;
36-
if ($job->state eq OpenQA::Jobs::Constants::DONE) {
37-
if ($job->result eq OpenQA::Jobs::Constants::PASSED) {
38-
$jr->{passed}++;
39-
return;
40-
}
41-
if ($job->result eq OpenQA::Jobs::Constants::SOFTFAILED) {
42-
$jr->{softfailed}++;
43-
return;
44-
}
45-
if (grep { $job->result eq $_ } OpenQA::Jobs::Constants::ABORTED_RESULTS) {
46-
$jr->{skipped}++;
47-
return;
48-
}
49-
if (grep { $job->result eq $_ } OpenQA::Jobs::Constants::NOT_OK_RESULTS) {
50-
my $comment_data = $labels->{$job->id};
51-
$jr->{failed}++;
52-
if ($comment_data) {
53-
$jr->{labeled}++ if $comment_data->{reviewed};
54-
$jr->{comments}++ if $comment_data->{comments} || $comment_data->{reviewed};
55-
}
56-
return;
56+
my ($state, $result) = ($job->state, $job->result);
57+
my $category = _get_job_result_category($state, $result);
58+
$jr->{$category}++;
59+
60+
if ($category eq 'failed') {
61+
my $comment_data = $labels->{$job->id};
62+
if ($comment_data) {
63+
$jr->{labeled}++ if $comment_data->{reviewed};
64+
$jr->{comments}++ if $comment_data->{comments} || $comment_data->{reviewed};
5765
}
58-
# note: Incompletes and timeouts are accounted to both categories - failed and skipped.
5966
}
60-
if ($job->state eq OpenQA::Jobs::Constants::CANCELLED) {
61-
$jr->{skipped}++;
62-
return;
67+
elsif ($category eq 'unfinished') {
68+
log_error('Encountered not-implemented state:' . $state . ' result:' . $result)
69+
unless any { $state eq $_ } (OpenQA::Jobs::Constants::PENDING_STATES);
6370
}
64-
my $state = $job->state;
65-
log_error('Encountered not-implemented state:' . $job->state . ' result:' . $job->result)
66-
unless grep { /$state/ } (OpenQA::Jobs::Constants::PENDING_STATES);
67-
$jr->{unfinished}++;
6871
return;
6972
}
7073

@@ -111,13 +114,17 @@ sub find_child_groups ($group, $subgroup_filter) {
111114
return filter_subgroups($group, $subgroup_filter);
112115
}
113116

114-
sub compute_build_results ($group, $limit, $time_limit_days, $tags, $subgroup_filter, $show_tags) {
117+
sub compute_build_results ($group, $limit, $time_limit_days, $tags, $subgroup_filter, $show_tags,
118+
$max_jobs_per_build = undef)
119+
{
115120

116121
# find relevant child groups taking filter into account
117122
my $child_groups = find_child_groups($group, $subgroup_filter);
118123
my $group_ids = $child_groups->{group_ids};
119124
my $children = $child_groups->{children};
120125

126+
$max_jobs_per_build //= DEFAULT_MAX_JOBS_PER_BUILD;
127+
121128
my @sorted_results;
122129
my %result = (
123130
build_results => \@sorted_results,
@@ -185,14 +192,24 @@ sub compute_build_results ($group, $limit, $time_limit_days, $tags, $subgroup_fi
185192
last if defined($limit) && (--$limit < 0);
186193

187194
my ($version, $buildnr) = ($build->VERSION, $build->BUILD);
188-
my $jobs = $jobs_resultset->search(
195+
my $jobs_search_filter = {
196+
VERSION => $version,
197+
BUILD => $buildnr,
198+
group_id => {in => $group_ids},
199+
clone_id => undef,
200+
};
201+
my $latest_job_ids_rs = $jobs_resultset->search($jobs_search_filter,
202+
{select => [{max => 'id'}], as => [qw(id)], group_by => [qw(TEST ARCH FLAVOR MACHINE)]});
203+
my @latest_ids = grep { defined } map { $_->get_column('id') } $latest_job_ids_rs->all;
204+
next unless @latest_ids;
205+
206+
my $stats_rs = $jobs_resultset->search(
207+
{id => {-in => \@latest_ids}},
189208
{
190-
VERSION => $version,
191-
BUILD => $buildnr,
192-
group_id => {in => $group_ids},
193-
clone_id => undef,
194-
},
195-
{order_by => 'me.id DESC'});
209+
select => [qw(state result group_id DISTRI), {count => '*'}],
210+
as => [qw(state result group_id DISTRI count)],
211+
group_by => [qw(state result group_id DISTRI)],
212+
});
196213
my %jr = (
197214
key => $build->{key},
198215
build => $buildnr,
@@ -201,34 +218,80 @@ sub compute_build_results ($group, $limit, $time_limit_days, $tags, $subgroup_fi
201218
);
202219
init_job_figures(\%jr);
203220
for my $child (@$children) {
204-
init_job_figures($jr{children}->{$child->id} = {});
221+
init_job_figures($jr{children}->{$child->id} = {version => $version});
205222
}
206-
207-
my %seen;
208-
my @jobs = map {
209-
my $key = $_->TEST . '-' . $_->ARCH . '-' . $_->FLAVOR . '-' . ($_->MACHINE // '');
210-
$seen{$key}++ ? () : $_;
211-
} $jobs->all;
212-
next unless @jobs;
213-
my $comment_data = $group->result_source->schema->resultset('Comments')->comment_data_for_jobs(\@jobs);
214-
for my $job (@jobs) {
215-
$jr{distris}->{$job->DISTRI} = 1;
223+
my $total_for_build = 0;
224+
while (my $stat = $stats_rs->next) {
225+
my $count = $stat->get_column('count');
226+
$total_for_build += $count;
227+
_count_job_aggregated($stat, \%jr, $count);
228+
my $distri = $stat->get_column('DISTRI');
229+
$jr{distris}->{$distri} = 1;
230+
if ($jr{children}) {
231+
my $child = $jr{children}->{$stat->group_id};
232+
_count_job_aggregated($stat, $child, $count);
233+
$child->{distris}->{$distri} = 1;
234+
}
235+
}
236+
if ($total_for_build > $max_jobs_per_build) {
237+
die 'Build '
238+
. $buildnr . ' has '
239+
. $total_for_build
240+
. ' jobs, which exceeds the limit of '
241+
. $max_jobs_per_build
242+
. '. Please contact your openQA administrator.';
243+
}
244+
next unless $total_for_build;
245+
my $extra_info_rs = $jobs_resultset->search(
246+
{id => {-in => \@latest_ids}},
247+
{
248+
select => [{($newest ? 'max' : 'min') => 't_created'}],
249+
as => [qw(t_created)],
250+
});
251+
while (my $info = $extra_info_rs->next) {
252+
my $t_created = $info->get_column('t_created');
253+
if ($t_created && !ref $t_created) {
254+
require DateTime::Format::Pg;
255+
$t_created = DateTime::Format::Pg->parse_datetime($t_created);
256+
}
216257
if ($newest) {
217-
$jr{oldest_newest} //= $job->t_created;
258+
$jr{oldest_newest} //= $t_created;
218259
}
219260
else {
220-
$jr{oldest_newest} = $job->t_created;
261+
$jr{oldest_newest} = $t_created;
221262
}
222-
count_job($job, \%jr, $comment_data);
223-
if ($jr{children}) {
224-
my $child = $jr{children}->{$job->group_id};
225-
$child->{distris}->{$job->DISTRI} = 1;
226-
$child->{version} //= $job->VERSION;
227-
$child->{build} //= $job->BUILD;
228-
count_job($job, $child, $comment_data);
229-
add_review_badge($child);
263+
}
264+
my $not_ok_rs = $jobs_resultset->search(
265+
{
266+
id => {-in => \@latest_ids},
267+
state => OpenQA::Jobs::Constants::DONE,
268+
result => {in => [OpenQA::Jobs::Constants::NOT_OK_RESULTS]},
269+
},
270+
{select => [qw(id group_id)]});
271+
my @not_ok_jobs = $not_ok_rs->all;
272+
my $comment_data = $group->result_source->schema->resultset('Comments')->comment_data_for_jobs(\@not_ok_jobs);
273+
for my $job (@not_ok_jobs) {
274+
my $cd = $comment_data->{$job->id};
275+
next unless $cd;
276+
if ($cd->{reviewed}) {
277+
$jr{labeled}++;
278+
if ($jr{children} && (my $child = $jr{children}->{$job->group_id})) {
279+
$child->{labeled}++;
280+
}
281+
}
282+
if ($cd->{comments} || $cd->{reviewed}) {
283+
$jr{comments}++;
284+
if ($jr{children} && (my $child = $jr{children}->{$job->group_id})) {
285+
$child->{comments}++;
286+
}
230287
}
231288
}
289+
if ($jr{children}) {
290+
for my $child_id (keys %{$jr{children}}) {
291+
add_review_badge($jr{children}->{$child_id});
292+
}
293+
}
294+
232295
$jr{date} = delete $jr{oldest_newest};
233296
$jr{escaped_version} = $jr{version};
234297
$jr{escaped_version} =~ s/\W/_/g;

lib/OpenQA/Setup.pm

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ sub read_config ($app) {
242242
generic_default_limit => 10000,
243243
generic_max_limit => 100000,
244244
tests_overview_max_jobs => 2000,
245+
job_group_overview_max_jobs => 5000,
245246
all_tests_default_finished_jobs => 500,
246247
all_tests_max_finished_jobs => 5000,
247248
list_templates_default_limit => 5000,

lib/OpenQA/WebAPI/Controller/API/V1/JobGroup.pm

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -400,10 +400,18 @@ sub build_results ($self) {
400400
my $show_tags = $validation->param('show_tags') // $only_tagged;
401401

402402
my $tags = $show_tags ? $group->tags : undef;
403-
my $cbr
404-
= OpenQA::BuildResults::compute_build_results($group, $limit_builds,
405-
$time_limit_days, $only_tagged ? $tags : undef,
406-
[], $tags);
403+
my $max_jobs_per_build = $self->app->config->{misc_limits}->{job_group_overview_max_jobs};
404+
my $cbr;
405+
try {
406+
$cbr
407+
= OpenQA::BuildResults::compute_build_results($group, $limit_builds,
408+
$time_limit_days, $only_tagged ? $tags : undef,
409+
[], $tags, $max_jobs_per_build);
410+
}
411+
catch ($e) {
412+
die $e unless $e =~ qr/^(invalid regex: |Build .* has .* jobs, which exceeds the limit)/;
413+
return $self->render(json => {error => "$e"}, status => 400);
414+
}
407415
$self->render(json => $cbr);
408416
}
409417

lib/OpenQA/WebAPI/Controller/Main.pm

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,15 @@ sub _group_overview ($self, $resultset, $template) {
134134

135135
my $tags = $group->tags;
136136
my $cbr;
137+
my $max_jobs_per_build = $self->app->config->{misc_limits}->{job_group_overview_max_jobs};
137138
try {
138139
$cbr
139140
= OpenQA::BuildResults::compute_build_results($group, $limit_builds,
140141
$time_limit_days, $only_tagged ? $tags : undef,
141-
$group_params, $tags);
142+
$group_params, $tags, $max_jobs_per_build);
142143
}
143144
catch ($e) {
144-
die $e unless $e =~ qr/^invalid regex: /;
145+
die $e unless $e =~ qr/^(invalid regex: |Build .* has .* jobs, which exceeds the limit)/;
145146
return $self->_respond_error_for_group_overview($e);
146147
}
147148
my $build_results = $cbr->{build_results};

0 commit comments

Comments
 (0)