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

plan classes: add workunit-related filters #2003

Merged
merged 4 commits into from
Jul 26, 2018

Conversation

bema-aei
Copy link
Contributor

@bema-aei bema-aei commented Aug 2, 2017

filters <min_wu_id>, <max_wu_id>, <min_batch>, <max_batch> and

This is a essentially a copy of #2000 which was merged prematurely (with an unfinished discussion) and later reverted in #2002 (without any discussion).

  • WU information is passed from check_homogeneous_app_version() down to PLAN_CLASS::check()

  • wu_is_infeasible_custom() is extended such that additional tasks also follow the plan class restrictions.

@bema-aei
Copy link
Contributor Author

bema-aei commented Aug 2, 2017

@davidpanderson If you aren't sure or convinced, please utter your concerns in the comments and only "merge" when you're sure and the discussion is finished (and tests have completed).

@bema-aei bema-aei force-pushed the plan_classes_add_wu_filters branch from ae1edb1 to aa9d1e3 Compare August 2, 2017 11:41
@bema-aei
Copy link
Contributor Author

bema-aei commented Aug 2, 2017

@davidpanderson [copied from #2000]:

There may be a problem with this, because the app version selection mechanism caches the selection. It might see an old WU, choose an app version, and then use the same version for a later WU. Can you please describe the use case in detail?

The modification of PLAN_CLASS::check() selects an application version that matches the WU criteria (max/min WU id, max/min batch). The addition to wu_is_infeasible_custom() ensures that subsequent tasks that get selected for this app version also conform to the WU criteria for the app version's plan class. wu_is_infeasible_custom() calls plan_class_specs.wu_is_infeasible(plan_class, wu), which looks up the WU criteria for this plan class and checks whether the WU conforms. WUs that don't conform to the plan class criteria get rejected as 'infeasible'.

@bema-aei
Copy link
Contributor Author

bema-aei commented Aug 3, 2017

I already edited https://boinc.berkeley.edu/trac/wiki/AppPlanSpec as I saw #2000 was merged. If this PR is not going through, the page has to be reverted to version 35.

@TheAspens
Copy link
Member

@bema-ligo - with the changes that @davidpanderson made recently - is this pull request still required or can this be closed?

@bema-aei
Copy link
Contributor Author

@TheAspens

@bema-ligo - with the changes that @davidpanderson made recently - is this pull request still required or can this be closed?

This wasn't covered in #2538, and the workunit app version assignment that @davidpanderson mentioned in the discussion at boinc_projects mailing list is not equivalent and doesn't cover our use case. If E@H should be able to use current server code, this PR would be a requirement. However I don't want to invest any more time in this PR (like for rebasing it) if it's going to be rejected anyway.

@davidpanderson
Copy link
Contributor

davidpanderson commented Jun 13, 2018 via email

@bema-aei
Copy link
Contributor Author

bema-aei commented Jun 13, 2018

@davidpanderson Citing from my mail on boinc_projects:

As far as I understand it this feature allows to pin certain workunits to be processed with app versions with one particular version mumber (only). If the respective DB field is unassigned (NUL, zero or whatever), then the workunit can and will be processed with any app version. This wouldn't work for us for a number of reasons:

  • For most applications we don't use BOINC's "create_work" script / tool, but custom workunit generators. We would need to change all that workunit generator codes to use that feature.

  • The app version to process a certain workunit with needs to be known at the create time of the workunit. Usually we discover a bug in an application, fix it in a new app version, and THEN find out that this fixed app (version) is incompatible with the already generated workunits, so after (a lot of) workunits have been generated. It is also not uncommon due to architecture-specific fixes & optimizations that the app versions in use at that time have different version numbers, so you can't even assign the old workunits to one specific version number in hindsight. At least durin transition time there is no way to prevent that "old" workunits are processed with "new" app versions, or worse, with a combination of "old" and "new".

  • The plan class feature allows to assign the WU limits to plan classes instead of app version numbers. This allows e.g. to specify something like "process these workunits with app versions NEWER (or, as in the above example OLDER) than a given app version", when the need arises. We don't want to change that pinning every time we bring out a new app version.

@davidpanderson
Copy link
Contributor

OK, I can see the need for this; sorry for the delay.
Can you please make the following small changes?

  • move the code that does the WU check into a function,
    called from both app_plan and wu_is_infeasible

  • keep track of whether any plan class has a WU restriction (in a bool in PLAN_CLASS_SPECS).
    In wu_is_infeasible_custom(), only call plan_class_specs.wu_is_infeasible() if this is true
    (avoid unneeded work for projects that don't use WU restrictions).

With these changes I'll be happy to merge.

…atch> and <disabled>

- WU information is passed from check_homogeneous_app_version()
  down to PLAN_CLASS::check()

- wu_is_infeasible_custom() is extended such that additional
  tasks also follow the plan class restrictions.
@bema-aei bema-aei force-pushed the plan_classes_add_wu_filters branch from aa9d1e3 to a92f567 Compare June 19, 2018 05:42
- i.e. if there exists a plan class with WU restrictions
@TheAspens
Copy link
Member

@davidpanderson - it looks like @bema-ligo made the changes you requested. Do you mind taking a look and seeing if it is ready to be merged? Thanks!

from that branch, doesn't belong here
…sses

- if there are no plan classes that restrict workunits
@bema-aei bema-aei force-pushed the plan_classes_add_wu_filters branch from ef5b62d to 05742a0 Compare July 25, 2018 10:31
@davidpanderson davidpanderson merged commit 0cc45a1 into BOINC:master Jul 26, 2018
@AenBleidd AenBleidd added this to the Client Release 7.14 milestone Oct 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants