Skip to content

Throttle openQA jobs resources based on test parameters size#6952

Merged
mergify[bot] merged 2 commits intoos-autoinst:masterfrom
m-dati:limit_resources_1
Feb 10, 2026
Merged

Throttle openQA jobs resources based on test parameters size#6952
mergify[bot] merged 2 commits intoos-autoinst:masterfrom
m-dati:limit_resources_1

Conversation

@m-dati
Copy link
Contributor

@m-dati m-dati commented Jan 29, 2026

Define a priority component for some resources to control, to be added to a job priority setting at test start time, to limit the jobs resources requests.

Store the configuration for all the needed resources in openqa.ini file, in prio_throttling_parameters parameter, in a string of a format described in the ticket: "param1:scale1[:reference1],param2:scale2,...", to load the control values at openQA server start.

@m-dati m-dati self-assigned this Jan 29, 2026
@m-dati m-dati force-pushed the limit_resources_1 branch 2 times, most recently from d533344 to 8773335 Compare January 29, 2026 22:14
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

In your commit message please write

Related progress issue: https://progress.opensuse.org/issues/192952

Please avoid the wording "malus" as it sounds so negative :) What we do is scale priority basically in both directions

@m-dati m-dati force-pushed the limit_resources_1 branch from 8773335 to 0ff3dc0 Compare January 30, 2026 18:55
@perlpunk
Copy link
Contributor

perlpunk commented Feb 2, 2026

it seems that https://app.circleci.com/pipelines/github/os-autoinst/openQA/19199/workflows/5a5b04f1-4a01-4606-8df0-4208907ae0b1/jobs/190404

Can't call method "config" on an undefined value at /home/squamata/project/lib/OpenQA/Schema/ResultSet/Jobs.pm line 170.
[19:01:53] t/ui/10-tests_overview.t ................... 
Dubious, test returned 255 (wstat 65280, 0xff00)
No subtests run 

is the same problem as the one we fixed in 21-needles.t.
In this case, just put a new Test::Mojo app before prepare_database:

my $t = Test::Mojo->new('OpenQA::WebAPI');
prepare_database;

and add it to the same commit as the 21-needles.t fix

@m-dati m-dati force-pushed the limit_resources_1 branch 2 times, most recently from 0e67e72 to 2108ec9 Compare February 2, 2026 22:53
@codecov
Copy link

codecov bot commented Feb 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.56%. Comparing base (ce2072d) to head (3557cb4).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6952   +/-   ##
=======================================
  Coverage   99.56%   99.56%           
=======================================
  Files         416      416           
  Lines       42396    42474   +78     
=======================================
+ Hits        42210    42288   +78     
  Misses        186      186           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@m-dati
Copy link
Contributor Author

m-dati commented Feb 3, 2026

Going to apply the last discussed throttling logic, that includes a reference value, as described in ticket note-10

@m-dati m-dati force-pushed the limit_resources_1 branch from 2108ec9 to 37e3bef Compare February 3, 2026 22:01
@m-dati m-dati force-pushed the limit_resources_1 branch from 37e3bef to 257e135 Compare February 3, 2026 22:23
@m-dati m-dati force-pushed the limit_resources_1 branch from 257e135 to b380acb Compare February 3, 2026 22:41
@m-dati m-dati force-pushed the limit_resources_1 branch from b380acb to ad9bd8d Compare February 3, 2026 22:47
@m-dati m-dati force-pushed the limit_resources_1 branch 3 times, most recently from 8c0125e to 64d82b1 Compare February 8, 2026 14:38
@okurz
Copy link
Member

okurz commented Feb 8, 2026

The changes look good. The still open points are something that we better solve in follow-up PRs. I only recommend to fix some issues in the git commit messages:

  1. s/parmeters/parameters/
  2. s/fix(test): Create/fix(test): create/
  3. "[feat(throttling): refactor the configuration logic". Either "feat" or "refactor", can't be both :) IMHO the commit should be squashed with ef8373f and e6bd688 anyway

@m-dati
Copy link
Contributor Author

m-dati commented Feb 8, 2026

About the non-squashed fix(test) commits, I did so after a peer-coding section, fixing unit tests impacted by the new code, having been suggested to keep separated the commits.
Therefore there are commit 'jumps', making the feat commits separated; willing to reorder the git hashes to unify those three in one, could cause conflicts in the code changes. But I try it.

@m-dati m-dati force-pushed the limit_resources_1 branch from 64d82b1 to 576c99c Compare February 8, 2026 16:16
@m-dati
Copy link
Contributor Author

m-dati commented Feb 8, 2026

Really unclear what CI error is: despite All tests successful. Result: PASS, then not ok - unhandled output found

@okurz
Copy link
Member

okurz commented Feb 9, 2026

Really unclear what CI error is: despite All tests successful. Result: PASS, then not ok - unhandled output found

It says "not ok - unhandled output found" so you need to ensure that tests run without any output. There must only be the line "$testname .... ok", nothing else.

See

[2026-02-08 16:31:37.49638] [2279] [warn] No configuration files supplied, will fallback to default configuration
[16:31:38] t/config.t ................................................ ok     1728 ms ( 0.01 usr  0.00 sy…

@m-dati m-dati force-pushed the limit_resources_1 branch 4 times, most recently from 62df4b4 to 0219620 Compare February 9, 2026 15:07
@m-dati m-dati requested review from Martchus, okurz and perlpunk February 9, 2026 15:40
@m-dati
Copy link
Contributor Author

m-dati commented Feb 9, 2026

CI successful, finally.

return;
}
my %hash = map { my ($k, $s, $r) = $_ =~ /$u/g; $k => {scale => $s, reference => $r // 0} }
split(',', $throttling);
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember that split always splits on a regex, even if you pass it as a simple string.

  step 1: define a priority scaling for jobs resources based on a configuration
    assign the configuration to parameter prio_throttling_parameters in openqa.ini
  step 2: apply changes requested in PR#6952
    Apply the priority throttling as by ticket's Suggestions in the description
    use configuration format "<parameter_name>:<scale_decimal>:<reference_valuee>,..."
    Set throttling code in create_from_settings,
    Add 2 subtests in priority tests of t/api/04-jobs.t
  step 3: 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

Ticket: https://progress.opensuse.org/issues/192952
@perlpunk
Copy link
Contributor

perlpunk commented Feb 10, 2026

We were in a collab session and refactored the throttling into its own function.
I pushed a commit on top.

... and use this in the tests to avoid executing the whole job posting process.
@mergify mergify bot merged commit 1141564 into os-autoinst:master Feb 10, 2026
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants