Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client/app interface: pass priority-related user preferences to wrappers #3826

Closed
wants to merge 2 commits into from

Conversation

davidpanderson
Copy link
Contributor

Fixes #3764

Description of the Change

Alternate Designs

Release Notes

Remote job submission was always using allocation-based prioritization.
This is not necessarily the right thing.
For example, nanoHUB submits batches of jobs that are mixtures
of speculative and user-submitted;
these simply need to be given low and high priorities.

The way things work now:
- if you want allocation-based prioritization,
    set the "allocation_priority" flag in the request object
    (Python or PHP API)
- or set the "priority" field in the request object;
    that sets the priority of all the jobs in the batch
- or set the "priority" field of jobs in the batch.

Updated wiki docs accordingly.

Also fixed a bug in the test script
(note to self: adding an object to an array adds a reference, not a copy).
The user can specify 3 priority-related prefs in cc_config.h:
bool no_priority_change
int process_priority
int process_priority_special

Wrappers should honor these prefs,
but currently they don't have access to them.
Pass them in the APP_INIT_DATA structure.

Modify wrapper.cpp to use these prefs if they exist;
this overrides the priority specified in the job file, if any.
@AenBleidd
Copy link
Member

@davidpanderson, I believe this commit bed2cd4 is not related to this particular PR.

@RichardHaselgrove
Copy link
Contributor

I see two different problems with the 'primary purpose' section of this PR (f257fbc).

1. Design

The need identified in #3764 was to adjust the priority of a single GPU app. It was the GPU nature of the app which was primary: the fact that it had been deployed via a wrapper was an incidental impediment.

Because cc_config.xml is global across all projects attached to the client, a 'process_priority_special' designed for that GPU app will also be applied to any CPU wrapper apps that may have been issued by another project. It would have been more appropriate to design this into the 'Project-level configuration' area, perhaps even down to the app_version level.

2. Implementation

Wrapper app priorities in the client are set (as in this PR) at https://github.com/BOINC/boinc/blob/master/lib/proc_control.cpp#L264 ff, using definitions contained in https://github.com/BOINC/boinc/blob/master/lib/common_defs.h#L81. This defines a base-1 list of process priorities:

1: lowest (Win: IDLE; Unix: 19)
2: low (Win: BELOW_NORMAL; Unix: 10
3: normal (Win NORMAL; Unix: 0)
4: high (Win: ABOVE_NORMAL; Unix: -10)
5: highest (Win: HIGH; Unix: -16)

However, for native (non-wrapper) apps, the client uses different process control logic, found at https://github.com/BOINC/boinc/blob/master/client/app_start.cpp#L508 ff, which hard-codes a base-0 list of process priorities:

    case 0: return IDLE_PRIORITY_CLASS;
    case 1: return BELOW_NORMAL_PRIORITY_CLASS;
    case 2: return NORMAL_PRIORITY_CLASS;
    case 3: return ABOVE_NORMAL_PRIORITY_CLASS;
    case 4: return HIGH_PRIORITY_CLASS;
    case 5: return REALTIME_PRIORITY_CLASS;

The implementation in cc_config.xml will use the same user-selected values against the two different process priority lists, and the results will differ from their expectations in at least one of the two cases.

For this reason, I am reverting the change made this morning to the User Manual (which appears to have been prompted by this confusion over process priority lists): it is important (by definition) that the User Manual describes the current behaviour of the public-release client, not the future behaviour of a putative revised client.

@RichardHaselgrove
Copy link
Contributor

@davidpanderson: Please cast your eye back over your two commits:

317dc91 (13 Oct 2014, for wrapper apps)
fcafba0 (29 Sep 2015, for 'native' apps)

I think these two, taken together, are where the confusion has arisen.

For the record, I still feel that the User Manual, relating to current clients, should reflect the 2015 numbering for native apps.

@davidpanderson
Copy link
Contributor Author

Oops! somehow this PR combined two unrelated things. I split them into separate PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

process priority flags don't work for wrapper tasks
3 participants