PART 2: fix: only report job limit in UI when globally reached - After #7232#7234
Draft
okurz wants to merge 3 commits intoos-autoinst:masterfrom
Draft
Conversation
okurz
changed the title
Motivation: The dynamic limit tests contained significant repetition in setup and execution logic, making them harder to maintain and read. Consolidating them into a table-driven structure further reduces line count and improves readability. Design Choices: - Introduced test_load_avg helper to encapsulate load parsing tests. - Implemented a table-driven test structure for scaling logic, using an array of case hashes and a test_dynamic_limit helper. - Used sensible defaults in helpers to minimize configuration noise in the test table. - Sanitized temp file prefixes to handle test names with special characters safely. Benefits: - Significantly reduced line count (~100 lines vs ~157 originally). - Improved maintainability through deduplication and a compact structure. - Better focus on test data rather than boilerplate setup. Related issue: os-autoinst#7202
Motivation: The initial dynamic global job limit was too slow to scale up when the system was idle, and its scaling factors were hardcoded and repeated across the codebase, making maintenance difficult. Design Choices: - Implemented 'fast ramp-up' logic doubling the step size when load is low. - Centralized all default scaling parameters as constants in DynamicLimit.pm. - Updated OpenQA::Setup to use these constants for the configuration schema by referencing DynamicLimit->DEFAULTS. - Refactored tests to rely on centralized constants and DEFAULTS, reducing repetition of literal values like '0.85' and '50'. - Added current min/max values to "Dynamic job limit" log messages. Benefits: - Faster utilization of spare host capacity (reaching ceilings ~4x faster). - Single source of truth for all dynamic limit defaults. - Cleaner and more maintainable codebase and tests. - Better operational visibility into the dynamic scaling behavior. Related issue: https://progress.opensuse.org/issues/198770
Motivation: The /tests page often displayed "(limited by server config)" even when far below the actual limit. This happened because the backend was reporting the limit based on filtered job counts rather than global counts, and was unconditionally sending the static limit parameter when dynamic limits were enabled. Design Choices: - Updated list_running_ajax to calculate the global count of running jobs. - Only include dynamic_job_limit or max_running_jobs in the API response when the global count actually reaches or exceeds the respective limit. - Deduplicated configuration extraction in DynamicLimit.pm by mapping over the DEFAULTS hash. - Refactored t/ui/01-list.t to reduce repetition of the scheduler config path by using a shared $scheduler_config variable. - Improved test description strings in t/ui/01-list.t and t/26-dynamic-job-limit.t to be self-explanatory, eliminating the need for in-file comments. - Added comprehensive subtests to verify limit reporting behavior with filters and both dynamic/static configurations. Benefits: - Eliminates misleading "limited by server config" messages. - Provides accurate server-wide limit status to users even in filtered views. - Cleaner and more maintainable test and implementation code. Related issue: https://progress.opensuse.org/issues/198770
okurz
force-pushed
the
feature/057_poo198770_dynamic_global_job_limit_limited_by_server_config
branch
from
46184e1 to
57df01c
Compare
Contributor
|
This pull request is now in conflicts. Could you fix it? 🙏 |
okurz
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
The /tests page often displayed "(limited by server config)" even when far
below the actual limit. This happened because the backend was reporting the
limit based on filtered job counts rather than global counts, and was
unconditionally sending the static limit parameter when dynamic limits were
enabled.
Design Choices:
the global count actually reaches or exceeds the respective limit.
the DEFAULTS hash.
by using a shared $scheduler_config variable.
t/26-dynamic-job-limit.t to be self-explanatory, eliminating the need for
in-file comments.
filters and both dynamic/static configurations.
Benefits:
Related issue: https://progress.opensuse.org/issues/198770
After: