Skip to content

Commit 64d82b1

Browse files
m-datim-dati
committed
feat(throttling): refactor the configuration logic
 move in a routine load_prio_throttling the format checking for throttling load the resulting hash after validated the throtting configuration and fix _load_prio_throttling,in Setup.pm calculate the resulting priority and update debug_message,in Jobs.pm update tests t/api/04-jobs.t and t/config.t to manage the throttling update the throttling description in openqa.ini fix comments in sub load_throttling_config apply changes requested in PR#6952 Ref. poo#192952
1 parent d668cbd commit 64d82b1

File tree

5 files changed

+120
-36
lines changed

5 files changed

+120
-36
lines changed

‎etc/openqa/openqa.ini‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -369,12 +369,11 @@ concurrent = 0
369369
## scheduled products without any jobs (e.g. due to errors) are not immediately
370370
## cleaned up.
371371
#scheduled_product_min_storage_duration = 34
372-
## Job resources throttling configuration:
373-
## a string of comma-separated jobs resources,
374-
## formatted like: "RES1:MAL1,RES2:MAL2,..."
375-
## where MAL# is a malus priority to multiply for RES value
376-
## PRIO1 = prio_base + int(RES1.val * MAL1)
377-
#prio_throttling_parameters
372+
## Throttling: set a comma-separated list of test parameters that trigger throttling
373+
## of scheduled openQA jobs by priority. Example: PARAM1:SCALE1[:THRESH1][,...]
374+
## Set a scale that a parameter value, minus an optional threshold, is multiplied by
375+
## and added to each job prio, as: '$base_prio + $scale# * ($param# - $thresh#)'
376+
#prio_throttling_parameters =
378377

379378
[archiving]
380379
## Moves logs of jobs which are preserved during the cleanup because they are

‎lib/OpenQA/Schema/ResultSet/Jobs.pm‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,6 @@ sub latest_jobs ($self, $until = undef) {
101101
return @latest;
102102
}
103103

104-
sub load_throttling_config ($config_string) {
105-
# parse to hash "{ PAR1 => MAL1, PAR2 => MAL2, ...}"
106-
# the prio_throttling_parameters configuration string; see openqa.ini
107-
return unless ($config_string && $config_string =~ /:/);
108-
$config_string =~ s/\s+//g;
109-
my %hash = map { my ($k, $v) = split /:/, $_, 2 } split /,/, $config_string;
110-
return \%hash;
111-
}
112-
113104
sub create_from_settings ($self, $settings, $scheduled_product_id = undef) {
114105
my %settings = %$settings;
115106
my %new_job_args;
@@ -167,17 +158,20 @@ sub create_from_settings ($self, $settings, $scheduled_product_id = undef) {
167158
$new_job_args{priority} += $malus;
168159
}
169160
}
170-
my $throttling = OpenQA::App->singleton->config->{misc_limits}->{prio_throttling_parameters};
171-
$throttling = load_throttling_config($throttling);
172161
# apply resources throttling control
173-
if ($throttling) {
162+
if (my $throttling = OpenQA::App->singleton->config->{misc_limits}->{prio_throttling_data}) {
163+
my $throttling_info;
174164
for my $resource (keys %$throttling) {
175165
next if !defined $settings{$resource};
176-
my $malus = int($settings{$resource} * $throttling->{$resource});
177-
$debug_msg = sprintf 'Adding priority malus to newly created job due to %s (old: %d, malus: %s)',
178-
$resource, $new_job_args{priority}, $malus;
179-
$new_job_args{priority} += $malus;
166+
my $scale = $throttling->{$resource}->{scale};
167+
my $reference = $throttling->{$resource}->{reference};
168+
my $prio = int(($settings{$resource} - $reference) * $scale);
169+
$throttling_info .= $resource . ", scale: $scale" . ($reference ? ", reference: $reference;" : ';');
170+
$new_job_args{priority} += $prio;
180171
}
172+
$debug_msg .= sprintf("\nAdjusting job priority by %s based on resource requirement(s): %s",
173+
$new_job_args{priority}, $throttling_info)
174+
if $throttling_info;
181175
}
182176

183177
my $job = $self->create(\%new_job_args);

‎lib/OpenQA/Setup.pm‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,29 @@ sub _load_config ($app, $defaults, $mode_specific_defaults) {
5454
_read_config_file($config, $config_file, $defaults, $mode_defaults);
5555
$config->{ini_config} = $config_file;
5656
}
57-
5857
# ensure default values are assigned; warn if config files were supplied at all
5958
_read_config_file($config, undef, $defaults, $mode_defaults);
6059
$app->log->warn('No configuration files supplied, will fallback to default configuration') unless $config_file;
6160
return $config;
6261
}
6362

63+
sub _load_prio_throttling ($app, $config) {
64+
return
65+
unless my $throttling = $config->{misc_limits}->{prio_throttling_parameters};
66+
# floating point number
67+
my $n = qr/[+-]?(?:\d+\.?\d*|\.\d+)/;
68+
# Unit format = Param.name: scale: reference(optional)
69+
my $u = qr/([A-Z_][A-Z0-9_]*):($n)(?::($n))?/i;
70+
$throttling =~ s/\s+//g;
71+
unless ($throttling =~ qr/^$u(?:,$u)*$/i) {
72+
$app->log->warn("Wrong format in openqa.ini 'prio_throttling_parameters': $throttling");
73+
return;
74+
}
75+
my %hash = map { my ($k, $s, $r) = $_ =~ /$u/g; $k => {scale => $s, reference => $r // 0} }
76+
split(',', $throttling);
77+
return \%hash;
78+
}
79+
6480
sub read_config ($app) {
6581
my %defaults = (
6682
global => {
@@ -244,6 +260,7 @@ sub read_config ($app) {
244260
max_job_time_prio_scale => 100,
245261
scheduled_product_min_storage_duration => 34,
246262
prio_throttling_parameters => '',
263+
prio_throttling_data => undef,
247264
},
248265
archiving => {
249266
archive_preserved_important_jobs => 0,
@@ -294,12 +311,7 @@ sub read_config ($app) {
294311
if (my $minion_fail_job_blocklist = $config->{influxdb}->{ignored_failed_minion_jobs}) {
295312
$config->{influxdb}->{ignored_failed_minion_jobs} = [split(/\s+/, $minion_fail_job_blocklist)];
296313
}
297-
my $throttling = $config->{misc_limits}->{prio_throttling_parameters};
298-
if ($throttling && length $throttling) {
299-
$config->{misc_limits}->{prio_throttling_parameters} =~ s/\s+//g;
300-
die("Wrong formatting for 'prio_throttling_parameters' in openqa.ini")
301-
unless ($throttling =~ /^[A-Z_]+:\d+(?:,[A-Z_]+:\d+)*$/i);
302-
}
314+
$config->{misc_limits}->{prio_throttling_data} = _load_prio_throttling($app, $config);
303315
my $results = delete $global_config->{parallel_children_collapsable_results};
304316
$global_config->{parallel_children_collapsable_results_sel}
305317
= ' .status' . join('', map { ":not(.result_$_)" } split(/\s+/, $results));

‎t/api/04-jobs.t‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,21 +1052,37 @@ subtest 'priority correctly assigned when posting job' => sub {
10521052
$t->json_is('/job/priority', 50, 'feature disabled: prio value unchanged');
10531053
};
10541054

1055-
subtest 'priority malus due to high QEMURAM demand' => sub {
1056-
my $add = 40;
1055+
subtest 'priority scaled up due to QEMURAM demand' => sub {
1056+
my $add = 20;
10571057
local $jobs_post_params{QEMURAM} = 4096;
1058-
my $limits = OpenQA::App->singleton->config->{misc_limits};
1059-
$limits->{prio_throttling_parameters} = 'QEMURAM:0.01';
1058+
1059+
my $config = OpenQA::Setup::read_config($t->app);
1060+
$config->{misc_limits}->{prio_throttling_parameters} = 'XXX :0.2, QEMURAM:0.01:2048';
1061+
$config->{misc_limits}->{prio_throttling_data} = OpenQA::Setup::_load_prio_throttling($t->app, $config);
10601062
$t->post_ok('/api/v1/jobs', form => \%jobs_post_params)->status_is(200);
10611063
$t->get_ok('/api/v1/jobs/' . $t->tx->res->json->{id})->status_is(200);
10621064
$t->json_is('/job/priority', 50 + $add, 'increased prio value');
10631065
};
10641066

1065-
subtest 'priority malus due to high HDDSIZEGB demand' => sub {
1067+
subtest 'priority improved due to QEMURAM low demand' => sub {
1068+
my $add = -10;
1069+
local $jobs_post_params{QEMURAM} = 1024;
1070+
1071+
my $config = OpenQA::Setup::read_config($t->app); # OpenQA::App->singleton->config;
1072+
$config->{misc_limits}->{prio_throttling_parameters} = 'XXX :0.2, QEMURAM:0.01:2048';
1073+
$config->{misc_limits}->{prio_throttling_data} = OpenQA::Setup::_load_prio_throttling($t->app, $config);
1074+
$t->post_ok('/api/v1/jobs', form => \%jobs_post_params)->status_is(200);
1075+
$t->get_ok('/api/v1/jobs/' . $t->tx->res->json->{id})->status_is(200);
1076+
$t->json_is('/job/priority', 50 + $add, 'decreased prio value');
1077+
};
1078+
1079+
subtest 'priority scaled up due to HDDSIZEGB demand' => sub {
10661080
my $add = 2;
10671081
local $jobs_post_params{HDDSIZEGB} = 40;
1068-
my $limits = OpenQA::App->singleton->config->{misc_limits};
1069-
$limits->{prio_throttling_parameters} = 'XXX :0.2, FAKE_HDDSIZEGB:0.01, HDDSIZEGB:0.05, YYY: 0.1';
1082+
my $config = OpenQA::Setup::read_config($t->app); # OpenQA::App->singleton->config;
1083+
$config->{misc_limits}->{prio_throttling_parameters}
1084+
= 'XXX :0.2, FAKE_HDDSIZEGB:0.01, HDDSIZEGB:0.05, YYY: 0.1';
1085+
$config->{misc_limits}->{prio_throttling_data} = OpenQA::Setup::_load_prio_throttling($t->app, $config);
10701086
$t->post_ok('/api/v1/jobs', form => \%jobs_post_params)->status_is(200);
10711087
$t->get_ok('/api/v1/jobs/' . $t->tx->res->json->{id})->status_is(200);
10721088
$t->json_is('/job/priority', 50 + $add, 'increased prio value');

‎t/config.t‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use lib "$FindBin::Bin/lib", "$FindBin::Bin/../external/os-autoinst-common/lib";
99
use Test::Warnings ':report_warnings';
1010
use Test::Output 'combined_like';
1111
use Test::MockModule;
12+
use Test::MockObject;
1213
use Mojolicious;
1314
use Mojo::Base -signatures;
1415
use Mojo::Log;
@@ -216,6 +217,7 @@ subtest 'Test configuration default modes' => sub {
216217
$test_config->{_openid_secret} = $config->{_openid_secret};
217218
$test_config->{logging}->{level} = 'debug';
218219
$test_config->{global}->{service_port_delta} = 2;
220+
$test_config->{misc_limits}->{prio_throttling_data} = undef;
219221
is ref delete $config->{global}->{auto_clone_regex}, 'Regexp', 'auto_clone_regex parsed as regex';
220222
ok delete $config->{'test_preset example'}, 'default values for example tests assigned';
221223
is_deeply $config, $test_config, '"test" configuration';
@@ -393,4 +395,65 @@ subtest 'Lookup precedence/hiding' => sub {
393395
is_deeply lookup_config_files(@args), \@expected, 'drop-in in overriden dir hides all other config';
394396
};
395397

398+
subtest 'check throttling configuration validation and application' => sub {
399+
my $log_mock = Test::MockObject->new();
400+
my $warn_called = 0;
401+
my $warn_msg;
402+
my $app_mock = Test::MockObject->new();
403+
my $quiet_log = Mojo::Log->new(level => 'warn');
404+
405+
$log_mock->mock('warn', sub { $warn_called = 1; });
406+
$app_mock->mock('log', sub { $log_mock });
407+
408+
OpenQA::App->set_singleton(my $app = Mojolicious->new(log => $quiet_log));
409+
my $config = OpenQA::Setup::read_config($app);
410+
411+
subtest 'invalid prio_throttling_parameters' => sub {
412+
$warn_called = 0;
413+
$config->{misc_limits}->{prio_throttling_parameters} = 'invalid';
414+
$config->{misc_limits}->{prio_throttling_data} = OpenQA::Setup::_load_prio_throttling($app_mock, $config);
415+
is_deeply $config->{misc_limits}->{prio_throttling_data}, undef,
416+
'prio_throttling_data is empty hash for invalid';
417+
is $warn_called, 1, 'warning called for invalid format';
418+
};
419+
420+
subtest 'no prio_throttling_parameters' => sub {
421+
$warn_called = 0;
422+
$config->{misc_limits}->{prio_throttling_parameters} = undef;
423+
$config->{misc_limits}->{prio_throttling_data} = OpenQA::Setup::_load_prio_throttling($app_mock, $config);
424+
is $config->{misc_limits}->{prio_throttling_data}, undef, 'prio_throttling_data is undef when no parameters';
425+
is $warn_called, 0, 'no warning for undef';
426+
};
427+
428+
subtest 'empty string' => sub {
429+
$warn_called = 0;
430+
$config->{misc_limits}->{prio_throttling_parameters} = '';
431+
$config->{misc_limits}->{prio_throttling_data} = OpenQA::Setup::_load_prio_throttling($app_mock, $config);
432+
is $config->{misc_limits}->{prio_throttling_data}, undef, 'prio_throttling_data is undef for empty string';
433+
is $warn_called, 0, 'no warning for empty';
434+
};
435+
436+
subtest 'valid prio_throttling_parameter with space separators' => sub {
437+
$warn_called = 0;
438+
$config->{misc_limits}->{prio_throttling_parameters} = 'KEY1ONE: 1.5';
439+
$config->{misc_limits}->{prio_throttling_data} = OpenQA::Setup::_load_prio_throttling($app_mock, $config);
440+
is_deeply $config->{misc_limits}->{prio_throttling_data},
441+
{KEY1ONE => {scale => 1.5, reference => 0}},
442+
'prio_throttling_data parsed correctly';
443+
};
444+
445+
subtest 'valid multiple prio_throttling_parameters keys' => sub {
446+
$warn_called = 0;
447+
$config->{misc_limits}->{prio_throttling_parameters} = 'A:1.04,B:2:3,C:0.04:5';
448+
$config->{misc_limits}->{prio_throttling_data} = OpenQA::Setup::_load_prio_throttling($app_mock, $config);
449+
is_deeply $config->{misc_limits}->{prio_throttling_data},
450+
{
451+
A => {scale => 1.04, reference => 0},
452+
B => {scale => 2, reference => 3},
453+
C => {scale => 0.04, reference => 5}
454+
},
455+
'prio_throttling_data parses multiple correctly';
456+
};
457+
};
458+
396459
done_testing();

0 commit comments

Comments
 (0)