dmwm / WMCore

Core workflow management components for CMS.
Apache License 2.0
46 stars 107 forks source link

Add setter methods map to WMWorkload and call it for all reqArg parameters #12099

Open todor-ivanov opened 2 months ago

todor-ivanov commented 2 months ago

Fixes #12038

Status

Ready

Description

With the current PR we add the new functionality to call all needed WMWorkload setter methods based on a full reqArgs dictionary passed from the upper level caller and try to set all of parameters at once, rather than calling every single setter method one by one. This is achieved by creating a proper map between methods and possible request arguments. And later validating if the set of arguments passed with reqArgs dictionary would properly match the signature of the setter method which is to be called. In the current implementation only a small set of methods is mapped to request parameters :

With this change a path for updating all possible workqueue elements in a single go was opened. In the WorkQueue service an additional method was developed for fetching all possible workqueue elements and update them all with the full set of arguments provided through the reqArgs dictionary with the cost of a single database action per WorkQuee element, rather than 3 (or more) separate calls to the database for updating every single element parameter separately. Upon updating all workqueue elements with the new parameters the WMSpec copy of the given workflow at the workqueue is also updated in a single push using the same logic from above.

Is it backward compatible (if not, which system it affects?)

YES

Related PRs

None

External dependencies / deployment changes

None

cmsdmwmbot commented 2 months ago

Jenkins results:

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/15218/artifact/artifacts/PullRequestReport.html

cmsdmwmbot commented 2 months ago

Jenkins results:

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/15219/artifact/artifacts/PullRequestReport.html

todor-ivanov commented 2 months ago

@amaltaro Please take a look at this PR, I think this one fully covers all the requirements and addresses our fears for affecting scalability due to increased database calls.

cmsdmwmbot commented 2 months ago

Jenkins results:

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/15220/artifact/artifacts/PullRequestReport.html

cmsdmwmbot commented 1 month ago

Jenkins results:

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/15240/artifact/artifacts/PullRequestReport.html

cmsdmwmbot commented 1 month ago

Jenkins results:

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/15241/artifact/artifacts/PullRequestReport.html

todor-ivanov commented 1 month ago

hi @amaltaro I have addressed your comments - Mostly, I removed the WQE status filter, based on the investigation I did on the JobUpdater component. You may take another look.

cmsdmwmbot commented 1 month ago

Jenkins results:

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/15249/artifact/artifacts/PullRequestReport.html

cmsdmwmbot commented 1 month ago

Can one of the admins verify this patch?

anpicci commented 1 month ago

@todor-ivanov apart from my comment to @amaltaro 's review, I only suggest to resolve the conflicts on src/python/WMCore/ReqMgr/Service/Request.py, if you have not considered yet to address that

anpicci commented 1 month ago

@vkuznet thank you for the feedback. @todor-ivanov @amaltaro , I propose to proceed as follows:

I want to clarify that I would like to adopt these accountability principles to every issue, meaning that this is not a special treatment outlined only for this PR. I will follow up on this during the next group meeting.

Thanks!